On Tue, Apr 07, 2015 at 03:51:44PM +0200, Maarten Lankhorst wrote: > Op 07-04-15 om 15:37 schreef Daniel Vetter: > > On Tue, Apr 07, 2015 at 11:32:02AM +0200, Maarten Lankhorst wrote: > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > >> --- > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> index b13c5526a73b..7aaf8eddf19c 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -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); > > 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. 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx