linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -next RFC 0/3] block: fix scan partition for exclusively open device again
@ 2023-02-12  9:26 Yu Kuai
  2023-02-12  9:26 ` [PATCH -next RFC 1/3] block: Revert "block: Do not reread partition table on exclusively open device" Yu Kuai
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Yu Kuai @ 2023-02-12  9:26 UTC (permalink / raw)
  To: hch, jack, axboe
  Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun

From: Yu Kuai <yukuai3@huawei.com>

Yu Kuai (3):
  block: Revert "block: Do not reread partition table on exclusively
    open device"
  block: factor out the setting of GD_NEED_PART_SCAN
  block: fix scan partition for exclusively open device again

 block/blk.h   |  2 +-
 block/genhd.c | 36 ++++++++++++++++++++++++++----------
 block/ioctl.c | 14 +++++++-------
 3 files changed, 34 insertions(+), 18 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH -next RFC 1/3] block: Revert "block: Do not reread partition table on exclusively open device"
  2023-02-12  9:26 [PATCH -next RFC 0/3] block: fix scan partition for exclusively open device again Yu Kuai
@ 2023-02-12  9:26 ` Yu Kuai
  2023-02-12  9:26 ` [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN Yu Kuai
  2023-02-12  9:26 ` [PATCH -next RFC 3/3] block: fix scan partition for exclusively open device again Yu Kuai
  2 siblings, 0 replies; 9+ messages in thread
From: Yu Kuai @ 2023-02-12  9:26 UTC (permalink / raw)
  To: hch, jack, axboe
  Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun

From: Yu Kuai <yukuai3@huawei.com>

This reverts commit 36369f46e91785688a5f39d7a5590e3f07981316.

This patch can't fix the problem in a corner case that device can be
opened exclusively after the checking and before blkdev_get_by_dev().
We'll use a new solution to fix the problem in following patches, and
the new solution doesn't need to change apis.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/blk.h   |  2 +-
 block/genhd.c |  7 ++-----
 block/ioctl.c | 13 ++++++-------
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index 8900001946c7..a8ac9803fcb3 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -426,7 +426,7 @@ int bio_add_hw_page(struct request_queue *q, struct bio *bio,
 
 struct request_queue *blk_alloc_queue(int node_id);
 
-int disk_scan_partitions(struct gendisk *disk, fmode_t mode, void *owner);
+int disk_scan_partitions(struct gendisk *disk, fmode_t mode);
 
 int disk_alloc_events(struct gendisk *disk);
 void disk_add_events(struct gendisk *disk);
diff --git a/block/genhd.c b/block/genhd.c
index 52d71a94a809..075d8da284f5 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -356,7 +356,7 @@ void disk_uevent(struct gendisk *disk, enum kobject_action action)
 }
 EXPORT_SYMBOL_GPL(disk_uevent);
 
-int disk_scan_partitions(struct gendisk *disk, fmode_t mode, void *owner)
+int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
 {
 	struct block_device *bdev;
 
@@ -366,9 +366,6 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode, void *owner)
 		return -EINVAL;
 	if (disk->open_partitions)
 		return -EBUSY;
-	/* Someone else has bdev exclusively open? */
-	if (disk->part0->bd_holder && 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);
@@ -498,7 +495,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
 
 		bdev_add(disk->part0, ddev->devt);
 		if (get_capacity(disk))
-			disk_scan_partitions(disk, FMODE_READ, NULL);
+			disk_scan_partitions(disk, FMODE_READ);
 
 		/*
 		 * Announce the disk and partitions after all partitions are
diff --git a/block/ioctl.c b/block/ioctl.c
index 96617512982e..6dd49d877584 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -467,10 +467,10 @@ static int blkdev_bszset(struct block_device *bdev, fmode_t mode,
  * user space. Note the separate arg/argp parameters that are needed
  * to deal with the compat_ptr() conversion.
  */
-static int blkdev_common_ioctl(struct file *file, fmode_t mode, unsigned cmd,
-			       unsigned long arg, void __user *argp)
+static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
+			       unsigned int cmd, unsigned long arg,
+			       void __user *argp)
 {
-	struct block_device *bdev = I_BDEV(file->f_mapping->host);
 	unsigned int max_sectors;
 
 	switch (cmd) {
@@ -528,8 +528,7 @@ static int blkdev_common_ioctl(struct file *file, fmode_t mode, unsigned cmd,
 			return -EACCES;
 		if (bdev_is_partition(bdev))
 			return -EINVAL;
-		return disk_scan_partitions(bdev->bd_disk, mode & ~FMODE_EXCL,
-					    file);
+		return disk_scan_partitions(bdev->bd_disk, mode & ~FMODE_EXCL);
 	case BLKTRACESTART:
 	case BLKTRACESTOP:
 	case BLKTRACETEARDOWN:
@@ -607,7 +606,7 @@ long blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 		break;
 	}
 
-	ret = blkdev_common_ioctl(file, mode, cmd, arg, argp);
+	ret = blkdev_common_ioctl(bdev, mode, cmd, arg, argp);
 	if (ret != -ENOIOCTLCMD)
 		return ret;
 
@@ -676,7 +675,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 		break;
 	}
 
-	ret = blkdev_common_ioctl(file, mode, cmd, arg, argp);
+	ret = blkdev_common_ioctl(bdev, mode, cmd, arg, argp);
 	if (ret == -ENOIOCTLCMD && disk->fops->compat_ioctl)
 		ret = disk->fops->compat_ioctl(bdev, mode, cmd, arg);
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN
  2023-02-12  9:26 [PATCH -next RFC 0/3] block: fix scan partition for exclusively open device again Yu Kuai
  2023-02-12  9:26 ` [PATCH -next RFC 1/3] block: Revert "block: Do not reread partition table on exclusively open device" Yu Kuai
@ 2023-02-12  9:26 ` Yu Kuai
  2023-02-12 11:36   ` Hannes Reinecke
  2023-02-16 13:17   ` Jan Kara
  2023-02-12  9:26 ` [PATCH -next RFC 3/3] block: fix scan partition for exclusively open device again Yu Kuai
  2 siblings, 2 replies; 9+ messages in thread
From: Yu Kuai @ 2023-02-12  9:26 UTC (permalink / raw)
  To: hch, jack, axboe
  Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun

From: Yu Kuai <yukuai3@huawei.com>

In order to prevent scan partition for a device that is opened
exclusively by someone else, new conditions will be added to
disk_scan_partitions() in the next patch. Hence if device is opened
exclusively between bdev_add() and disk_scan_partitions(), the first
partition scan will fail unexpected. This patch factor out the setting
of GD_NEED_PART_SCAN to prevent the problem.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/genhd.c | 2 +-
 block/ioctl.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/genhd.c b/block/genhd.c
index 075d8da284f5..c0d1220bd798 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -367,7 +367,6 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
 	if (disk->open_partitions)
 		return -EBUSY;
 
-	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);
@@ -493,6 +492,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
 		if (ret)
 			goto out_unregister_bdi;
 
+		set_bit(GD_NEED_PART_SCAN, &disk->state);
 		bdev_add(disk->part0, ddev->devt);
 		if (get_capacity(disk))
 			disk_scan_partitions(disk, FMODE_READ);
diff --git a/block/ioctl.c b/block/ioctl.c
index 6dd49d877584..0eefcdb936a0 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -528,6 +528,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
 			return -EACCES;
 		if (bdev_is_partition(bdev))
 			return -EINVAL;
+		set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
 		return disk_scan_partitions(bdev->bd_disk, mode & ~FMODE_EXCL);
 	case BLKTRACESTART:
 	case BLKTRACESTOP:
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH -next RFC 3/3] block: fix scan partition for exclusively open device again
  2023-02-12  9:26 [PATCH -next RFC 0/3] block: fix scan partition for exclusively open device again Yu Kuai
  2023-02-12  9:26 ` [PATCH -next RFC 1/3] block: Revert "block: Do not reread partition table on exclusively open device" Yu Kuai
  2023-02-12  9:26 ` [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN Yu Kuai
@ 2023-02-12  9:26 ` Yu Kuai
  2023-02-16 13:19   ` Jan Kara
  2 siblings, 1 reply; 9+ messages in thread
From: Yu Kuai @ 2023-02-12  9:26 UTC (permalink / raw)
  To: hch, jack, axboe
  Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun

From: Yu Kuai <yukuai3@huawei.com>

As explained in commit 36369f46e917 ("block: Do not reread partition table
on exclusively open device"), reread partition on the device that is
exclusively opened by someone else is problematic.

This patch will make sure partition scan will only be proceed if current
thread open the device exclusively, or the device is not opened
exclusively, and in the later case, other scanners and exclusive openers
will be blocked temporarily until partition scan is done.

Fixes: 10c70d95c0f2 ("block: remove the bd_openers checks in blk_drop_partitions")
Cc: <stable@vger.kernel.org>
Suggested-by: Jan Kara <jack@suse.cz>
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/genhd.c | 27 +++++++++++++++++++++++----
 block/ioctl.c |  2 +-
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index c0d1220bd798..6ec10ffeb9cc 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -359,6 +359,7 @@ EXPORT_SYMBOL_GPL(disk_uevent);
 int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
 {
 	struct block_device *bdev;
+	int ret = 0;
 
 	if (disk->flags & (GENHD_FL_NO_PART | GENHD_FL_HIDDEN))
 		return -EINVAL;
@@ -367,11 +368,29 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
 	if (disk->open_partitions)
 		return -EBUSY;
 
-	bdev = blkdev_get_by_dev(disk_devt(disk), mode, NULL);
-	if (IS_ERR(bdev))
-		return PTR_ERR(bdev);
+	/*
+	 * If the device is opened exclusively by current thread already, it's
+	 * safe to scan partitons, otherwise, use bd_prepare_to_claim() to
+	 * synchronize with other exclusivet openers and other partition
+	 * scanners.
+	 */
+	if (!(mode & FMODE_EXCL)) {
+		ret = bd_prepare_to_claim(disk->part0, disk_scan_partitions);
+		if (ret)
+			return ret;
+	}
+
+	bdev = blkdev_get_by_dev(disk_devt(disk), mode & ~FMODE_EXCL, NULL);
+	if (IS_ERR(bdev)) {
+		ret =  PTR_ERR(bdev);
+		goto out;
+	}
 	blkdev_put(bdev, mode);
-	return 0;
+
+out:
+	if (!(mode & FMODE_EXCL))
+		bd_abort_claiming(disk->part0, disk_scan_partitions);
+	return ret;
 }
 
 /**
diff --git a/block/ioctl.c b/block/ioctl.c
index 0eefcdb936a0..3adfdb904dd0 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -529,7 +529,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
 		if (bdev_is_partition(bdev))
 			return -EINVAL;
 		set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
-		return disk_scan_partitions(bdev->bd_disk, mode & ~FMODE_EXCL);
+		return disk_scan_partitions(bdev->bd_disk, mode);
 	case BLKTRACESTART:
 	case BLKTRACESTOP:
 	case BLKTRACETEARDOWN:
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN
  2023-02-12  9:26 ` [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN Yu Kuai
@ 2023-02-12 11:36   ` Hannes Reinecke
  2023-02-13  2:41     ` Yu Kuai
  2023-02-16 13:17   ` Jan Kara
  1 sibling, 1 reply; 9+ messages in thread
From: Hannes Reinecke @ 2023-02-12 11:36 UTC (permalink / raw)
  To: Yu Kuai, hch, jack, axboe
  Cc: linux-block, linux-kernel, yukuai3, yi.zhang, yangerkun

On 2/12/23 10:26, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> In order to prevent scan partition for a device that is opened
> exclusively by someone else, new conditions will be added to
> disk_scan_partitions() in the next patch. Hence if device is opened
> exclusively between bdev_add() and disk_scan_partitions(), the first
> partition scan will fail unexpected. This patch factor out the setting
> of GD_NEED_PART_SCAN to prevent the problem.
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>   block/genhd.c | 2 +-
>   block/ioctl.c | 1 +
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 075d8da284f5..c0d1220bd798 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -367,7 +367,6 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
>   	if (disk->open_partitions)
>   		return -EBUSY;
>   
> -	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);
> @@ -493,6 +492,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
>   		if (ret)
>   			goto out_unregister_bdi;
>   
> +		set_bit(GD_NEED_PART_SCAN, &disk->state);
>   		bdev_add(disk->part0, ddev->devt);
>   		if (get_capacity(disk))
>   			disk_scan_partitions(disk, FMODE_READ);
Usual caveat:
What happens if the flag is already set here?
Wouldn't that imply that another scan is underway?
And wouldn't it be better to use 'test_and_set()'?


> diff --git a/block/ioctl.c b/block/ioctl.c
> index 6dd49d877584..0eefcdb936a0 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -528,6 +528,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
>   			return -EACCES;
>   		if (bdev_is_partition(bdev))
>   			return -EINVAL;
> +		set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
>   		return disk_scan_partitions(bdev->bd_disk, mode & ~FMODE_EXCL);
>   	case BLKTRACESTART:
>   	case BLKTRACESTOP:
Similar here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN
  2023-02-12 11:36   ` Hannes Reinecke
@ 2023-02-13  2:41     ` Yu Kuai
  0 siblings, 0 replies; 9+ messages in thread
From: Yu Kuai @ 2023-02-13  2:41 UTC (permalink / raw)
  To: Hannes Reinecke, Yu Kuai, hch, jack, axboe
  Cc: linux-block, linux-kernel, yi.zhang, yangerkun, yukuai (C)

Hi,

在 2023/02/12 19:36, Hannes Reinecke 写道:
> On 2/12/23 10:26, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> In order to prevent scan partition for a device that is opened
>> exclusively by someone else, new conditions will be added to
>> disk_scan_partitions() in the next patch. Hence if device is opened
>> exclusively between bdev_add() and disk_scan_partitions(), the first
>> partition scan will fail unexpected. This patch factor out the setting
>> of GD_NEED_PART_SCAN to prevent the problem.
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>>   block/genhd.c | 2 +-
>>   block/ioctl.c | 1 +
>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/genhd.c b/block/genhd.c
>> index 075d8da284f5..c0d1220bd798 100644
>> --- a/block/genhd.c
>> +++ b/block/genhd.c
>> @@ -367,7 +367,6 @@ int disk_scan_partitions(struct gendisk *disk, 
>> fmode_t mode)
>>       if (disk->open_partitions)
>>           return -EBUSY;
>> -    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);
>> @@ -493,6 +492,7 @@ int __must_check device_add_disk(struct device 
>> *parent, struct gendisk *disk,
>>           if (ret)
>>               goto out_unregister_bdi;
>> +        set_bit(GD_NEED_PART_SCAN, &disk->state);
>>           bdev_add(disk->part0, ddev->devt);
>>           if (get_capacity(disk))
>>               disk_scan_partitions(disk, FMODE_READ);
> Usual caveat:
> What happens if the flag is already set here?
> Wouldn't that imply that another scan is underway?

I think this is not true, currently set the state just means we're going
to scan, and if some checking failed, the state will be left. I don't
have a good idea how to fix this yet...

Thanks,
Kuai

> And wouldn't it be better to use 'test_and_set()'?
> 
> 
>> diff --git a/block/ioctl.c b/block/ioctl.c
>> index 6dd49d877584..0eefcdb936a0 100644
>> --- a/block/ioctl.c
>> +++ b/block/ioctl.c
>> @@ -528,6 +528,7 @@ static int blkdev_common_ioctl(struct block_device 
>> *bdev, fmode_t mode,
>>               return -EACCES;
>>           if (bdev_is_partition(bdev))
>>               return -EINVAL;
>> +        set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
>>           return disk_scan_partitions(bdev->bd_disk, mode & ~FMODE_EXCL);
>>       case BLKTRACESTART:
>>       case BLKTRACESTOP:
> Similar here.
> 
> Cheers,
> 
> Hannes


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN
  2023-02-12  9:26 ` [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN Yu Kuai
  2023-02-12 11:36   ` Hannes Reinecke
@ 2023-02-16 13:17   ` Jan Kara
  2023-02-17  1:44     ` Yu Kuai
  1 sibling, 1 reply; 9+ messages in thread
From: Jan Kara @ 2023-02-16 13:17 UTC (permalink / raw)
  To: Yu Kuai
  Cc: hch, jack, axboe, linux-block, linux-kernel, yukuai3, yi.zhang,
	yangerkun

On Sun 12-02-23 17:26:40, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> In order to prevent scan partition for a device that is opened
> exclusively by someone else, new conditions will be added to
> disk_scan_partitions() in the next patch. Hence if device is opened
> exclusively between bdev_add() and disk_scan_partitions(), the first
> partition scan will fail unexpected. This patch factor out the setting
> of GD_NEED_PART_SCAN to prevent the problem.
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>

I'd rather leave the setting of GD_NEED_PART_SCAN in disk_scan_partitions()
to keep it self-contained. On top of that we can set GD_NEED_PART_SCAN in
device_add_disk() to ensure initial partition scan but we should probably
also make sure flags like GD_SUPPRESS_PART_SCAN or GENHD_FL_NO_PART are not
set to avoid unwanted partition scanning.

								Honza

> ---
>  block/genhd.c | 2 +-
>  block/ioctl.c | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 075d8da284f5..c0d1220bd798 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -367,7 +367,6 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
>  	if (disk->open_partitions)
>  		return -EBUSY;
>  
> -	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);
> @@ -493,6 +492,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
>  		if (ret)
>  			goto out_unregister_bdi;
>  
> +		set_bit(GD_NEED_PART_SCAN, &disk->state);
>  		bdev_add(disk->part0, ddev->devt);
>  		if (get_capacity(disk))
>  			disk_scan_partitions(disk, FMODE_READ);
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 6dd49d877584..0eefcdb936a0 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -528,6 +528,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
>  			return -EACCES;
>  		if (bdev_is_partition(bdev))
>  			return -EINVAL;
> +		set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
>  		return disk_scan_partitions(bdev->bd_disk, mode & ~FMODE_EXCL);
>  	case BLKTRACESTART:
>  	case BLKTRACESTOP:
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next RFC 3/3] block: fix scan partition for exclusively open device again
  2023-02-12  9:26 ` [PATCH -next RFC 3/3] block: fix scan partition for exclusively open device again Yu Kuai
@ 2023-02-16 13:19   ` Jan Kara
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Kara @ 2023-02-16 13:19 UTC (permalink / raw)
  To: Yu Kuai
  Cc: hch, jack, axboe, linux-block, linux-kernel, yukuai3, yi.zhang,
	yangerkun

On Sun 12-02-23 17:26:41, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> As explained in commit 36369f46e917 ("block: Do not reread partition table
> on exclusively open device"), reread partition on the device that is
> exclusively opened by someone else is problematic.
> 
> This patch will make sure partition scan will only be proceed if current
> thread open the device exclusively, or the device is not opened
> exclusively, and in the later case, other scanners and exclusive openers
> will be blocked temporarily until partition scan is done.
> 
> Fixes: 10c70d95c0f2 ("block: remove the bd_openers checks in blk_drop_partitions")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>

Just one nit below.

> diff --git a/block/genhd.c b/block/genhd.c
> index c0d1220bd798..6ec10ffeb9cc 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -359,6 +359,7 @@ EXPORT_SYMBOL_GPL(disk_uevent);
>  int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
>  {
>  	struct block_device *bdev;
> +	int ret = 0;
>  
>  	if (disk->flags & (GENHD_FL_NO_PART | GENHD_FL_HIDDEN))
>  		return -EINVAL;
> @@ -367,11 +368,29 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
>  	if (disk->open_partitions)
>  		return -EBUSY;
>  
> -	bdev = blkdev_get_by_dev(disk_devt(disk), mode, NULL);
> -	if (IS_ERR(bdev))
> -		return PTR_ERR(bdev);
> +	/*
> +	 * If the device is opened exclusively by current thread already, it's
> +	 * safe to scan partitons, otherwise, use bd_prepare_to_claim() to
> +	 * synchronize with other exclusivet openers and other partition
				  ^^^ typo here

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN
  2023-02-16 13:17   ` Jan Kara
@ 2023-02-17  1:44     ` Yu Kuai
  0 siblings, 0 replies; 9+ messages in thread
From: Yu Kuai @ 2023-02-17  1:44 UTC (permalink / raw)
  To: Jan Kara, Yu Kuai
  Cc: hch, axboe, linux-block, linux-kernel, yi.zhang, yangerkun, yukuai (C)

Hi, jan!

在 2023/02/16 21:17, Jan Kara 写道:
> On Sun 12-02-23 17:26:40, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> In order to prevent scan partition for a device that is opened
>> exclusively by someone else, new conditions will be added to
>> disk_scan_partitions() in the next patch. Hence if device is opened
>> exclusively between bdev_add() and disk_scan_partitions(), the first
>> partition scan will fail unexpected. This patch factor out the setting
>> of GD_NEED_PART_SCAN to prevent the problem.
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> 
> I'd rather leave the setting of GD_NEED_PART_SCAN in disk_scan_partitions()
> to keep it self-contained. On top of that we can set GD_NEED_PART_SCAN in
> device_add_disk() to ensure initial partition scan but we should probably
> also make sure flags like GD_SUPPRESS_PART_SCAN or GENHD_FL_NO_PART are not
> set to avoid unwanted partition scanning.

Thanks for the suggestion, I'll remove this patch and do that in patch 3
in the next version.

Thanks,
Kuai
> 
> 								Honza
> 
>> ---
>>   block/genhd.c | 2 +-
>>   block/ioctl.c | 1 +
>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/genhd.c b/block/genhd.c
>> index 075d8da284f5..c0d1220bd798 100644
>> --- a/block/genhd.c
>> +++ b/block/genhd.c
>> @@ -367,7 +367,6 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
>>   	if (disk->open_partitions)
>>   		return -EBUSY;
>>   
>> -	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);
>> @@ -493,6 +492,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
>>   		if (ret)
>>   			goto out_unregister_bdi;
>>   
>> +		set_bit(GD_NEED_PART_SCAN, &disk->state);
>>   		bdev_add(disk->part0, ddev->devt);
>>   		if (get_capacity(disk))
>>   			disk_scan_partitions(disk, FMODE_READ);
>> diff --git a/block/ioctl.c b/block/ioctl.c
>> index 6dd49d877584..0eefcdb936a0 100644
>> --- a/block/ioctl.c
>> +++ b/block/ioctl.c
>> @@ -528,6 +528,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
>>   			return -EACCES;
>>   		if (bdev_is_partition(bdev))
>>   			return -EINVAL;
>> +		set_bit(GD_NEED_PART_SCAN, &bdev->bd_disk->state);
>>   		return disk_scan_partitions(bdev->bd_disk, mode & ~FMODE_EXCL);
>>   	case BLKTRACESTART:
>>   	case BLKTRACESTOP:
>> -- 
>> 2.31.1
>>


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-02-17  1:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-12  9:26 [PATCH -next RFC 0/3] block: fix scan partition for exclusively open device again Yu Kuai
2023-02-12  9:26 ` [PATCH -next RFC 1/3] block: Revert "block: Do not reread partition table on exclusively open device" Yu Kuai
2023-02-12  9:26 ` [PATCH -next RFC 2/3] block: factor out the setting of GD_NEED_PART_SCAN Yu Kuai
2023-02-12 11:36   ` Hannes Reinecke
2023-02-13  2:41     ` Yu Kuai
2023-02-16 13:17   ` Jan Kara
2023-02-17  1:44     ` Yu Kuai
2023-02-12  9:26 ` [PATCH -next RFC 3/3] block: fix scan partition for exclusively open device again Yu Kuai
2023-02-16 13:19   ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).