Re: [PATCH] ublk: Add UBLK_U_CMD_SET_SIZE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ming,

On 05/04/2025 5:51, Ming Lei wrote:
External email: Use caution opening links or attachments


Hello Jared,

On Thu, Apr 03, 2025 at 12:37:11PM +0000, Jared Holzman wrote:
Apologies if this is a dup, but I am not seeing the original mail on the mailing list archive.
I guess it is because the patch is sent as html, instead of plain test,
please follow the patch submission guide:

https://www.kernel.org/doc/Documentation/process/submitting-patches.rst

Sorry about that, I originally sent the mail using git send-mail, but our internal smtp relay does not support outside addresses. I then tried forwarding it from Outlook and it decided to add HTML without telling me.

I'm using Thunderbird now, so hopefully it will be in plain-text as required.

________________________________
From: Jared Holzman <jholzman@xxxxxxxxxx>
Sent: Monday, 31 March 2025 4:54 PM
To: linux-block@xxxxxxxxxxxxxxx <linux-block@xxxxxxxxxxxxxxx>
Cc: ming.lei@xxxxxxxxxx <ming.lei@xxxxxxxxxx>; Omri Mann <omri@xxxxxxxxxx>; Ofer Oshri <ofer@xxxxxxxxxx>; Omri Levi <omril@xxxxxxxxxx>; Jared Holzman <jholzman@xxxxxxxxxx>
Subject: [PATCH] ublk: Add UBLK_U_CMD_SET_SIZE

From: Omri Mann <omri@xxxxxxxxxx>

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.
The requirement is reasonable.

This patch adds a new ublk command UBLK_U_CMD_SET_SIZE to allow the
ublk block device to be resized on-the-fly.
Looks CMD_SET_SIZE is not generic enough, maybe UBLK_CMD_UPDATE_PARAMS
can be added for support any parameter update by allowing to do it
when device is in LIVE state.

That's fine, but we'd rather not take on the burden of verifying all of ublk_params to see which ones can be safely changed on-the-fly.

Would it be reasonable to have UBLK_CMD_UPDATE_PARAMS accept a different struct "ublk_param_update" which contains only the parameters that can be updated in the LIVE state and will include only max_sectors for now?

Alternatively if you know off the top of your head which parameters can be easily changed on-the-fly and we will add only those.

Signed-off-by: Omri Mann <omri@xxxxxxxxxx>
---
  Documentation/block/ublk.rst  |  5 +++++
  drivers/block/ublk_drv.c      | 26 +++++++++++++++++++++++++-
  include/uapi/linux/ublk_cmd.h |  7 +++++++
  3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/Documentation/block/ublk.rst b/Documentation/block/ublk.rst
index 1e0e7358e14a..7eca87a66b9c 100644
--- a/Documentation/block/ublk.rst
+++ b/Documentation/block/ublk.rst
@@ -139,6 +139,11 @@ managing and controlling ublk devices with help of several control commands:
    set up the per-queue context efficiently, such as bind affine CPUs with IO
    pthread and try to allocate buffers in IO thread context.

+- ``UBLK_F_SET_SIZE``
+
+  Allows changing the size of the block device after it has started. Useful for
+  userspace implementations that allow extending the underlying block device.
+
  - ``UBLK_CMD_GET_DEV_INFO``

    For retrieving device info via ``ublksrv_ctrl_dev_info``. It is the server's
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index c060da409ed8..ab6364475b9c 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_SET_SIZE)

  #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
                  | UBLK_F_USER_RECOVERY_REISSUE \
@@ -2917,6 +2918,25 @@ static int ublk_ctrl_get_features(struct io_uring_cmd *cmd)
          return 0;
  }

+static int ublk_ctrl_set_size(struct ublk_device *ub,
+               struct io_uring_cmd *cmd)
+{
+       const struct ublksrv_ctrl_cmd *header = io_uring_sqe_cmd(cmd->sqe);
+       struct ublk_param_basic *p = &ub->params.basic;
+       size_t new_size = (int)header->data[0];
+       int ret = 0;
+       unsigned int memflags;
+
+       p->dev_sectors = new_size;
+
+       memflags = blk_mq_freeze_queue(ub->ub_disk->queue);
+       mutex_lock(&ub->mutex);
+       set_capacity_and_notify(ub->ub_disk, p->dev_sectors);
+       mutex_unlock(&ub->mutex);
+       blk_mq_unfreeze_queue(ub->ub_disk->queue, memflags);
Actually if it is just for updating device size, queue freeze isn't needed,
because bio_check_eod() is called without grabbing ->q_usage_counter, but
for updating other parameters, freezing queue is often needed.

Gotcha. I will remove it and send a new version of the patch.

Thanks,

Jared



Thanks,
Ming

--
Jared Holzman
Senior Software Engineer
NVIDIA
jholzman@xxxxxxxxxx





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux