On 2021/07/24 16:15, Christoph Hellwig wrote:
> Instead of acquiring an inode reference on open make sure partitions
> always hold device model references to the disk while alive, and switch
> open to grab only a device model reference to the opened block device.
> If that is a partition the disk reference is transitively held by the
> partition already.
Your SoB is missing...
> ---
> block/partitions/core.c | 4 +++
> fs/block_dev.c | 60 ++++++++++++++++-------------------------
> 2 files changed, 27 insertions(+), 37 deletions(-)
>
> diff --git a/block/partitions/core.c b/block/partitions/core.c
> index ae88b5439056..1b02073a2047 100644
> --- a/block/partitions/core.c
> +++ b/block/partitions/core.c
> @@ -261,6 +261,7 @@ static void part_release(struct device *dev)
> {
> if (MAJOR(dev->devt) == BLOCK_EXT_MAJOR)
> blk_free_ext_minor(MINOR(dev->devt));
> + put_disk(dev_to_bdev(dev)->bd_disk);
> bdput(dev_to_bdev(dev));
> }
>
> @@ -364,6 +365,9 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
> pdev->type = &part_type;
> pdev->parent = ddev;
>
> + /* ensure we always have a reference to the whole disk */
> + get_device(disk_to_dev(disk));
> +
> /* in consecutive minor range? */
> if (bdev->bd_partno < disk->minors) {
> devt = MKDEV(disk->major, disk->first_minor + bdev->bd_partno);
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 932f4034ad66..4a6c8c0a3bc9 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -921,16 +921,6 @@ void bdev_add(struct block_device *bdev, dev_t dev)
> insert_inode_hash(bdev->bd_inode);
> }
>
> -static struct block_device *bdget(dev_t dev)
> -{
> - struct inode *inode;
> -
> - inode = ilookup(blockdev_superblock, dev);
> - if (!inode)
> - return NULL;
> - return &BDEV_I(inode)->bdev;
> -}
> -
> /**
> * bdgrab -- Grab a reference to an already referenced block device
> * @bdev: Block device to grab a reference to.
> @@ -1282,16 +1272,14 @@ static void blkdev_put_whole(struct block_device *bdev, fmode_t mode)
> static int blkdev_get_part(struct block_device *part, fmode_t mode)
> {
> struct gendisk *disk = part->bd_disk;
> - struct block_device *whole;
> int ret;
>
> if (part->bd_openers)
> goto done;
>
> - whole = bdgrab(disk->part0);
> - ret = blkdev_get_whole(whole, mode);
> + ret = blkdev_get_whole(bdev_whole(part), mode);
> if (ret)
> - goto out_put_whole;
> + return ret;
>
> ret = -ENXIO;
> if (!bdev_nr_sectors(part))
> @@ -1306,9 +1294,7 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
> return 0;
>
> out_blkdev_put:
> - blkdev_put_whole(whole, mode);
> -out_put_whole:
> - bdput(whole);
> + blkdev_put_whole(bdev_whole(part), mode);
> return ret;
> }
>
> @@ -1321,42 +1307,42 @@ static void blkdev_put_part(struct block_device *part, fmode_t mode)
> blkdev_flush_mapping(part);
> whole->bd_disk->open_partitions--;
> blkdev_put_whole(whole, mode);
> - bdput(whole);
> }
>
> struct block_device *blkdev_get_no_open(dev_t dev)
> {
> struct block_device *bdev;
> - struct gendisk *disk;
> + struct inode *inode;
>
> - bdev = bdget(dev);
> - if (!bdev) {
> + inode = ilookup(blockdev_superblock, dev);
> + if (!inode) {
> blk_request_module(dev);
> - bdev = bdget(dev);
> - if (!bdev)
> + inode = ilookup(blockdev_superblock, dev);
> + if (!inode)
> return NULL;
> }
>
> - disk = bdev->bd_disk;
> - if (!kobject_get_unless_zero(&disk_to_dev(disk)->kobj))
> - goto bdput;
> - if (disk->flags & GENHD_FL_HIDDEN)
> - goto put_disk;
> - if (!try_module_get(bdev->bd_disk->fops->owner))
> - goto put_disk;
> + /* switch from the inode reference to a device mode one: */
> + bdev = &BDEV_I(inode)->bdev;
> + if (!kobject_get_unless_zero(&bdev->bd_device.kobj))
> + bdev = NULL;
> + iput(inode);
> +
> + if (!bdev)
> + return NULL;
> + if ((bdev->bd_disk->flags & GENHD_FL_HIDDEN) ||
> + !try_module_get(bdev->bd_disk->fops->owner)) {
> + put_device(&bdev->bd_device);
> + return NULL;
> + }
> +
> return bdev;
> -put_disk:
> - put_disk(disk);
> -bdput:
> - bdput(bdev);
> - return NULL;
> }
>
> void blkdev_put_no_open(struct block_device *bdev)
> {
> module_put(bdev->bd_disk->fops->owner);
> - put_disk(bdev->bd_disk);
> - bdput(bdev);
> + put_device(&bdev->bd_device);
> }
>
> /**
>
--
Damien Le Moal
Western Digital Research
