Re: [PATCH v4 2/2] ublk: add UBLK_CMD_TRY_STOP_DEV command

[Yoav Cohen <yoav@nvidia.com>]

On 07/01/2026 15:20, Ming Lei wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Tue, Jan 06, 2026 at 10:33:33PM +0200, Yoav Cohen wrote:
>> This command is similar to UBLK_CMD_STOP_DEV, but it only stops the
>> device if there are no active openers for the ublk block device.
>> If the device is busy, the command returns -EBUSY instead of
>> disrupting active clients. This allows safe, non-destructive stopping.
>>
>> Advertise UBLK_CMD_TRY_STOP_DEV support via UBLK_F_SAFE_STOP_DEV
>> feature flag.
>>
>> Signed-off-by: Yoav Cohen <yoav@nvidia.com>
>> ---
>>   drivers/block/ublk_drv.c             | 44 ++++++++++++++++++++++++++--
>>   include/uapi/linux/ublk_cmd.h        |  9 +++++-
>>   tools/testing/selftests/ublk/kublk.c |  1 +
>>   3 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
>> index 2d5602ef05cc..fc8b87902f8f 100644
>> --- a/drivers/block/ublk_drv.c
>> +++ b/drivers/block/ublk_drv.c
>> @@ -54,6 +54,7 @@
>>   #define UBLK_CMD_DEL_DEV_ASYNC       _IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
>>   #define UBLK_CMD_UPDATE_SIZE _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
>>   #define UBLK_CMD_QUIESCE_DEV _IOC_NR(UBLK_U_CMD_QUIESCE_DEV)
>> +#define UBLK_CMD_TRY_STOP_DEV        _IOC_NR(UBLK_U_CMD_TRY_STOP_DEV)
>>
>>   #define UBLK_IO_REGISTER_IO_BUF              _IOC_NR(UBLK_U_IO_REGISTER_IO_BUF)
>>   #define UBLK_IO_UNREGISTER_IO_BUF    _IOC_NR(UBLK_U_IO_UNREGISTER_IO_BUF)
>> @@ -73,7 +74,8 @@
>>                | UBLK_F_AUTO_BUF_REG \
>>                | UBLK_F_QUIESCE \
>>                | UBLK_F_PER_IO_DAEMON \
>> -             | UBLK_F_BUF_REG_OFF_DAEMON)
>> +             | UBLK_F_BUF_REG_OFF_DAEMON \
>> +             | UBLK_F_SAFE_STOP_DEV)
>>
>>   #define UBLK_F_ALL_RECOVERY_FLAGS (UBLK_F_USER_RECOVERY \
>>                | UBLK_F_USER_RECOVERY_REISSUE \
>> @@ -239,6 +241,8 @@ struct ublk_device {
>>        struct delayed_work     exit_work;
>>        struct work_struct      partition_scan_work;
>>
>> +     bool                    block_open; /* protected by open_mutex */
>> +
>>        struct ublk_queue       *queues[];
>>   };
>>
>> @@ -919,6 +923,9 @@ static int ublk_open(struct gendisk *disk, blk_mode_t mode)
>>                        return -EPERM;
>>        }
>>
>> +     if (ub->block_open)
>> +             return -EBUSY;
>> +
>>        return 0;
>>   }
>>
>> @@ -3188,7 +3195,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
>>        ub->dev_info.flags |= UBLK_F_CMD_IOCTL_ENCODE |
>>                UBLK_F_URING_CMD_COMP_IN_TASK |
>>                UBLK_F_PER_IO_DAEMON |
>> -             UBLK_F_BUF_REG_OFF_DAEMON;
>> +             UBLK_F_BUF_REG_OFF_DAEMON |
>> +             UBLK_F_SAFE_STOP_DEV;
>>
>>        /* GET_DATA isn't needed any more with USER_COPY or ZERO COPY */
>>        if (ub->dev_info.flags & (UBLK_F_USER_COPY | UBLK_F_SUPPORT_ZERO_COPY |
>> @@ -3309,6 +3317,34 @@ static void ublk_ctrl_stop_dev(struct ublk_device *ub)
>>        ublk_stop_dev(ub);
>>   }
>>
>> +static int ublk_ctrl_try_stop_dev(struct ublk_device *ub)
>> +{
>> +     struct gendisk *disk;
>> +     int ret = 0;
>> +
>> +     disk = ublk_get_disk(ub);
>> +     if (!disk)
>> +             return -ENODEV;
>> +
>> +     mutex_lock(&disk->open_mutex);
>> +     if (disk_openers(disk) > 0) {
>> +             ret = -EBUSY;
>> +             goto unlock;
>> +     }
>> +     ub->block_open = true;
>> +     /* release open_mutex as del_gendisk() will reacquire it */
>> +     mutex_unlock(&disk->open_mutex);
>> +
>> +     ublk_ctrl_stop_dev(ub);
>> +     goto out;
>> +
>> +unlock:
>> +     mutex_unlock(&disk->open_mutex);
>> +out:
>> +     ublk_put_disk(disk);
>> +     return ret;
>> +}
>> +
>>   static int ublk_ctrl_get_dev_info(struct ublk_device *ub,
>>                const struct ublksrv_ctrl_cmd *header)
>>   {
>> @@ -3704,6 +3740,7 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
>>        case UBLK_CMD_END_USER_RECOVERY:
>>        case UBLK_CMD_UPDATE_SIZE:
>>        case UBLK_CMD_QUIESCE_DEV:
>> +     case UBLK_CMD_TRY_STOP_DEV:
>>                mask = MAY_READ | MAY_WRITE;
>>                break;
>>        default:
>> @@ -3817,6 +3854,9 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
>>        case UBLK_CMD_QUIESCE_DEV:
>>                ret = ublk_ctrl_quiesce_dev(ub, header);
>>                break;
>> +     case UBLK_CMD_TRY_STOP_DEV:
>> +             ret = ublk_ctrl_try_stop_dev(ub);
>> +             break;
>>        default:
>>                ret = -EOPNOTSUPP;
>>                break;
>> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
>> index ec77dabba45b..9daa8ab372f0 100644
>> --- a/include/uapi/linux/ublk_cmd.h
>> +++ b/include/uapi/linux/ublk_cmd.h
>> @@ -55,7 +55,8 @@
>>        _IOWR('u', 0x15, struct ublksrv_ctrl_cmd)
>>   #define UBLK_U_CMD_QUIESCE_DEV               \
>>        _IOWR('u', 0x16, struct ublksrv_ctrl_cmd)
>> -
>> +#define UBLK_U_CMD_TRY_STOP_DEV              \
>> +     _IOWR('u', 0x17, struct ublksrv_ctrl_cmd)
>>   /*
>>    * 64bits are enough now, and it should be easy to extend in case of
>>    * running out of feature flags
>> @@ -311,6 +312,12 @@
>>    */
>>   #define UBLK_F_BUF_REG_OFF_DAEMON (1ULL << 14)
>>
>> +/*
>> + * The device supports the UBLK_CMD_TRY_STOP_DEV command, which
>> + * allows stopping the device only if there are no openers.
>> + */
>> +#define UBLK_F_SAFE_STOP_DEV (1ULL << 17)
>> +
>>   /* device state */
>>   #define UBLK_S_DEV_DEAD      0
>>   #define UBLK_S_DEV_LIVE      1
>> diff --git a/tools/testing/selftests/ublk/kublk.c b/tools/testing/selftests/ublk/kublk.c
>> index 185ba553686a..6739e28c4059 100644
>> --- a/tools/testing/selftests/ublk/kublk.c
>> +++ b/tools/testing/selftests/ublk/kublk.c
>> @@ -1454,6 +1454,7 @@ static int cmd_dev_get_features(void)
>>                FEAT_NAME(UBLK_F_QUIESCE),
>>                FEAT_NAME(UBLK_F_PER_IO_DAEMON),
>>                FEAT_NAME(UBLK_F_BUF_REG_OFF_DAEMON),
>> +             FEAT_NAME(UBLK_F_SAFE_STOP_DEV),
> 
> We usually split driver and test code into different patches, and the patch
> looks fine:
> 
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> 
> But we usually separate test code from driver change, also new feature needs
> selftest code for verifying, can you move `+FEAT_NAME(UBLK_F_SAFE_STOP_DEV)` into
> the attached selftest patch and post all 3 in v5?
> 
> 
> 
> Thanks,
> Ming

Hi,

Thank you Ming, just to be sure:
I'll split the kublk change to a 3rd patch - that's ok.
But do you also want me to write a selftest for this feature as part of 
the 3rd patch?

Thank you