Commit 64b582ca authored by John Garry's avatar John Garry Committed by Jens Axboe

block: Read max write zeroes once for __blkdev_issue_write_zeroes()

As reported in [0], we may get a hang when formatting a XFS FS on a RAID0
drive.

Commit 73a768d5 ("block: factor out a blk_write_zeroes_limit helper")
changed __blkdev_issue_write_zeroes() to read the max write zeroes
value in the loop. This is not safe as max write zeroes may change in
value. Specifically for the case of [0], the value goes to 0, and we get
an infinite loop.

Lift the limit reading out of the loop.

[0] https://lore.kernel.org/linux-xfs/4d31268f-310b-4220-88a2-e191c3932a82@oracle.com/T/#t

Fixes: 73a768d5 ("block: factor out a blk_write_zeroes_limit helper")
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarJohn Garry <john.g.garry@oracle.com>
Reviewed-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
Link: https://lore.kernel.org/r/20240815163228.216051-2-john.g.garry@oracle.comSigned-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent b313a8c8
...@@ -111,13 +111,20 @@ static sector_t bio_write_zeroes_limit(struct block_device *bdev) ...@@ -111,13 +111,20 @@ static sector_t bio_write_zeroes_limit(struct block_device *bdev)
(UINT_MAX >> SECTOR_SHIFT) & ~bs_mask); (UINT_MAX >> SECTOR_SHIFT) & ~bs_mask);
} }
/*
* There is no reliable way for the SCSI subsystem to determine whether a
* device supports a WRITE SAME operation without actually performing a write
* to media. As a result, write_zeroes is enabled by default and will be
* disabled if a zeroing operation subsequently fails. This means that this
* queue limit is likely to change at runtime.
*/
static void __blkdev_issue_write_zeroes(struct block_device *bdev, static void __blkdev_issue_write_zeroes(struct block_device *bdev,
sector_t sector, sector_t nr_sects, gfp_t gfp_mask, sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
struct bio **biop, unsigned flags) struct bio **biop, unsigned flags, sector_t limit)
{ {
while (nr_sects) { while (nr_sects) {
unsigned int len = min_t(sector_t, nr_sects, unsigned int len = min(nr_sects, limit);
bio_write_zeroes_limit(bdev));
struct bio *bio; struct bio *bio;
if ((flags & BLKDEV_ZERO_KILLABLE) && if ((flags & BLKDEV_ZERO_KILLABLE) &&
...@@ -141,12 +148,14 @@ static void __blkdev_issue_write_zeroes(struct block_device *bdev, ...@@ -141,12 +148,14 @@ static void __blkdev_issue_write_zeroes(struct block_device *bdev,
static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector, static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp, unsigned flags) sector_t nr_sects, gfp_t gfp, unsigned flags)
{ {
sector_t limit = bio_write_zeroes_limit(bdev);
struct bio *bio = NULL; struct bio *bio = NULL;
struct blk_plug plug; struct blk_plug plug;
int ret = 0; int ret = 0;
blk_start_plug(&plug); blk_start_plug(&plug);
__blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp, &bio, flags); __blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp, &bio,
flags, limit);
if (bio) { if (bio) {
if ((flags & BLKDEV_ZERO_KILLABLE) && if ((flags & BLKDEV_ZERO_KILLABLE) &&
fatal_signal_pending(current)) { fatal_signal_pending(current)) {
...@@ -165,7 +174,7 @@ static int blkdev_issue_write_zeroes(struct block_device *bdev, sector_t sector, ...@@ -165,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 * on an I/O error, in which case we'll turn any error into
* "not supported" here. * "not supported" here.
*/ */
if (ret && !bdev_write_zeroes_sectors(bdev)) if (ret && !limit)
return -EOPNOTSUPP; return -EOPNOTSUPP;
return ret; return ret;
} }
...@@ -265,12 +274,14 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector, ...@@ -265,12 +274,14 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, struct bio **biop, sector_t nr_sects, gfp_t gfp_mask, struct bio **biop,
unsigned flags) unsigned flags)
{ {
sector_t limit = bio_write_zeroes_limit(bdev);
if (bdev_read_only(bdev)) if (bdev_read_only(bdev))
return -EPERM; return -EPERM;
if (bdev_write_zeroes_sectors(bdev)) { if (limit) {
__blkdev_issue_write_zeroes(bdev, sector, nr_sects, __blkdev_issue_write_zeroes(bdev, sector, nr_sects,
gfp_mask, biop, flags); gfp_mask, biop, flags, limit);
} else { } else {
if (flags & BLKDEV_ZERO_NOFALLBACK) if (flags & BLKDEV_ZERO_NOFALLBACK)
return -EOPNOTSUPP; return -EOPNOTSUPP;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment