[PATCH] drm: Don't allow interruptions when opening debugfs/crc

[Ville Syrjälä]

On Fri, Apr 07, 2017 at 12:17:12PM +0100, Chris Wilson wrote:
> The code does not like to be interrupted when waiting for the first
> vblank after opening a debugfs/crc channel, so don't.
> 
> [66285.716870] WARNING: CPU: 1 PID: 16615 at drivers/gpu/drm/drm_debugfs_crc.c:185 crtc_crc_open+0x1d0/0x1f0 [drm]
> [66285.716877] Modules linked in: i915 intel_powerclamp crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel cryptd intel_gtt i2c_algo_bit lpc_ich mfd_core drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops prime_numbers drm video button autofs4 sd_mod ahci libahci libata i2c_i801 scsi_mod i2c_designware_platform i2c_designware_core i2c_core
> [66285.716929] CPU: 1 PID: 16615 Comm: kms_frontbuffer Not tainted 4.11.0-rc5+ #7
> [66285.716935] Hardware name: GIGABYTE GB-BXBT-1900/MZBAYAB-00, BIOS F8 03/02/2016
> [66285.716941] Call Trace:
> [66285.716955]  dump_stack+0x4d/0x6f
> [66285.716966]  __warn+0xc1/0xe0
> [66285.716975]  warn_slowpath_null+0x18/0x20
> [66285.717004]  crtc_crc_open+0x1d0/0x1f0 [drm]
> [66285.717014]  ? wake_atomic_t_function+0x50/0x50
> [66285.717024]  full_proxy_open+0xf0/0x1b0
> [66285.717032]  ? full_proxy_release+0x80/0x80
> [66285.717042]  do_dentry_open.isra.17+0x14b/0x2d0
> [66285.717051]  vfs_open+0x42/0x60
> [66285.717064]  path_openat+0x5e7/0x13d0
> [66285.717074]  ? refcount_dec_and_test+0x11/0x20
> [66285.717081]  ? down_read+0xd/0x30
> [66285.717087]  do_filp_open+0x85/0xf0
> [66285.717093]  ? __vfs_write+0x23/0x120
> [66285.717100]  ? __alloc_fd+0x3a/0x170
> [66285.717107]  do_sys_open+0x11e/0x1f0
> [66285.717113]  ? do_sys_open+0x11e/0x1f0
> [66285.717119]  SyS_openat+0xf/0x20
> [66285.717125]  entry_SYSCALL_64_fastpath+0x17/0x98
> [66285.717131] RIP: 0033:0x7f5f2235146a
> [66285.717135] RSP: 002b:00007ffd892e6bc0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
> [66285.717142] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f5f2235146a
> [66285.717147] RDX: 0000000000000000 RSI: 00007ffd892e6c40 RDI: 0000000000000006
> [66285.717151] RBP: 00007ffd892e6b20 R08: 0000000000000000 R09: 000000000000000f
> [66285.717156] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> [66285.717161] R13: 00007ffd892e6b10 R14: 0000000000000004 R15: 00000000007e61f4
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100610
> Fixes: e8fa5671183c ("drm: crc: Wait for a frame before returning from open()")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tomeu Vizoso <tomeu.vizoso at collabora.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/drm_debugfs_crc.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
> index 1722d8f21449..aa13e734c9e5 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -177,13 +177,9 @@ static int crtc_crc_open(struct inode *inode, struct file *filep)
>  	 * guess when this particular piece of HW will be ready to start
>  	 * generating CRCs.
>  	 */
> -	ret = wait_event_interruptible_lock_irq(crc->wq,
> -						crtc_crc_data_count(crc),
> -						crc->lock);
> +	wait_event_lock_irq(crc->wq, crtc_crc_data_count(crc), crc->lock);

What if the crc never gets produced? The first read will block if
the user asked it to block, so I don't really see the point of this
wait at all.

Seems to me that the crc thing needs to be pimped work better in cases
where the crtc is disabled. Currently it's kinda trying to hide a bit
too much to be useful for observing the hardware behaviour during crtc
enable/disable. I have definitely used it in the past to observe that,
but I'm not sure the current thing really works for that anymore.

>  	spin_unlock_irq(&crc->lock);
>  
> -	WARN_ON(ret);
> -
>  	return 0;
>  
>  err_disable:
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC