On Mon, Jun 20, 2022 at 5:50 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxx> wrote:
>
> 1. process_msg_open_rsp checks if capacity changed or not before call
> rnbd_clt_change_capacity while the checking also make sense for
> rnbd_clt_resize_dev_store, let's move the checking into the function.
>
> 2. change the parameter type to 'sector_t' then we don't need to cast
> it from rnbd_clt_resize_dev_store, and update rnbd_clt_resize_disk too.
>
> 3. no need to checking the return value, make it return void.
>
better to split this into 3 patches.
> Signed-off-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> ---
> drivers/block/rnbd/rnbd-clt-sysfs.c | 2 +-
> drivers/block/rnbd/rnbd-clt.c | 24 ++++++++++++------------
> drivers/block/rnbd/rnbd-clt.h | 2 +-
> 3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-clt-sysfs.c b/drivers/block/rnbd/rnbd-clt-sysfs.c
> index 2be5d87a3ca6..e7c7d9a68168 100644
> --- a/drivers/block/rnbd/rnbd-clt-sysfs.c
> +++ b/drivers/block/rnbd/rnbd-clt-sysfs.c
> @@ -376,7 +376,7 @@ static ssize_t rnbd_clt_resize_dev_store(struct kobject *kobj,
> if (ret)
> return ret;
>
> - ret = rnbd_clt_resize_disk(dev, (size_t)sectors);
> + ret = rnbd_clt_resize_disk(dev, sectors);
> if (ret)
> return ret;
>
> diff --git a/drivers/block/rnbd/rnbd-clt.c b/drivers/block/rnbd/rnbd-clt.c
> index 2c63cd5ac09d..6c6c4ba3d41d 100644
> --- a/drivers/block/rnbd/rnbd-clt.c
> +++ b/drivers/block/rnbd/rnbd-clt.c
> @@ -68,13 +68,18 @@ static inline bool rnbd_clt_get_dev(struct rnbd_clt_dev *dev)
> return refcount_inc_not_zero(&dev->refcount);
> }
>
> -static int rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
> - size_t new_nsectors)
> +static void rnbd_clt_change_capacity(struct rnbd_clt_dev *dev,
> + sector_t new_nsectors)
> {
> - rnbd_clt_info(dev, "Device size changed from %llu to %zu sectors\n",
> + if (get_capacity(dev->gd) != new_nsectors)
> + return;
This change is broken, it leads to resize no longer work, should be "=="
> +
> + /*
> + * If the size changed, we need to revalidate it
> + */
> + rnbd_clt_info(dev, "Device size changed from %llu to %llu sectors\n",
> get_capacity(dev->gd), new_nsectors);
> set_capacity_and_notify(dev->gd, new_nsectors);
> - return 0;
> }
>
> static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
> @@ -93,12 +98,7 @@ static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
> if (dev->dev_state == DEV_STATE_MAPPED_DISCONNECTED) {
> u64 nsectors = le64_to_cpu(rsp->nsectors);
>
> - /*
> - * If the device was remapped and the size changed in the
> - * meantime we need to revalidate it
> - */
> - if (get_capacity(dev->gd) != nsectors)
> - rnbd_clt_change_capacity(dev, nsectors);
> + rnbd_clt_change_capacity(dev, nsectors);
> gd_kobj = &disk_to_dev(dev->gd)->kobj;
> kobject_uevent(gd_kobj, KOBJ_ONLINE);
> rnbd_clt_info(dev, "Device online, device remapped successfully\n");
> @@ -116,7 +116,7 @@ static int process_msg_open_rsp(struct rnbd_clt_dev *dev,
> return err;
> }
>
> -int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, size_t newsize)
> +int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, sector_t newsize)
> {
> int ret = 0;
>
> @@ -126,7 +126,7 @@ int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, size_t newsize)
> ret = -ENOENT;
> goto out;
> }
> - ret = rnbd_clt_change_capacity(dev, newsize);
> + rnbd_clt_change_capacity(dev, newsize);
>
> out:
> mutex_unlock(&dev->lock);
> diff --git a/drivers/block/rnbd/rnbd-clt.h b/drivers/block/rnbd/rnbd-clt.h
> index df237d2ea0d9..a48e040abe63 100644
> --- a/drivers/block/rnbd/rnbd-clt.h
> +++ b/drivers/block/rnbd/rnbd-clt.h
> @@ -138,7 +138,7 @@ int rnbd_clt_unmap_device(struct rnbd_clt_dev *dev, bool force,
> const struct attribute *sysfs_self);
>
> int rnbd_clt_remap_device(struct rnbd_clt_dev *dev);
> -int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, size_t newsize);
> +int rnbd_clt_resize_disk(struct rnbd_clt_dev *dev, sector_t newsize);
>
> /* rnbd-clt-sysfs.c */
>
> --
> 2.34.1
>
