On Mon, Jun 17, 2024 at 01:44:50PM -0600, Uday Shankar wrote:
> Save some lines by merging stop_work and quiesce_work into nosrv_work,
> which looks at the recovery flags and does the right thing when the "no
> ublk server" condition is detected.
>
> Signed-off-by: Uday Shankar <ushankar@xxxxxxxxxxxxxxx>
> ---
> drivers/block/ublk_drv.c | 64 ++++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index e8cca58a71bc..0496fa372cc1 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -182,8 +182,7 @@ struct ublk_device {
> unsigned int nr_queues_ready;
> unsigned int nr_privileged_daemon;
>
> - struct work_struct quiesce_work;
> - struct work_struct stop_work;
> + struct work_struct nosrv_work;
> };
>
> /* header of ublk_params */
> @@ -1229,10 +1228,7 @@ static enum blk_eh_timer_return ublk_timeout(struct request *rq)
> struct ublk_device *ub = ubq->dev;
>
> if (ublk_abort_requests(ub, ubq)) {
> - if (ublk_nosrv_should_stop_dev(ub))
> - schedule_work(&ub->stop_work);
> - else
> - schedule_work(&ub->quiesce_work);
> + schedule_work(&ub->nosrv_work);
> }
> return BLK_EH_DONE;
> }
> @@ -1482,10 +1478,7 @@ static void ublk_uring_cmd_cancel_fn(struct io_uring_cmd *cmd,
> ublk_cancel_cmd(ubq, io, issue_flags);
>
> if (need_schedule) {
> - if (ublk_nosrv_should_stop_dev(ub))
> - schedule_work(&ub->stop_work);
> - else
> - schedule_work(&ub->quiesce_work);
> + schedule_work(&ub->nosrv_work);
> }
> }
>
> @@ -1548,20 +1541,6 @@ static void __ublk_quiesce_dev(struct ublk_device *ub)
> ub->dev_info.state = UBLK_S_DEV_QUIESCED;
> }
>
> -static void ublk_quiesce_work_fn(struct work_struct *work)
> -{
> - struct ublk_device *ub =
> - container_of(work, struct ublk_device, quiesce_work);
> -
> - mutex_lock(&ub->mutex);
> - if (ub->dev_info.state != UBLK_S_DEV_LIVE)
> - goto unlock;
> - __ublk_quiesce_dev(ub);
> - unlock:
> - mutex_unlock(&ub->mutex);
> - ublk_cancel_dev(ub);
> -}
> -
> static void ublk_unquiesce_dev(struct ublk_device *ub)
> {
> int i;
> @@ -1610,6 +1589,25 @@ static void ublk_stop_dev(struct ublk_device *ub)
> ublk_cancel_dev(ub);
> }
>
> +static void ublk_nosrv_work(struct work_struct *work)
> +{
> + struct ublk_device *ub =
> + container_of(work, struct ublk_device, nosrv_work);
> +
> + if (ublk_nosrv_should_stop_dev(ub)) {
> + ublk_stop_dev(ub);
> + return;
> + }
> +
> + mutex_lock(&ub->mutex);
> + if (ub->dev_info.state != UBLK_S_DEV_LIVE)
> + goto unlock;
> + __ublk_quiesce_dev(ub);
> + unlock:
> + mutex_unlock(&ub->mutex);
> + ublk_cancel_dev(ub);
> +}
> +
> /* device can only be started after all IOs are ready */
> static void ublk_mark_io_ready(struct ublk_device *ub, struct ublk_queue *ubq)
> {
> @@ -2124,14 +2122,6 @@ static int ublk_add_chdev(struct ublk_device *ub)
> return ret;
> }
>
> -static void ublk_stop_work_fn(struct work_struct *work)
> -{
> - struct ublk_device *ub =
> - container_of(work, struct ublk_device, stop_work);
> -
> - ublk_stop_dev(ub);
> -}
> -
> /* align max io buffer size with PAGE_SIZE */
> static void ublk_align_max_io_size(struct ublk_device *ub)
> {
> @@ -2156,8 +2146,7 @@ static int ublk_add_tag_set(struct ublk_device *ub)
> static void ublk_remove(struct ublk_device *ub)
> {
> ublk_stop_dev(ub);
> - cancel_work_sync(&ub->stop_work);
> - cancel_work_sync(&ub->quiesce_work);
> + cancel_work_sync(&ub->nosrv_work);
> cdev_device_del(&ub->cdev, &ub->cdev_dev);
> ublk_put_device(ub);
> ublks_added--;
> @@ -2412,8 +2401,7 @@ static int ublk_ctrl_add_dev(struct io_uring_cmd *cmd)
> goto out_unlock;
> mutex_init(&ub->mutex);
> spin_lock_init(&ub->lock);
> - INIT_WORK(&ub->quiesce_work, ublk_quiesce_work_fn);
> - INIT_WORK(&ub->stop_work, ublk_stop_work_fn);
> + INIT_WORK(&ub->nosrv_work, ublk_nosrv_work);
>
> ret = ublk_alloc_dev_number(ub, header->dev_id);
> if (ret < 0)
> @@ -2548,9 +2536,7 @@ static inline void ublk_ctrl_cmd_dump(struct io_uring_cmd *cmd)
> static int ublk_ctrl_stop_dev(struct ublk_device *ub)
> {
> ublk_stop_dev(ub);
> - cancel_work_sync(&ub->stop_work);
> - cancel_work_sync(&ub->quiesce_work);
> -
> + cancel_work_sync(&ub->nosrv_work);
> return 0;
> }
Looks fine,
Reviewed-by: Ming Lei <ming.lei@xxxxxxxxxx>
Thanks,
Ming
