2014-09-24 16:55 GMT-03:00 Daniel Vetter <daniel.vetter@xxxxxxxx>:
> Paulo Zanoni reported a lockdep splat with a locking inversion between
> fpriv->fbs_lock and the modeset locks. This issue was introduced in
>
> commit f2b50c1161590c3bcdbf3455fe4c575f1c1bd293
> Author: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Date: Fri Sep 12 17:07:32 2014 +0200
>
> drm: Fixup locking for universal cursor planes
>
> This here is actually one of the rare cases where lockdep hits a false
> positive: The deadlock only happens in drm_fb_release, which cleans up
> the file private structure when all the references are gone. So the
> locking is the very last one and no one else can deadlock. It also
> doesn't protect anything at all, since all ioctls are guaranteed to
> have returned at this point - otherwise they'd still hold a reference
> on the file.
>
> So let's just drop it and replace it with a big comment.
>
> Cc: David Herrmann <dh.herrmann@xxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Cc: Paulo Zanoni <przanoni@xxxxxxxxx>
> Reported-by: Paulo Zanoni <przanoni@xxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Apparently, it fixes the problem I was seeing while running
igt/pm_rpm. It was not 100% reproducible, but it seems to be gone.
Smoke-tested-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
Testcase: igt/pm_rpm
> ---
> drivers/gpu/drm/drm_crtc.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index b7021069b078..e79c8d3700d8 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3400,7 +3400,16 @@ void drm_fb_release(struct drm_file *priv)
> struct drm_device *dev = priv->minor->dev;
> struct drm_framebuffer *fb, *tfb;
>
> - mutex_lock(&priv->fbs_lock);
> + /*
> + * When the file gets released that means no one else can access the fb
> + * list any more, so no need to grab fpriv->fbs_lock. And we need to to
> + * avoid upsetting lockdep since the universal cursor code adds a
> + * framebuffer while holding mutex locks.
> + *
> + * Note that a real deadlock between fpriv->fbs_lock and the modeset
> + * locks is impossible here since no one else but this function can get
> + * at it any more.
> + */
> list_for_each_entry_safe(fb, tfb, &priv->fbs, filp_head) {
>
> mutex_lock(&dev->mode_config.fb_lock);
> @@ -3413,7 +3422,6 @@ void drm_fb_release(struct drm_file *priv)
> /* This will also drop the fpriv->fbs reference. */
> drm_framebuffer_remove(fb);
> }
> - mutex_unlock(&priv->fbs_lock);
> }
>
> /**
> --
> 2.1.0
>
--
Paulo Zanoni
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
