Re: [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer
Date: Wed, 30 Mar 2016 20:50:14 +0200	[thread overview]
Message-ID: <20160330185014.GQ2510@phenom.ffwll.local> (raw)
In-Reply-To: <1459347521-31558-1-git-send-email-chris@chris-wilson.co.uk>

On Wed, Mar 30, 2016 at 03:18:40PM +0100, Chris Wilson wrote:
> Since describe_obj() looks at state guarded by the struct_mutex, we need
> to be holding it.
> 
> [  580.201054] drv_suspend: starting subtest debugfs-reader
> [  580.239652] ------------[ cut here ]------------
> [  580.239696] WARNING: CPU: 0 PID: 920 at include/linux/list_check.h:25 describe_obj+0x419/0x440()
> [  580.239725] CPU: 0 PID: 920 Comm: cat Not tainted 4.5.0-rc6+ #835
> [  580.239745] Hardware name:                  /NUC5CPYB, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [  580.239767]  0000000000000000 ffff88027554fcf8 ffffffff812c1135 0000000000000000
> [  580.239815]  ffffffff8193dc42 ffff88027554fd30 ffffffff8107419d ffff880071727c00
> [  580.239858]  ffff8802757d8000 ffffffff818f693c ffffffff818f693c ffff8802757b9048
> [  580.239896] Call Trace:
> [  580.239917]  [<ffffffff812c1135>] dump_stack+0x67/0x92
> [  580.239939]  [<ffffffff8107419d>] warn_slowpath_common+0x7d/0xb0
> [  580.239959]  [<ffffffff810742ba>] warn_slowpath_null+0x1a/0x20
> [  580.239981]  [<ffffffff813ce579>] describe_obj+0x419/0x440
> [  580.240006]  [<ffffffff813ced22>] i915_gem_framebuffer_info+0xa2/0x100
> [  580.240033]  [<ffffffff811a9286>] seq_read+0xe6/0x3b0
> [  580.240059]  [<ffffffff81182288>] __vfs_read+0x28/0xd0
> [  580.240085]  [<ffffffff81173378>] ? SyS_fadvise64+0x228/0x2c0
> [  580.240112]  [<ffffffff811823b2>] vfs_read+0x82/0x110
> [  580.240137]  [<ffffffff811827d9>] SyS_read+0x49/0xa0
> [  580.240162]  [<ffffffff815bac57>] entry_SYSCALL_64_fastpath+0x12/0x6b
> [  580.240187] ---[ end trace 3e2cbf34576c9878 ]---
> [  580.281900] ------------[ cut here ]------------

Missing sob-line here. With that fixed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d02f8ce0b1c8..adabbb47a8aa 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -134,6 +134,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
>  	int pin_count = 0;
>  	enum intel_engine_id id;
>  
> +	lockdep_assert_held(&obj->base.dev->struct_mutex);
> +
>  	seq_printf(m, "%pK: %s%s%s%s %8zdKiB %02x %02x [ ",
>  		   &obj->base,
>  		   obj->active ? "*" : " ",
> @@ -1895,6 +1897,11 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
>  	struct drm_device *dev = node->minor->dev;
>  	struct intel_framebuffer *fbdev_fb = NULL;
>  	struct drm_framebuffer *drm_fb;
> +	int ret;
> +
> +	ret = mutex_lock_interruptible(&dev->struct_mutex);
> +	if (ret)
> +		return ret;
>  
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
>         if (to_i915(dev)->fbdev) {
> @@ -1929,6 +1936,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
>  		seq_putc(m, '\n');
>  	}
>  	mutex_unlock(&dev->mode_config.fb_lock);
> +	mutex_unlock(&dev->struct_mutex);
>  
>  	return 0;
>  }
> -- 
> 2.8.0.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx