| Message ID | 20230217022200.3092987-3-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: fix scan partition for exclusively open device again | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Fri 17-02-23 10:22:00, 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> Looks good to me, just two minor comments below: > diff --git a/block/genhd.c b/block/genhd.c > index b30d5538710c..3ee5577e1586 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; > @@ -368,11 +369,27 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode) > return -EBUSY; > > set_bit(GD_NEED_PART_SCAN, &disk->state); I'd move the set_bit() after we are sure we have exclusive access to the bdev. Otherwise we could set GD_NEED_PART_SCAN on a device exclusively open by someone else and if we race with open in an unfortunate way, we could trigger unexpected partition scan... > - bdev = blkdev_get_by_dev(disk_devt(disk), mode, NULL); > + /* > + * 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 exclusive 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)) > - return PTR_ERR(bdev); > - blkdev_put(bdev, mode); > - return 0; > + ret = PTR_ERR(bdev); > + else > + blkdev_put(bdev, mode); > + > + if (!(mode & FMODE_EXCL)) > + bd_abort_claiming(disk->part0, disk_scan_partitions); > + return ret; > } > > /** > @@ -494,6 +511,11 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk, > if (ret) > goto out_unregister_bdi; > > + /* Make sure the first partition scan will be proceed */ ^^^^^^ "will happen" probably makes more sense here. Honza
diff --git a/block/genhd.c b/block/genhd.c index b30d5538710c..3ee5577e1586 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; @@ -368,11 +369,27 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode) return -EBUSY; set_bit(GD_NEED_PART_SCAN, &disk->state); - bdev = blkdev_get_by_dev(disk_devt(disk), mode, NULL); + /* + * 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 exclusive 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)) - return PTR_ERR(bdev); - blkdev_put(bdev, mode); - return 0; + ret = PTR_ERR(bdev); + else + blkdev_put(bdev, mode); + + if (!(mode & FMODE_EXCL)) + bd_abort_claiming(disk->part0, disk_scan_partitions); + return ret; } /** @@ -494,6 +511,11 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk, if (ret) goto out_unregister_bdi; + /* Make sure the first partition scan will be proceed */ + if (get_capacity(disk) && !(disk->flags & GENHD_FL_NO_PART) && + !test_bit(GD_SUPPRESS_PART_SCAN, &disk->state)) + 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..9c5f637ff153 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -528,7 +528,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode, return -EACCES; if (bdev_is_partition(bdev)) return -EINVAL; - return disk_scan_partitions(bdev->bd_disk, mode & ~FMODE_EXCL); + return disk_scan_partitions(bdev->bd_disk, mode); case BLKTRACESTART: case BLKTRACESTOP: case BLKTRACETEARDOWN: