On Mon, Jun 13, 2016 at 11:19:20AM -0400, Alex Deucher wrote: > On Thu, Jun 9, 2016 at 3:29 PM, Rob Clark <robdclark@xxxxxxxxx> wrote: > > There were a couple messed up things about this fail path. > > (1) it would drop object_name_lock twice > > (2) drm_gem_handle_delete() (in drm_gem_remove_prime_handles()) > > needs to grab prime_lock > > > > Reported-by: Alex Deucher <alexdeucher@xxxxxxxxx> > > Signed-off-by: Rob Clark <robdclark@xxxxxxxxx> > > --- > > Untested, but I think this should fix the potential deadlock that Alex > > reported. Would be nice if someone with setup to reproduce could test > > this. > > Sorry for the confusion. There were actually two deadlocks. The > first one (https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1579610 > fixed by https://github.com/torvalds/linux/commit/6984128d01cf935820a0563f3a00c6623ba58109) > was that one we hit in testing. This one was just one that Qiang > noticed by inspection while debugging the first. That said, the error > path is obviously wrong and the patch looks correct to me. > > Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx> Ok, I didn't add the bugzilla to this one. Applied to drm-misc, thanks. -Daniel > > > > > drivers/gpu/drm/drm_prime.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > index aab0f3f..780589b 100644 > > --- a/drivers/gpu/drm/drm_prime.c > > +++ b/drivers/gpu/drm/drm_prime.c > > @@ -593,7 +593,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, > > get_dma_buf(dma_buf); > > } > > > > - /* drm_gem_handle_create_tail unlocks dev->object_name_lock. */ > > + /* _handle_create_tail unconditionally unlocks dev->object_name_lock. */ > > ret = drm_gem_handle_create_tail(file_priv, obj, handle); > > drm_gem_object_unreference_unlocked(obj); > > if (ret) > > @@ -601,11 +601,10 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, > > > > ret = drm_prime_add_buf_handle(&file_priv->prime, > > dma_buf, *handle); > > + mutex_unlock(&file_priv->prime.lock); > > if (ret) > > goto fail; > > > > - mutex_unlock(&file_priv->prime.lock); > > - > > dma_buf_put(dma_buf); > > > > return 0; > > @@ -615,11 +614,14 @@ fail: > > * to detach.. which seems ok.. > > */ > > drm_gem_handle_delete(file_priv, *handle); > > + dma_buf_put(dma_buf); > > + return ret; > > + > > out_unlock: > > mutex_unlock(&dev->object_name_lock); > > out_put: > > - dma_buf_put(dma_buf); > > mutex_unlock(&file_priv->prime.lock); > > + dma_buf_put(dma_buf); > > return ret; > > } > > EXPORT_SYMBOL(drm_gem_prime_fd_to_handle); > > -- > > 2.5.5 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel