| Message ID | 20180802174442.7310-1-bart.vanassche@wdc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | sr: Avoid that opening a CD-ROM hangs with runtime power management enabled | expand |
On Thu, Aug 02, 2018 at 10:44:42AM -0700, Bart Van Assche wrote: > Surround scsi_execute() calls with scsi_autopm_get_device() and > scsi_autopm_put_device(). Note: removing sr_mutex protection from > the scsi_cd_get() and scsi_cd_put() calls is safe because the > purpose of sr_mutex is to serialize cdrom_*() calls. > [...] Hi Bart, With this patch I still have the hang on next-20180801: [ 247.875657] INFO: task swapper/0:1 blocked for more than 120 seconds. [ 247.877560] Not tainted 4.18.0-rc7-next-20180801+ #131 [ 247.878703] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 247.880497] swapper/0 D13496 1 0 0x80000000 [ 247.881542] Call Trace: [ 247.882638] ? __schedule+0x253/0x700 [ 247.882898] schedule+0x23/0x80 [ 247.883149] async_synchronize_cookie_domain+0x89/0xd0 [ 247.883509] ? wait_woken+0x80/0x80 [ 247.883712] ? rest_init+0xb0/0xb0 [ 247.883907] kernel_init+0xa/0x100 [ 247.884096] ret_from_fork+0x35/0x40 [ 370.755258] INFO: task swapper/0:1 blocked for more than 120 seconds. [ 370.756735] Not tainted 4.18.0-rc7-next-20180801+ #131 [ 370.757931] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 370.759417] swapper/0 D13496 1 0 0x80000000 [ 370.760567] Call Trace: [ 370.761141] ? __schedule+0x253/0x700 [ 370.761982] schedule+0x23/0x80 [ 370.762534] async_synchronize_cookie_domain+0x89/0xd0 [ 370.762818] ? wait_woken+0x80/0x80 [ 370.763020] ? rest_init+0xb0/0xb0 [ 370.763279] kernel_init+0xa/0x100 [ 370.763478] ret_from_fork+0x35/0x40 [ 493.635108] INFO: task swapper/0:1 blocked for more than 120 seconds. [ 493.636403] Not tainted 4.18.0-rc7-next-20180801+ #131 [ 493.637449] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 493.638803] swapper/0 D13496 1 0 0x80000000 [ 493.639963] Call Trace: [ 493.640669] ? __schedule+0x253/0x700 [ 493.641446] schedule+0x23/0x80 [ 493.642023] async_synchronize_cookie_domain+0x89/0xd0 [ 493.643114] ? wait_woken+0x80/0x80 [ 493.643923] ? rest_init+0xb0/0xb0 [ 493.644411] kernel_init+0xa/0x100 [ 493.644680] ret_from_fork+0x35/0x40 [ 616.515472] INFO: task swapper/0:1 blocked for more than 120 seconds. [ 616.516773] Not tainted 4.18.0-rc7-next-20180801+ #131 [ 616.517790] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 616.519260] swapper/0 D13496 1 0 0x80000000 [ 616.520322] Call Trace: [ 616.520854] ? __schedule+0x253/0x700 [ 616.521662] schedule+0x23/0x80 [ 616.522249] async_synchronize_cookie_domain+0x89/0xd0 [ 616.523402] ? wait_woken+0x80/0x80 [ 616.524002] ? rest_init+0xb0/0xb0 [ 616.524779] kernel_init+0xa/0x100 [ 616.525054] ret_from_fork+0x35/0x40 [ 739.394997] INFO: task swapper/0:1 blocked for more than 120 seconds. [ 739.396351] Not tainted 4.18.0-rc7-next-20180801+ #131 [ 739.397411] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 739.398751] swapper/0 D13496 1 0 0x80000000 [ 739.399841] Call Trace: [ 739.400506] ? __schedule+0x253/0x700 [ 739.401152] schedule+0x23/0x80 [ 739.401856] async_synchronize_cookie_domain+0x89/0xd0 [ 739.402527] ? wait_woken+0x80/0x80 [ 739.402773] ? rest_init+0xb0/0xb0 [ 739.402997] kernel_init+0xa/0x100 [ 739.403311] ret_from_fork+0x35/0x40 [ 862.275229] INFO: task swapper/0:1 blocked for more than 120 seconds. [ 862.276636] Not tainted 4.18.0-rc7-next-20180801+ #131 [ 862.277882] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 862.279358] swapper/0 D13496 1 0 0x80000000 [ 862.280492] Call Trace: [ 862.281074] ? __schedule+0x253/0x700 [ 862.281958] schedule+0x23/0x80 [ 862.282765] async_synchronize_cookie_domain+0x89/0xd0 [ 862.283921] ? wait_woken+0x80/0x80 [ 862.284836] ? rest_init+0xb0/0xb0 [ 862.285327] kernel_init+0xa/0x100 [ 862.285520] ret_from_fork+0x35/0x40 [ 985.155031] INFO: task swapper/0:1 blocked for more than 120 seconds. [ 985.156626] Not tainted 4.18.0-rc7-next-20180801+ #131 [ 985.157699] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 985.159032] swapper/0 D13496 1 0 0x80000000 [ 985.160313] Call Trace: [ 985.160819] ? __schedule+0x253/0x700 [ 985.161303] schedule+0x23/0x80 [ 985.161480] async_synchronize_cookie_domain+0x89/0xd0 [ 985.161734] ? wait_woken+0x80/0x80 [ 985.161914] ? rest_init+0xb0/0xb0 [ 985.162080] kernel_init+0xa/0x100 [ 985.162334] ret_from_fork+0x35/0x40
On Fri, Aug 03, 2018 at 10:28:45AM +0200, Johannes Thumshirn wrote: > On Thu, Aug 02, 2018 at 10:44:42AM -0700, Bart Van Assche wrote: > > Surround scsi_execute() calls with scsi_autopm_get_device() and > > scsi_autopm_put_device(). Note: removing sr_mutex protection from > > the scsi_cd_get() and scsi_cd_put() calls is safe because the > > purpose of sr_mutex is to serialize cdrom_*() calls. > > > [...] > > Hi Bart, > > With this patch I still have the hang on next-20180801: Forget my last email, I didn't have Ming's 'blk-mq: fix blk_mq_tagset_busy_iter' applied. Tested-by: Johannes Thumshirn <jthumshirn@suse.de> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Bart, > Surround scsi_execute() calls with scsi_autopm_get_device() and > scsi_autopm_put_device(). Note: removing sr_mutex protection from > the scsi_cd_get() and scsi_cd_put() calls is safe because the > purpose of sr_mutex is to serialize cdrom_*() calls. > > This patch avoids that complaints similar to the following appear in > the kernel log if runtime power management is enabled: Applied to 4.18/scsi-fixes, thanks!
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index 3f3cb72e0c0c..d0389b20574d 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -523,18 +523,26 @@ static int sr_init_command(struct scsi_cmnd *SCpnt) static int sr_block_open(struct block_device *bdev, fmode_t mode) { struct scsi_cd *cd; + struct scsi_device *sdev; int ret = -ENXIO; + cd = scsi_cd_get(bdev->bd_disk); + if (!cd) + goto out; + + sdev = cd->device; + scsi_autopm_get_device(sdev); check_disk_change(bdev); mutex_lock(&sr_mutex); - cd = scsi_cd_get(bdev->bd_disk); - if (cd) { - ret = cdrom_open(&cd->cdi, bdev, mode); - if (ret) - scsi_cd_put(cd); - } + ret = cdrom_open(&cd->cdi, bdev, mode); mutex_unlock(&sr_mutex); + + scsi_autopm_put_device(sdev); + if (ret) + scsi_cd_put(cd); + +out: return ret; } @@ -562,6 +570,8 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd, if (ret) goto out; + scsi_autopm_get_device(sdev); + /* * Send SCSI addressing ioctls directly to mid level, send other * ioctls to cdrom/block level. @@ -570,15 +580,18 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd, case SCSI_IOCTL_GET_IDLUN: case SCSI_IOCTL_GET_BUS_NUMBER: ret = scsi_ioctl(sdev, cmd, argp); - goto out; + goto put; } ret = cdrom_ioctl(&cd->cdi, bdev, mode, cmd, arg); if (ret != -ENOSYS) - goto out; + goto put; ret = scsi_ioctl(sdev, cmd, argp); +put: + scsi_autopm_put_device(sdev); + out: mutex_unlock(&sr_mutex); return ret;
Surround scsi_execute() calls with scsi_autopm_get_device() and scsi_autopm_put_device(). Note: removing sr_mutex protection from the scsi_cd_get() and scsi_cd_put() calls is safe because the purpose of sr_mutex is to serialize cdrom_*() calls. This patch avoids that complaints similar to the following appear in the kernel log if runtime power management is enabled: INFO: task systemd-udevd:650 blocked for more than 120 seconds. Not tainted 4.18.0-rc7-dbg+ #1 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. systemd-udevd D28176 650 513 0x00000104 Call Trace: __schedule+0x444/0xfe0 schedule+0x4e/0xe0 schedule_preempt_disabled+0x18/0x30 __mutex_lock+0x41c/0xc70 mutex_lock_nested+0x1b/0x20 __blkdev_get+0x106/0x970 blkdev_get+0x22c/0x5a0 blkdev_open+0xe9/0x100 do_dentry_open.isra.19+0x33e/0x570 vfs_open+0x7c/0xd0 path_openat+0x6e3/0x1120 do_filp_open+0x11c/0x1c0 do_sys_open+0x208/0x2d0 __x64_sys_openat+0x59/0x70 do_syscall_64+0x77/0x230 entry_SYSCALL_64_after_hwframe+0x49/0xbe Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> Cc: Maurizio Lombardi <mlombard@redhat.com> Cc: Johannes Thumshirn <jthumshirn@suse.de> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: <stable@vger.kernel.org> --- drivers/scsi/sr.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-)