Saranya, while I evaluate the corresponding patch for blktests, I found a weird behavior, and had a chance to look into this patch. Please find my comments in line.
On Apr 06, 2024 / 04:46, Saranya Muruganandam wrote:
The BLKRRPART ioctl used to report errors such as EIO before we changed the blkdev_reread_part() logic.
Lets add a flag and capture the errors returned by bdev_disk_changed() when the flag is set. Setting this flag for the BLKRRPART path when we want the errors to be reported when rereading partitions on the disk.
Documentation/process/submitting-patches.rst guides to describe changes in imperative mood, then the sentences above can be rephrased like,
Add a flag... Set this flag...
Link: https://lore.kernel.org/all/20240320015134.GA14267@lst.de/ Suggested-by: Christoph Hellwig hch@lst.de Tested: Tested by simulating failure to the block device and will propose a new test to blktests. Fixes: 4601b4b130de ("block: reopen the device in blkdev_reread_part") Reported-by: Saranya Muruganandam saranyamohan@google.com Signed-off-by: Saranya Muruganandam saranyamohan@google.com
Change-Id: Idf3d97390ed78061556f8468d10d6cab24ae20b1
block/bdev.c | 29 +++++++++++++++++++---------- block/ioctl.c | 3 ++- include/linux/blkdev.h | 2 ++ 3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/block/bdev.c b/block/bdev.c index 7a5f611c3d2e3..42940bced33bb 100644 --- a/block/bdev.c +++ b/block/bdev.c @@ -652,6 +652,14 @@ static void blkdev_flush_mapping(struct block_device *bdev) bdev_write_inode(bdev); } +static void blkdev_put_whole(struct block_device *bdev) +{
- if (atomic_dec_and_test(&bdev->bd_openers))
blkdev_flush_mapping(bdev);- if (bdev->bd_disk->fops->release)
bdev->bd_disk->fops->release(bdev->bd_disk);+}
static int blkdev_get_whole(struct block_device *bdev, blk_mode_t mode) { struct gendisk *disk = bdev->bd_disk; @@ -670,20 +678,21 @@ static int blkdev_get_whole(struct block_device *bdev, blk_mode_t mode) if (!atomic_read(&bdev->bd_openers)) set_init_blocksize(bdev);
- if (test_bit(GD_NEED_PART_SCAN, &disk->state))
bdev_disk_changed(disk, false);
- if (test_bit(GD_NEED_PART_SCAN, &disk->state)) {
/** Only return scanning errors if we are called from conexts
s/conexts/contexts/
* that explicitly want them, e.g. the BLKRRPART ioctl.*/ret = bdev_disk_changed(disk, false);if (ret && (mode & BLK_OPEN_STRICT_SCAN)) {blkdev_put_whole(bdev);return ret;}- } return 0;
} -static void blkdev_put_whole(struct block_device *bdev) -{
- if (atomic_dec_and_test(&bdev->bd_openers))
blkdev_flush_mapping(bdev);- if (bdev->bd_disk->fops->release)
bdev->bd_disk->fops->release(bdev->bd_disk);-}
static int blkdev_get_part(struct block_device *part, blk_mode_t mode) { struct gendisk *disk = part->bd_disk; diff --git a/block/ioctl.c b/block/ioctl.c index 0c76137adcaaa..128f503828cee 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -562,7 +562,8 @@ static int blkdev_common_ioctl(struct block_device *bdev, blk_mode_t mode, return -EACCES; if (bdev_is_partition(bdev)) return -EINVAL;
return disk_scan_partitions(bdev->bd_disk, mode);
return disk_scan_partitions(bdev->bd_disk,mode | BLK_OPEN_STRICT_SCAN);diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index c3e8f7cf96be9..3ed5e03109c29 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -128,6 +128,8 @@ typedef unsigned int __bitwise blk_mode_t; #define BLK_OPEN_WRITE_IOCTL ((__force blk_mode_t)(1 << 4)) /* open is exclusive wrt all other BLK_OPEN_WRITE opens to the device */ #define BLK_OPEN_RESTRICT_WRITES ((__force blk_mode_t)(1 << 5)) +/* return partition scanning errors */ +#define BLK_OPEN_STRICT_SCAN ((__force blk_mode_t)(1 << 5))
The line above assigns the same number for BLK_OPEN_STRICT_SCAN and BLK_OPEN_RESTRICT_WRITES, then blockdev --rereadpt fails with EINVAL, not EIO. I modified it to "1 << 6", then it looks working good.
struct gendisk { /* -- 2.44.0.478.gd926399ef9-goog