* [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER
@ 2017-10-20 18:46 Bart Van Assche
2017-10-23 5:45 ` Damien Le Moal
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Bart Van Assche @ 2017-10-20 18:46 UTC (permalink / raw)
To: Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Bart Van Assche, Damien Le Moal, Christoph Hellwig,
Hannes Reinecke, Johannes Thumshirn
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:
<IRQ>
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
</IRQ>
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 <bart.vanassche@wdc.com>
Cc: Damien Le Moal <damien.lemoal@wdc.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
drivers/scsi/scsi_lib.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 1779c8e91d09..5745af3e81bd 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1378,8 +1378,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);
}
@@ -1899,7 +1897,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);
@@ -1933,10 +1930,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)
--
2.14.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER
2017-10-20 18:46 [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER Bart Van Assche
@ 2017-10-23 5:45 ` Damien Le Moal
2017-10-23 6:00 ` Bart Van Assche
2017-10-23 8:06 ` Johannes Thumshirn
2017-10-23 8:21 ` Martin K. Petersen
2 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2017-10-23 5:45 UTC (permalink / raw)
To: Bart Van Assche, Martin K . Petersen, James E . J . Bottomley
Cc: linux-scsi, Christoph Hellwig, Hannes Reinecke, Johannes Thumshirn
On 10/20/17 20:46, Bart Van Assche wrote:
> 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:
> <IRQ>
> 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
> </IRQ>
> 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 <bart.vanassche@wdc.com>
> Cc: Damien Le Moal <damien.lemoal@wdc.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Johannes Thumshirn <jthumshirn@suse.de>
> ---
> drivers/scsi/scsi_lib.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 1779c8e91d09..5745af3e81bd 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1378,8 +1378,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);
> }
>
> @@ -1899,7 +1897,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);
>
> @@ -1933,10 +1930,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)
>
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER
2017-10-23 5:45 ` Damien Le Moal
@ 2017-10-23 6:00 ` Bart Van Assche
2017-10-23 6:15 ` Martin K. Petersen
0 siblings, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2017-10-23 6:00 UTC (permalink / raw)
To: jejb, Damien Le Moal, martin.petersen; +Cc: linux-scsi, hch, hare, jthumshirn
On Mon, 2017-10-23 at 07:45 +0200, Damien Le Moal wrote:
> On 10/20/17 20:46, Bart Van Assche wrote:
> > [ ... ]
> > This patch avoids that the following warning is triggered when using
> > the legacy block layer:
> > [ ... ]
>
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
Martin, if others agree with this patch, can it be queued for kernel v4.14?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER
2017-10-23 6:00 ` Bart Van Assche
@ 2017-10-23 6:15 ` Martin K. Petersen
0 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2017-10-23 6:15 UTC (permalink / raw)
To: Bart Van Assche
Cc: jejb, Damien Le Moal, martin.petersen, linux-scsi, hch, hare, jthumshirn
Bart,
> if others agree with this patch, can it be queued for kernel v4.14?
Yep, I'll take a look later.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER
2017-10-20 18:46 [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER Bart Van Assche
2017-10-23 5:45 ` Damien Le Moal
@ 2017-10-23 8:06 ` Johannes Thumshirn
2017-10-23 8:21 ` Martin K. Petersen
2 siblings, 0 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2017-10-23 8:06 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
Damien Le Moal, Christoph Hellwig, Hannes Reinecke
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER
2017-10-20 18:46 [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER Bart Van Assche
2017-10-23 5:45 ` Damien Le Moal
2017-10-23 8:06 ` Johannes Thumshirn
@ 2017-10-23 8:21 ` Martin K. Petersen
2 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2017-10-23 8:21 UTC (permalink / raw)
To: Bart Van Assche
Cc: Martin K . Petersen, James E . J . Bottomley, linux-scsi,
Damien Le Moal, Christoph Hellwig, Hannes Reinecke,
Johannes Thumshirn
Bart,
> This patch avoids that the following warning is triggered when using
> the legacy block layer:
Applied to 4.14/scsi-fixes. Thank you!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-23 8:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-20 18:46 [PATCH] Suppress a kernel warning in case the prep function returns BLKPREP_DEFER Bart Van Assche
2017-10-23 5:45 ` Damien Le Moal
2017-10-23 6:00 ` Bart Van Assche
2017-10-23 6:15 ` Martin K. Petersen
2017-10-23 8:06 ` Johannes Thumshirn
2017-10-23 8:21 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.