> On Jan 25, 2023, at 1:05 PM, Jens Axboe <axboe@xxxxxxxxx> wrote:
>
> On 1/25/23 12:50?PM, Harris, James R wrote:
>> Hi,
>>
>> I can reliably hit a kernel oops with ublk_drv using the following abnormal sequence of events (repro .c file at end of this e-mail):
>>
>> 1) modprobe ublk_drv
>> 2) run test app which basically does:
>> a) submit UBLK_CMD_ADD_DEV
>> b) submit UBLK_CMD_SET_PARAMS
>> c) wait for completions
>> d) do *not* submit UBLK_CMD_START_DEV
>> e) exit
>> 3) rmmod ublk_drv
>>
>> Reproduces on 6.2-rc5, 6.1.5 and 6.1.
>
> Something like this may do it, but I'll let Ming sort out the details
> on what's the most appropriate fix.
Thanks Jens. This patch fixes things for me. If you and Ming decide to go forward
with this patch, feel free to add:
Tested-by: Jim Harris <james.r.harris@xxxxxxxxx>
>
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 17b677b5d3b2..dacc13e2a476 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -1149,11 +1149,17 @@ static void ublk_unquiesce_dev(struct ublk_device *ub)
> blk_mq_kick_requeue_list(ub->ub_disk->queue);
> }
>
> -static void ublk_stop_dev(struct ublk_device *ub)
> +/*
> + * Returns if the device was live or not
> + */
> +static bool ublk_stop_dev(struct ublk_device *ub)
> {
> + bool was_live = false;
> +
> mutex_lock(&ub->mutex);
> if (ub->dev_info.state == UBLK_S_DEV_DEAD)
> goto unlock;
> + was_live = true;
> if (ublk_can_use_recovery(ub)) {
> if (ub->dev_info.state == UBLK_S_DEV_LIVE)
> __ublk_quiesce_dev(ub);
> @@ -1168,6 +1174,7 @@ static void ublk_stop_dev(struct ublk_device *ub)
> ublk_cancel_dev(ub);
> mutex_unlock(&ub->mutex);
> cancel_delayed_work_sync(&ub->monitor_work);
> + return was_live;
> }
>
> /* device can only be started after all IOs are ready */
> @@ -1470,7 +1477,8 @@ static int ublk_add_tag_set(struct ublk_device *ub)
>
> static void ublk_remove(struct ublk_device *ub)
> {
> - ublk_stop_dev(ub);
> + if (!ublk_stop_dev(ub))
> + return;
> cancel_work_sync(&ub->stop_work);
> cancel_work_sync(&ub->quiesce_work);
> cdev_device_del(&ub->cdev, &ub->cdev_dev);
> @@ -1771,7 +1779,8 @@ static int ublk_ctrl_stop_dev(struct io_uring_cmd *cmd)
> if (!ub)
> return -EINVAL;
>
> - ublk_stop_dev(ub);
> + if (!ublk_stop_dev(ub))
> + return -EINVAL;
> cancel_work_sync(&ub->stop_work);
> cancel_work_sync(&ub->quiesce_work);
>
>
> --
> Jens Axboe
>
