diff mbox series

[v2] block: propagate partition scanning errors to the BLKRRPART ioctl

Message ID 20240417144743.2277601-1-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [v2] block: propagate partition scanning errors to the BLKRRPART ioctl | expand

Commit Message

Christoph Hellwig April 17, 2024, 2:47 p.m. UTC
Commit 4601b4b130de ("block: reopen the device in blkdev_reread_part")
lost the propagation of I/O errors from the low-level read of the
partition table to the user space caller of the BLKRRPART.

Apparently some user space relies on, so restore the propagation.  This
isn't exactly pretty as other block device open calls explicitly do not
are about these errors, so add a new BLK_OPEN_STRICT_SCAN to opt into
the error propagation.

Fixes: 4601b4b130de ("block: reopen the device in blkdev_reread_part")
Reported-by: Saranya Muruganandam <saranyamohan@google.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---

Changes since v1:
 - fix the rebase error that causes the flag to reuse a bit number
 - fixed a comment typo

 block/bdev.c           | 29 +++++++++++++++++++----------
 block/ioctl.c          |  3 ++-
 include/linux/blkdev.h |  2 ++
 3 files changed, 23 insertions(+), 11 deletions(-)

Comments

Shin'ichiro Kawasaki April 18, 2024, 2:10 a.m. UTC | #1
On Apr 17, 2024 / 16:47, Christoph Hellwig wrote:
> Commit 4601b4b130de ("block: reopen the device in blkdev_reread_part")
> lost the propagation of I/O errors from the low-level read of the
> partition table to the user space caller of the BLKRRPART.
> 
> Apparently some user space relies on, so restore the propagation.  This
> isn't exactly pretty as other block device open calls explicitly do not
> are about these errors, so add a new BLK_OPEN_STRICT_SCAN to opt into
> the error propagation.
> 
> Fixes: 4601b4b130de ("block: reopen the device in blkdev_reread_part")
> Reported-by: Saranya Muruganandam <saranyamohan@google.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> 
> Changes since v1:
>  - fix the rebase error that causes the flag to reuse a bit number
>  - fixed a comment typo

Thanks. The v2 patch looks good to me. I also confirmed that the corresponding
blktests test case block/036 passes with this patch.

Reviewed-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Chaitanya Kulkarni April 18, 2024, 3:09 a.m. UTC | #2
On 4/17/24 19:10, Shinichiro Kawasaki wrote:
> On Apr 17, 2024 / 16:47, Christoph Hellwig wrote:
>> Commit 4601b4b130de ("block: reopen the device in blkdev_reread_part")
>> lost the propagation of I/O errors from the low-level read of the
>> partition table to the user space caller of the BLKRRPART.
>>
>> Apparently some user space relies on, so restore the propagation.  This
>> isn't exactly pretty as other block device open calls explicitly do not
>> are about these errors, so add a new BLK_OPEN_STRICT_SCAN to opt into
>> the error propagation.
>>
>> Fixes: 4601b4b130de ("block: reopen the device in blkdev_reread_part")
>> Reported-by: Saranya Muruganandam <saranyamohan@google.com>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>
>> Changes since v1:
>>   - fix the rebase error that causes the flag to reuse a bit number
>>   - fixed a comment typo
> Thanks. The v2 patch looks good to me. I also confirmed that the corresponding
> blktests test case block/036 passes with this patch.
>
> Reviewed-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

seems like my reviewed-by tag from V1 is missing ...

here you go again

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck
Jens Axboe April 18, 2024, 3:34 p.m. UTC | #3
On Wed, 17 Apr 2024 16:47:43 +0200, Christoph Hellwig wrote:
> Commit 4601b4b130de ("block: reopen the device in blkdev_reread_part")
> lost the propagation of I/O errors from the low-level read of the
> partition table to the user space caller of the BLKRRPART.
> 
> Apparently some user space relies on, so restore the propagation.  This
> isn't exactly pretty as other block device open calls explicitly do not
> are about these errors, so add a new BLK_OPEN_STRICT_SCAN to opt into
> the error propagation.
> 
> [...]

Applied, thanks!

[1/1] block: propagate partition scanning errors to the BLKRRPART ioctl
      commit: 752863bddacab6b5c5164b1df8c8b2e3a175ee28

Best regards,
diff mbox series

Patch

diff --git a/block/bdev.c b/block/bdev.c
index 7a5f611c3d2e3e..cea51dca875315 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);
 	atomic_inc(&bdev->bd_openers);
+	if (test_bit(GD_NEED_PART_SCAN, &disk->state)) {
+		/*
+		 * Only return scanning errors if we are called from 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 0c76137adcaaa5..128f503828cee7 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);
 	case BLKTRACESTART:
 	case BLKTRACESTOP:
 	case BLKTRACETEARDOWN:
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c3e8f7cf96be9e..d16320852c4ba2 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 << 6))
 
 struct gendisk {
 	/*