drm/i915: use kref_put_mutex in i915_gem_request_unreference__unlocked
Subsystems (2)
PATCH 1/1 drm/i915: use kref_put_mutex in i915_gem_request_unreference__unlocked
---
drivers/gpu/drm/i915/i915_drv.h +6 -6
@@ -2146,14 +2146,14 @@ i915_gem_request_unreference(struct drm_i915_gem_request *req)static inline void i915_gem_request_unreference__unlocked (struct drm_i915_gem_request *req) { -if (req && !atomic_add_unless (&req->ref.refcount,-1 ,1 )) { -struct drm_device *dev = req->ring->dev; +struct drm_device *dev; + +if (!req) +return ; -mutex_lock (&dev->struct_mutex); -if (likely (atomic_dec_and_test (&req->ref.refcount))) -i915_gem_request_free (&req->ref); + dev = req->ring->dev; +if (kref_put_mutex (&req->ref, i915_gem_request_free, &dev->struct_mutex))mutex_unlock (&dev->struct_mutex); - } } static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst,
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 13 lines · Maarten Lankhorst
Signed-off-by Maarten Lankhorst <[email protected]>---
drivers/gpu/drm/i915/i915_drv.h +1
-2@@ -2146,14 +2146,14 @@ i915_gem_request_unreference(struct drm_i915_gem_request *req)static inline void i915_gem_request_unreference__unlocked (struct drm_i915_gem_request *req) { -if (req && !atomic_add_unless (&req->ref.refcount,-1 ,1 )) { -struct drm_device *dev = req->ring->dev; +struct drm_device *dev;
drivers/gpu/drm/i915/i915_drv.h i915_gem_request_unreference(struct drm_i915_gem_request *req) · line 2156
Quoted 10 lines
drivers/gpu/drm/i915/i915_drv.h +5
-3+ +if (!req) +return ; -mutex_lock (&dev->struct_mutex); -if (likely (atomic_dec_and_test (&req->ref.refcount))) -i915_gem_request_free (&req->ref); + dev = req->ring->dev; +if (kref_put_mutex (&req->ref, i915_gem_request_free, &dev->struct_mutex))mutex_unlock (&dev->struct_mutex);
We don't need this conditional unlock here since that's only possible if you have a weak reference somewhere (i.e. using kref_get_unless_zero). If the object only has strong references and you're dropping the last one it can't magically get resurrected somehow.
And drm_gem_object_unreference_unlocked wants the same patch I think. -Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 07-04-15 om 15:37 schreef Daniel Vetter:
Quoted 18 lines · Maarten Lankhorst
Quoted 17 lines
Signed-off-by Maarten Lankhorst <[email protected]>---
drivers/gpu/drm/i915/i915_drv.h +4
-2@@ -2146,14 +2146,14 @@ i915_gem_request_unreference(struct drm_i915_gem_request *req)static inline void i915_gem_request_unreference__unlocked (struct drm_i915_gem_request *req) { -if (req && !atomic_add_unless (&req->ref.refcount,-1 ,1 )) { -struct drm_device *dev = req->ring->dev; +struct drm_device *dev; + +if (!req) +return ;
Quoted 10 lines
drivers/gpu/drm/i915/i915_drv.h i915_gem_request_unreference(struct drm_i915_gem_request *req) · line 2156
Quoted 6 lines
drivers/gpu/drm/i915/i915_drv.h +2
-3-mutex_lock (&dev->struct_mutex); -if (likely (atomic_dec_and_test (&req->ref.refcount))) -i915_gem_request_free (&req->ref); + dev = req->ring->dev; +if (kref_put_mutex (&req->ref, i915_gem_request_free, &dev->struct_mutex))mutex_unlock (&dev->struct_mutex);We don't need this conditional unlock here since that's only possible if you have a weak reference somewhere (i.e. using
kref_get_unless_zero). If the object only has strong references and you're dropping the last one it can't magically get resurrected somehow.
Because we use the same put call for kref_put and kref_put_mutex we do need to unlock struct_mutex here, kref_put_mutex doesn't release the mutex if true, so either the release call needs to do it or the callee.
Quoted 1 line
And
drm_gem_object_unreference_unlockedwants the same patch I think.
Indeed it does, but with slightly more lockdep annotation!
_______________________________________________
Intel-gfx mailing list [email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 29 lines · Maarten Lankhorst
Op 07-04-15 om 15:37 schreef Daniel Vetter:
Quoted 18 lines · Maarten Lankhorst
Quoted 17 lines
Signed-off-by Maarten Lankhorst <[email protected]>---
drivers/gpu/drm/i915/i915_drv.h +4
-2@@ -2146,14 +2146,14 @@ i915_gem_request_unreference(struct drm_i915_gem_request *req)static inline void i915_gem_request_unreference__unlocked (struct drm_i915_gem_request *req) { -if (req && !atomic_add_unless (&req->ref.refcount,-1 ,1 )) { -struct drm_device *dev = req->ring->dev; +struct drm_device *dev; + +if (!req) +return ;Quoted 10 lines
drivers/gpu/drm/i915/i915_drv.h i915_gem_request_unreference(struct drm_i915_gem_request *req) · line 2156
Quoted 6 lines
drivers/gpu/drm/i915/i915_drv.h +2
-3-mutex_lock (&dev->struct_mutex); -if (likely (atomic_dec_and_test (&req->ref.refcount))) -i915_gem_request_free (&req->ref); + dev = req->ring->dev; +if (kref_put_mutex (&req->ref, i915_gem_request_free, &dev->struct_mutex))mutex_unlock (&dev->struct_mutex);We don't need this conditional unlock here since that's only possible if you have a weak reference somewhere (i.e. using
kref_get_unless_zero). If the object only has strong references and you're dropping the last one it can't magically get resurrected somehow.
Quoted 2 lines
Because we use the same put call for
kref_putandkref_put_mutexwe do need to unlockstruct_mutexhere,kref_put_mutexdoesn't release the mutex if true, so either the release call needs to do it or the callee.
Indeed I didn't realize that all existing users of kref_put_mutex unlock the mutex in the free callback. So looks all good, so applied it.
Thanks, Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx