* [PATCH] block: fix unintended fallthrough in generic_make_request_checks()
@ 2016-12-04 13:56 Nicolai Stange
2016-12-05 12:37 ` Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Nicolai Stange @ 2016-12-04 13:56 UTC (permalink / raw)
To: Jens Axboe
Cc: Chaitanya Kulkarni, Shaun Tancheff, Damien Le Moal, linux-block,
linux-kernel, Nicolai Stange
Since commit e73c23ff736e ("block: add async variant of
blkdev_issue_zeroout") messages like the following show up:
EXT4-fs (dm-1): Delayed block allocation failed for inode 2368848 at
logical offset 0 with max blocks 1 with error 95
EXT4-fs (dm-1): This should not happen!! Data will be lost
Due to the following fallthrough introduced with
commit 2d253440b5af ("block: Define zoned block device operations"),
generic_make_request_checks() would accept a REQ_OP_WRITE_SAME bio only
if the block device supports "write same" *and* is a zoned one:
switch (bio_op(bio)) {
[...]
case REQ_OP_WRITE_SAME:
if (!bdev_write_same(bio->bi_bdev))
goto not_supported;
case REQ_OP_ZONE_REPORT:
case REQ_OP_ZONE_RESET:
if (!bdev_is_zoned(bio->bi_bdev))
goto not_supported;
break;
[...]
}
Thus, although the bio setup as done by __blkdev_issue_write_same() from
commit e73c23ff736e ("block: add async variant of blkdev_issue_zeroout")
would succeed, its actual submission would not, resulting in the
EOPNOTSUPP == 95.
Fix this by removing the fallthrough which, due to the lack of an explicit
comment, seems to be unintended anyway.
Fixes: e73c23ff736e ("block: add async variant of blkdev_issue_zeroout")
Fixes: 2d253440b5af ("block: Define zoned block device operations")
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
Applicable to next-20161202.
Note that after this patch, I'm seeing a single
EXT4-fs (dm-1): Delayed block allocation failed for inode 2625094 at
logical offset 2032 with max blocks 2 with error 121
with 121 == EREMOTEIO
This is because my SATA sda reports 0x20 (invalid command) back
in response to 0x41 (WRITE_SAME).
After this has happened, sd_done() disables "write same" once and for
all, hence only this single message.
I expect this to get fixed by
https://patchwork.kernel.org/patch/9321963/
block/blk-core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/blk-core.c b/block/blk-core.c
index 24de87d..1998aa4 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1947,6 +1947,7 @@ generic_make_request_checks(struct bio *bio)
case REQ_OP_WRITE_SAME:
if (!bdev_write_same(bio->bi_bdev))
goto not_supported;
+ break;
case REQ_OP_ZONE_REPORT:
case REQ_OP_ZONE_RESET:
if (!bdev_is_zoned(bio->bi_bdev))
--
2.10.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] block: fix unintended fallthrough in generic_make_request_checks()
2016-12-04 13:56 [PATCH] block: fix unintended fallthrough in generic_make_request_checks() Nicolai Stange
@ 2016-12-05 12:37 ` Christoph Hellwig
2016-12-05 14:55 ` Jens Axboe
2016-12-06 0:04 ` Nicolai Stange
2 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2016-12-05 12:37 UTC (permalink / raw)
To: Nicolai Stange
Cc: Jens Axboe, Chaitanya Kulkarni, Shaun Tancheff, Damien Le Moal,
linux-block, linux-kernel
Oops, thanks for the fix:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] block: fix unintended fallthrough in generic_make_request_checks()
2016-12-04 13:56 [PATCH] block: fix unintended fallthrough in generic_make_request_checks() Nicolai Stange
2016-12-05 12:37 ` Christoph Hellwig
@ 2016-12-05 14:55 ` Jens Axboe
2016-12-06 0:04 ` Nicolai Stange
2 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2016-12-05 14:55 UTC (permalink / raw)
To: Nicolai Stange
Cc: Chaitanya Kulkarni, Shaun Tancheff, Damien Le Moal, linux-block,
linux-kernel
On 12/04/2016 06:56 AM, Nicolai Stange wrote:
> Since commit e73c23ff736e ("block: add async variant of
> blkdev_issue_zeroout") messages like the following show up:
>
> EXT4-fs (dm-1): Delayed block allocation failed for inode 2368848 at
> logical offset 0 with max blocks 1 with error 95
> EXT4-fs (dm-1): This should not happen!! Data will be lost
>
> Due to the following fallthrough introduced with
> commit 2d253440b5af ("block: Define zoned block device operations"),
> generic_make_request_checks() would accept a REQ_OP_WRITE_SAME bio only
> if the block device supports "write same" *and* is a zoned one:
>
> switch (bio_op(bio)) {
> [...]
> case REQ_OP_WRITE_SAME:
> if (!bdev_write_same(bio->bi_bdev))
> goto not_supported;
> case REQ_OP_ZONE_REPORT:
> case REQ_OP_ZONE_RESET:
> if (!bdev_is_zoned(bio->bi_bdev))
> goto not_supported;
> break;
> [...]
> }
>
> Thus, although the bio setup as done by __blkdev_issue_write_same() from
> commit e73c23ff736e ("block: add async variant of blkdev_issue_zeroout")
> would succeed, its actual submission would not, resulting in the
> EOPNOTSUPP == 95.
>
> Fix this by removing the fallthrough which, due to the lack of an explicit
> comment, seems to be unintended anyway.
>
> Fixes: e73c23ff736e ("block: add async variant of blkdev_issue_zeroout")
> Fixes: 2d253440b5af ("block: Define zoned block device operations")
> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Added, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] block: fix unintended fallthrough in generic_make_request_checks()
2016-12-04 13:56 [PATCH] block: fix unintended fallthrough in generic_make_request_checks() Nicolai Stange
2016-12-05 12:37 ` Christoph Hellwig
2016-12-05 14:55 ` Jens Axboe
@ 2016-12-06 0:04 ` Nicolai Stange
2 siblings, 0 replies; 4+ messages in thread
From: Nicolai Stange @ 2016-12-06 0:04 UTC (permalink / raw)
To: Nicolai Stange
Cc: Christoph Hellwig, Jens Axboe, Chaitanya Kulkarni,
Shaun Tancheff, Damien Le Moal, linux-block, linux-kernel
Nicolai Stange <nicstange@gmail.com> writes:
> ---
> Applicable to next-20161202.
>
> Note that after this patch, I'm seeing a single
> EXT4-fs (dm-1): Delayed block allocation failed for inode 2625094 at
> logical offset 2032 with max blocks 2 with error 121
> with 121 == EREMOTEIO
>
> This is because my SATA sda reports 0x20 (invalid command) back
> in response to 0x41 (WRITE_SAME).
>
> After this has happened, sd_done() disables "write same" once and for
> all, hence only this single message.
>
> I expect this to get fixed by
> https://patchwork.kernel.org/patch/9321963/
Just FYI: Not really, the (supposedly) corresponding commit has already
been in linux-next when I was testing: commit 0ce1b18c42a5 ("libata:
Some drives failing on SCT Write Same").
I sent separate patch for this issue:
http://lkml.kernel.org/r/20161205235638.11539-1-nicstange@gmail.com
Regards,
Nicolai
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-06 0:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-04 13:56 [PATCH] block: fix unintended fallthrough in generic_make_request_checks() Nicolai Stange
2016-12-05 12:37 ` Christoph Hellwig
2016-12-05 14:55 ` Jens Axboe
2016-12-06 0:04 ` Nicolai Stange
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).