| Message ID | 20240827175340.GB1977952@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block: fix detection of unsupported WRITE SAME in blkdev_issue_write_zeroes | expand |
On Tue, Aug 27, 2024 at 10:53:40AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > On error, blkdev_issue_write_zeroes used to recheck the block device's > WRITE SAME queue limits after submitting WRITE SAME bios. As stated in > the comment, the purpose of this was to collapse all IO errors to > EOPNOTSUPP if the effect of issuing bios was that WRITE SAME got turned > off in the queue limits. Therefore, it does not make sense to reuse the > zeroes limit that was read earlier in the function because we only care > about the queue limit *now*, not what it was at the start of the > function. Yes, that was a bit overeager.. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On 27/08/2024 18:53, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > On error, blkdev_issue_write_zeroes used to recheck the block device's > WRITE SAME queue limits after submitting WRITE SAME bios. As stated in > the comment, the purpose of this was to collapse all IO errors to > EOPNOTSUPP if the effect of issuing bios was that WRITE SAME got turned > off in the queue limits. Therefore, it does not make sense to reuse the > zeroes limit that was read earlier in the function because we only care > about the queue limit *now*, not what it was at the start of the > function. > > Found by running generic/351 from fstests. thanks for the fix Reviewed-by: John Garry <john.g.garry@oracle.com> > > Fixes: 64b582ca88ca1 ("block: Read max write zeroes once for __blkdev_issue_write_zeroes()") > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > block/blk-lib.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/blk-lib.c b/block/blk-lib.c > index 83eb7761c2bfb..4c9f20a689f7b 100644 > --- a/block/blk-lib.c > +++ b/block/blk-lib.c > @@ -174,7 +174,7 @@ static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector, > * on an I/O error, in which case we'll turn any error into > * "not supported" here. > */ > - if (ret && !limit) I don't think that we still require local variable @limit. Actually we can probably clean this up later, and Nitesh's suggestion on the original patch could have been considered more > + if (ret && !bdev_write_zeroes_sectors(bdev)) > return -EOPNOTSUPP; > return ret; > } >
On Tue, 27 Aug 2024 10:53:40 -0700, Darrick J. Wong wrote: > On error, blkdev_issue_write_zeroes used to recheck the block device's > WRITE SAME queue limits after submitting WRITE SAME bios. As stated in > the comment, the purpose of this was to collapse all IO errors to > EOPNOTSUPP if the effect of issuing bios was that WRITE SAME got turned > off in the queue limits. Therefore, it does not make sense to reuse the > zeroes limit that was read earlier in the function because we only care > about the queue limit *now*, not what it was at the start of the > function. > > [...] Applied, thanks! [1/1] block: fix detection of unsupported WRITE SAME in blkdev_issue_write_zeroes (no commit info) Best regards,
diff --git a/block/blk-lib.c b/block/blk-lib.c index 83eb7761c2bfb..4c9f20a689f7b 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -174,7 +174,7 @@ static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector, * on an I/O error, in which case we'll turn any error into * "not supported" here. */ - if (ret && !limit) + if (ret && !bdev_write_zeroes_sectors(bdev)) return -EOPNOTSUPP; return ret; }