From 8fe8ffb12c81b36877984274db184953c337db73 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 20 Oct 2017 11:46:45 -0700 Subject: scsi: Suppress a kernel warning in case the prep function returns BLKPREP_DEFER The legacy block layer handles requests as follows: - If the prep function returns BLKPREP_OK, let blk_peek_request() return the pointer to that request. - If the prep function returns BLKPREP_DEFER, keep the RQF_STARTED flag and retry calling the prep function later. - If the prep function returns BLKPREP_KILL or BLKPREP_INVALID, end the request. In none of these cases it is correct to clear the SCMD_INITIALIZED flag from inside scsi_prep_fn(). Since scsi_prep_fn() already guarantees that scsi_init_command() will be called once even if scsi_prep_fn() is called multiple times, remove the code that clears SCMD_INITIALIZED from scsi_prep_fn(). The scsi-mq code handles requests as follows: - If scsi_mq_prep_fn() returns BLKPREP_OK, set the RQF_DONTPREP flag and submit the request to the SCSI LLD. - If scsi_mq_prep_fn() returns BLKPREP_DEFER, call blk_mq_delay_run_hw_queue() and return BLK_STS_RESOURCE. - If the prep function returns BLKPREP_KILL or BLKPREP_INVALID, call scsi_mq_uninit_cmd() and let the blk-mq core end the request. In none of these cases scsi_mq_prep_fn() should clear the SCMD_INITIALIZED flag. Hence remove the code from scsi_mq_prep_fn() function that clears that flag. This patch avoids that the following warning is triggered when using the legacy block layer: ------------[ cut here ]------------ WARNING: CPU: 1 PID: 4198 at drivers/scsi/scsi_lib.c:654 scsi_end_request+0x1de/0x220 CPU: 1 PID: 4198 Comm: mkfs.f2fs Not tainted 4.14.0-rc5+ #1 task: ffff91c147a4b800 task.stack: ffffb282c37b8000 RIP: 0010:scsi_end_request+0x1de/0x220 Call Trace: scsi_io_completion+0x204/0x5e0 scsi_finish_command+0xce/0xe0 scsi_softirq_done+0x126/0x130 blk_done_softirq+0x6e/0x80 __do_softirq+0xcf/0x2a8 irq_exit+0xab/0xb0 do_IRQ+0x7b/0xc0 common_interrupt+0x90/0x90 RIP: 0010:_raw_spin_unlock_irqrestore+0x9/0x10 __test_set_page_writeback+0xc7/0x2c0 __block_write_full_page+0x158/0x3b0 block_write_full_page+0xc4/0xd0 blkdev_writepage+0x13/0x20 __writepage+0x12/0x40 write_cache_pages+0x204/0x500 generic_writepages+0x48/0x70 blkdev_writepages+0x9/0x10 do_writepages+0x34/0xc0 __filemap_fdatawrite_range+0x6c/0x90 file_write_and_wait_range+0x31/0x90 blkdev_fsync+0x16/0x40 vfs_fsync_range+0x44/0xa0 do_fsync+0x38/0x60 SyS_fsync+0xb/0x10 entry_SYSCALL_64_fastpath+0x13/0x94 ---[ end trace 86e8ef85a4a6c1d1 ]--- Fixes: commit 64104f703212 ("scsi: Call scsi_initialize_rq() for filesystem requests") Signed-off-by: Bart Van Assche Cc: Damien Le Moal Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Johannes Thumshirn Reviewed-by: Damien Le Moal Reviewed-by: Johannes Thumshirn Signed-off-by: Martin K. Petersen --- drivers/scsi/scsi_lib.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/scsi/scsi_lib.c') diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 9cf6a80fe297..ad3ea24f0885 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1379,8 +1379,6 @@ static int scsi_prep_fn(struct request_queue *q, struct request *req) ret = scsi_setup_cmnd(sdev, req); out: - if (ret != BLKPREP_OK) - cmd->flags &= ~SCMD_INITIALIZED; return scsi_prep_return(q, req, ret); } @@ -1900,7 +1898,6 @@ static int scsi_mq_prep_fn(struct request *req) struct scsi_device *sdev = req->q->queuedata; struct Scsi_Host *shost = sdev->host; struct scatterlist *sg; - int ret; scsi_init_command(sdev, cmd); @@ -1934,10 +1931,7 @@ static int scsi_mq_prep_fn(struct request *req) blk_mq_start_request(req); - ret = scsi_setup_cmnd(sdev, req); - if (ret != BLK_STS_OK) - cmd->flags &= ~SCMD_INITIALIZED; - return ret; + return scsi_setup_cmnd(sdev, req); } static void scsi_mq_done(struct scsi_cmnd *cmd) -- cgit v1.2.3-59-g8ed1b From a817e73fe693f0718b6210f4b959478877fb2e2f Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Tue, 7 Nov 2017 09:04:32 -0800 Subject: Revert "scsi: make 'state' device attribute pollable" This reverts commit 8a97712e5314aefe16b3ffb4583a34deaa49de04. This commit added a call to sysfs_notify() from within scsi_device_set_state(), which in turn turns out to make libata very unhappy, because ata_eh_detach_dev() does spin_lock_irqsave(ap->lock, flags); .. if (ata_scsi_offline_dev(dev)) { dev->flags |= ATA_DFLAG_DETACHED; ap->pflags |= ATA_PFLAG_SCSI_HOTPLUG; } and ata_scsi_offline_dev() then does that scsi_device_set_state() to set it offline. So now we called sysfs_notify() from within a spinlocked region, which really doesn't work. The 0day robot reported this as: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:238 because sysfs_notify() ends up calling kernfs_find_and_get_ns() which then does mutex_lock(&kernfs_mutex).. The pollability of the device state isn't critical, so revert this all for now, and maybe we'll do it differently in the future. Reported-by: Fengguang Wu Acked-by: Tejun Heo Acked-by: Martin K. Petersen Acked-by: Hannes Reinecke Signed-off-by: Linus Torvalds --- drivers/scsi/scsi_lib.c | 3 --- drivers/scsi/scsi_transport_srp.c | 5 +---- 2 files changed, 1 insertion(+), 7 deletions(-) (limited to 'drivers/scsi/scsi_lib.c') diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index ad3ea24f0885..bcc1694cebcd 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2685,7 +2685,6 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state) } sdev->sdev_state = state; - sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state"); return 0; illegal: @@ -3109,7 +3108,6 @@ int scsi_internal_device_unblock_nowait(struct scsi_device *sdev, case SDEV_BLOCK: case SDEV_TRANSPORT_OFFLINE: sdev->sdev_state = new_state; - sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state"); break; case SDEV_CREATED_BLOCK: if (new_state == SDEV_TRANSPORT_OFFLINE || @@ -3117,7 +3115,6 @@ int scsi_internal_device_unblock_nowait(struct scsi_device *sdev, sdev->sdev_state = new_state; else sdev->sdev_state = SDEV_CREATED; - sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state"); break; case SDEV_CANCEL: case SDEV_OFFLINE: diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c index 4f6f01cf9968..36f6190931bc 100644 --- a/drivers/scsi/scsi_transport_srp.c +++ b/drivers/scsi/scsi_transport_srp.c @@ -556,11 +556,8 @@ int srp_reconnect_rport(struct srp_rport *rport) */ shost_for_each_device(sdev, shost) { mutex_lock(&sdev->state_mutex); - if (sdev->sdev_state == SDEV_OFFLINE) { + if (sdev->sdev_state == SDEV_OFFLINE) sdev->sdev_state = SDEV_RUNNING; - sysfs_notify(&sdev->sdev_gendev.kobj, - NULL, "state"); - } mutex_unlock(&sdev->state_mutex); } } else if (rport->state == SRP_RPORT_RUNNING) { -- cgit v1.2.3-59-g8ed1b