Message ID | 918750ec-42e8-46d4-bbfb-e01d3dce6ed0@nvidia.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [v4] : ublk: Add UBLK_U_CMD_UPDATE_SIZE | expand |
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. Many of the other parameters in ublk_params/ublksrv_ctrl_dev_info could also feasibly be changed after initial device creation/start. Is it possible to design the API in a way so that we don't have to have one command per parameter? > 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) > 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): > 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): > + 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) > + > /* device state */ > #define UBLK_S_DEV_DEAD 0 > #define UBLK_S_DEV_LIVE 1 > -- > 2.43.0 > >
On 17/04/2025 21:23, Uday Shankar wrote: > 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. > Many of the other parameters in ublk_params/ublksrv_ctrl_dev_info could > also feasibly be changed after initial device creation/start. Is it > possible to design the API in a way so that we don't have to have one > command per parameter? Already spoke to Ming about this. Changing the size is reasonable and easy with a simple api call i.e. set_capacity_and_notify() Changing the other parameters I struggle to see a reasonable need for it and is significantly more complicated. > >> 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) >> 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): >> 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): >> + 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) >> + >> /* device state */ >> #define UBLK_S_DEV_DEAD 0 >> #define UBLK_S_DEV_LIVE 1 >> -- >> 2.43.0 >> >>
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
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) 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): 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): + 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) + /* device state */ #define UBLK_S_DEV_DEAD 0 #define UBLK_S_DEV_LIVE 1
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(-)