All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Jared Holzman <jholzman@nvidia.com>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Jens Axboe <axboe@kernel.dk>,
	csander@purestorage.com
Subject: Re: [PATCH v4]: ublk: Add UBLK_U_CMD_UPDATE_SIZE
Date: Fri, 18 Apr 2025 07:35:09 +0800	[thread overview]
Message-ID: <aAGQLYDOFY5PyUMJ@fedora> (raw)
In-Reply-To: <918750ec-42e8-46d4-bbfb-e01d3dce6ed0@nvidia.com>

On Wed, Apr 16, 2025 at 01:07:47PM +0300, Jared Holzman wrote:
> Currently ublk only allows the size of the ublkb block device to be
> set via UBLK_CMD_SET_PARAMS before UBLK_CMD_START_DEV is triggered.
> 
> This does not provide support for extendable user-space block devices
> without having to stop and restart the underlying ublkb block device
> causing IO interruption.
> 
> This patch adds a new ublk command UBLK_U_CMD_UPDATE_SIZE to allow the
> ublk block device to be resized on-the-fly.
> 
> Feature flag UBLK_F_UPDATE_SIZE is also added to indicate support for this
> command.
> 
> Signed-off-by: Omri Mann <omri@nvidia.com>
> ---
>  drivers/block/ublk_drv.c      | 18 +++++++++++++++++-
>  include/uapi/linux/ublk_cmd.h |  7 +++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index cdb1543fa4a9..128f094efbad 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -64,7 +64,8 @@
>          | UBLK_F_CMD_IOCTL_ENCODE \
>          | UBLK_F_USER_COPY \
>          | UBLK_F_ZONED \
> -        | UBLK_F_USER_RECOVERY_FAIL_IO)
> +        | UBLK_F_USER_RECOVERY_FAIL_IO \
> +        | UBLK_F_UPDATE_SIZE)
> 
>  #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
>          | UBLK_F_USER_RECOVERY_REISSUE \
> @@ -3067,6 +3068,16 @@ static int ublk_ctrl_get_features(const struct
> ublksrv_ctrl_cmd *header)

I try to apply this patch downloaded from both lore or patchwork, and 'git
am' always complains the patch is broken:

[root@ktest-40 linux]# git am raw
warning: Patch sent with format=flowed; space at the end of lines might be lost.
Applying: ublk: Add UBLK_U_CMD_UPDATE_SIZE
error: corrupt patch at line 11
Patch failed at 0001 ublk: Add UBLK_U_CMD_UPDATE_SIZE
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
[root@ktest-40 linux]# patch -p1 < raw
patching file drivers/block/ublk_drv.c
Hunk #1 FAILED at 64.
patch: **** malformed patch at line 192: ublksrv_ctrl_cmd *header)

Please use 'git-format-patch' to make patch and run `./scripts/checkpatch.pl -g HEAD`
in kernel top directory to make sure no ERROR.

>      return 0;
>  }
> 
> +static void ublk_ctrl_set_size(struct ublk_device *ub, const struct
> ublksrv_ctrl_cmd *header)
> +{
> +    struct ublk_param_basic *p = &ub->params.basic;
> +    u64 new_size = header->data[0];
> +
> +    mutex_lock(&ub->mutex);
> +    p->dev_sectors = new_size;
> +    set_capacity_and_notify(ub->ub_disk, p->dev_sectors);
> +    mutex_unlock(&ub->mutex);
> +}
>  /*
>   * All control commands are sent via /dev/ublk-control, so we have to check
>   * the destination device's permission
> @@ -3152,6 +3163,7 @@ static int ublk_ctrl_uring_cmd_permission(struct
> ublk_device *ub,
>      case UBLK_CMD_SET_PARAMS:
>      case UBLK_CMD_START_USER_RECOVERY:
>      case UBLK_CMD_END_USER_RECOVERY:
> +    case _IOC_NR(UBLK_U_CMD_UPDATE_SIZE):

Here it could be more clean to add one private definition:

	#define UBLK_CMD_UPDATE_SIZE _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)

just like what we did for `UBLK_CMD_DEL_DEV_ASYNC`.

Then use UBLK_CMD_UPDATE_SIZE directly.

>          mask = MAY_READ | MAY_WRITE;
>          break;
>      default:
> @@ -3243,6 +3255,10 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd
> *cmd,
>      case UBLK_CMD_END_USER_RECOVERY:
>          ret = ublk_ctrl_end_recovery(ub, header);
>          break;
> +    case _IOC_NR(UBLK_U_CMD_UPDATE_SIZE):

Same with above.

> +        ublk_ctrl_set_size(ub, header);
> +        ret = 0;
> +        break;
>      default:
>          ret = -EOPNOTSUPP;
>          break;
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index 583b86681c93..587a54b3cfe1 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -51,6 +51,8 @@
>      _IOR('u', 0x13, struct ublksrv_ctrl_cmd)
>  #define UBLK_U_CMD_DEL_DEV_ASYNC    \
>      _IOR('u', 0x14, struct ublksrv_ctrl_cmd)
> +#define UBLK_U_CMD_UPDATE_SIZE        \
> +    _IOWR('u', 0x15, struct ublksrv_ctrl_cmd)
> 
>  /*
>   * 64bits are enough now, and it should be easy to extend in case of
> @@ -211,6 +213,11 @@
>   */
>  #define UBLK_F_USER_RECOVERY_FAIL_IO (1ULL << 9)
> 
> +/*
> + * Resizing a block device is possible with UBLK_U_CMD_UPDATE_SIZE
> + */
> +#define UBLK_F_UPDATE_SIZE         (1ULL << 10)

Please document how size is passed, and the unit is sector.

With above addressed, this patch looks fine.


Thanks,
Ming


  parent reply	other threads:[~2025-04-17 23:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-16 10:07 [PATCH v4]: ublk: Add UBLK_U_CMD_UPDATE_SIZE Jared Holzman
2025-04-17 18:23 ` Uday Shankar
2025-04-17 21:05   ` Jared Holzman
2025-04-17 23:35 ` Ming Lei [this message]
2025-04-20  8:06   ` Jared Holzman
2025-04-21  2:51     ` Ming Lei
2025-04-21 11:03       ` Jared Holzman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aAGQLYDOFY5PyUMJ@fedora \
    --to=ming.lei@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=csander@purestorage.com \
    --cc=jholzman@nvidia.com \
    --cc=linux-block@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.