Re: [PATCH] block: Do not reread partition table on exclusively open device

[Yu Kuai <yukuai1@xxxxxxxxxxxxxxx>]

Hi, Jan

在 2022/12/01 1:56, Jan Kara 写道:

> -int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
> +int disk_scan_partitions(struct gendisk *disk, fmode_t mode, void *owner)
>   {
>   	struct block_device *bdev;
>   
> @@ -366,6 +366,9 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
>   		return -EINVAL;
>   	if (disk->open_partitions)
>   		return -EBUSY;
> +	/* Someone else has bdev exclusively open? */
> +	if (disk->part0->bd_holder != owner)
> +		return -EBUSY;
>   
>   	set_bit(GD_NEED_PART_SCAN, &disk->state);
>   	bdev = blkdev_get_by_dev(disk_devt(disk), mode, NULL);

It just by code review, but I think the above checking should be inside
open_mutex, which is used to protect bdev claming. Otherwise there can
be race that this check is pass while someone else exclusive open the
disk before blkdev_get_by_dev().

How do you think about open coding blkdev_get_by_dev() here, something
like:

diff --git a/block/genhd.c b/block/genhd.c
index 23cf83b3331c..341af4db7d54 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -358,7 +358,7 @@ EXPORT_SYMBOL_GPL(disk_uevent);

 int disk_scan_partitions(struct gendisk *disk, fmode_t mode, void *owner)
 {
-       struct block_device *bdev;
+       int ret = 0;

        if (disk->flags & (GENHD_FL_NO_PART | GENHD_FL_HIDDEN))
                return -EINVAL;
@@ -366,16 +366,31 @@ int disk_scan_partitions(struct gendisk *disk, 
fmode_t mode, void *owner)
                return -EINVAL;
        if (disk->open_partitions)
                return -EBUSY;
+
+       disk_block_events(disk);
+       mutex_lock(&disk->open_mutex);
+
        /* Someone else has bdev exclusively open? */
-       if (disk->part0->bd_holder && disk->part0->bd_holder != owner)
-               return -EBUSY;
+       if (disk->part0->bd_holder && disk->part0->bd_holder != owner) {
+               ret = -EBUSY;
+               goto out;
+       }
+
+       if (!disk_live(disk)) {
+               ret = -ENODEV;
+               goto out;
+       }

        set_bit(GD_NEED_PART_SCAN, &disk->state);
-       bdev = blkdev_get_by_dev(disk_devt(disk), mode, NULL);
-       if (IS_ERR(bdev))
-               return PTR_ERR(bdev);
-       blkdev_put(bdev, mode);
-       return 0;
+       ret = blkdev_get_whole(disk->part0, mode);
+out:
+       mutex_unlock(&disk->open_mutex);
+       disk_unblock_events(disk);
+
+       if (!ret)
+               blkdev_put_whole(disk->part0, mode);
+
+       return ret;
 }