On 4/10/25 7:00 PM, Ming Lei wrote:
> /*
> * Use the default elevator settings. If the chosen elevator initialization
> * fails, fall back to the "none" elevator (no elevator).
> */
> -void elevator_init_mq(struct request_queue *q)
> +void elevator_set_default(struct request_queue *q)
> {
> - struct elevator_type *e;
> - unsigned int memflags;
> + struct elev_change_ctx ctx = { };
> int err;
>
> - WARN_ON_ONCE(blk_queue_registered(q));
> -
> - if (unlikely(q->elevator))
> + if (!queue_is_mq(q))
> return;
>
> - e = elevator_get_default(q);
> - if (!e)
> + ctx.name = use_default_elevator(q) ? "mq-deadline" : "none";
> + if (!q->elevator && !strcmp(ctx.name, "none"))
> return;
> + err = elevator_change(q, &ctx);
> + if (err < 0)
> + pr_warn("\"%s\" set elevator failed %d, "
> + "falling back to \"none\"\n", ctx.name, err);
> +}
>
If we fail to set the evator to default (mq-deadline) while registering queue,
because nr_hw_queue update is simultaneously running then we may end up setting
the queue elevator to none and that's not correct. Isn't it?
> +void elevator_set_none(struct request_queue *q)
> +{
> + struct elev_change_ctx ctx = {
> + .name = "none",
> + .uevent = 1,
> + };
> + int err;
>
> - blk_mq_unfreeze_queue(q, memflags);
> + if (!queue_is_mq(q))
> + return;
>
> - if (err) {
> - pr_warn("\"%s\" elevator initialization failed, "
> - "falling back to \"none\"\n", e->elevator_name);
> - }
> + if (!q->elevator)
> + return;
>
> - elevator_put(e);
> + err = elevator_change(q, &ctx);
> + if (err < 0)
> + pr_warn("%s: set none elevator failed %d\n", __func__, err);
> }
>
Here as well if we fail to disable/exit elevator while deleting disk
because nr_hw_queue update is simultaneously running then we may
leak elevator resource?
> @@ -565,11 +559,7 @@ int __must_check add_disk_fwnode(struct device *parent, struct gendisk *disk,
> if (disk->major == BLOCK_EXT_MAJOR)
> blk_free_ext_minor(disk->first_minor);
> out_exit_elevator:
> - if (disk->queue->elevator) {
> - mutex_lock(&disk->queue->elevator_lock);
> - elevator_exit(disk->queue);
> - mutex_unlock(&disk->queue->elevator_lock);
> - }
> + elevator_set_none(disk->queue);
Same comment as above here as well but this is in add_disk code path.
Thanks,
--Nilay
