On 2019/10/21 17:38, Jan Kara wrote:
> Factor out code handling revalidation of bdev on disk change into a
> common helper.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> fs/block_dev.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 9c073dbdc1b0..88c6d35ec71d 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1512,6 +1512,14 @@ EXPORT_SYMBOL(bd_set_size);
>
> static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part);
>
> +static void bdev_disk_changed(struct block_device *bdev, bool invalidate)
> +{
> + if (invalidate)
> + invalidate_partitions(bdev->bd_disk, bdev);
> + else
> + rescan_partitions(bdev->bd_disk, bdev);
> +}
> +
> /*
> * bd_mutex locking:
> *
> @@ -1594,12 +1602,9 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
> * The latter is necessary to prevent ghost
> * partitions on a removed medium.
> */
> - if (bdev->bd_invalidated) {
> - if (!ret)
> - rescan_partitions(disk, bdev);
> - else if (ret == -ENOMEDIUM)
> - invalidate_partitions(disk, bdev);
> - }
> + if (bdev->bd_invalidated &&
> + (!ret || ret == -ENOMEDIUM))
> + bdev_disk_changed(bdev, ret == -ENOMEDIUM);
This is a little confusing since from its name, bdev_disk_changed() seem
to imply that a new disk is present but this is called only if bdev is
invalidated... What about calling this simply bdev_revalidate_disk(),
since rescan_partitions() will call the disk revalidate method.
Also, it seems to me that this function could be used without the
complex "if ()" condition by slightly modifying it:
static void bdev_revalidate_disk(struct block_device *bdev,
bool invalidate)
{
if (bdev->bd_invalidated && invalidate)
invalidate_partitions(bdev->bd_disk, bdev);
else
rescan_partitions(bdev->bd_disk, bdev);
}
Otherwise, this all looks fine to me.
>
> if (ret)
> goto out_clear;
> @@ -1632,12 +1637,9 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part)
> if (bdev->bd_disk->fops->open)
> ret = bdev->bd_disk->fops->open(bdev, mode);
> /* the same as first opener case, read comment there */
> - if (bdev->bd_invalidated) {
> - if (!ret)
> - rescan_partitions(bdev->bd_disk, bdev);
> - else if (ret == -ENOMEDIUM)
> - invalidate_partitions(bdev->bd_disk, bdev);
> - }
> + if (bdev->bd_invalidated &&
> + (!ret || ret == -ENOMEDIUM))
> + bdev_disk_changed(bdev, ret == -ENOMEDIUM);
> if (ret)
> goto out_unlock_bdev;
> }
>
--
Damien Le Moal
Western Digital Research
