Re: [PATCH 15/15] block: move wbt_enable_default() out of queue freezing from scheduler's ->exit()

[Ming Lei <ming.lei@redhat.com>]

On Fri, Apr 11, 2025 at 12:50:42AM +0530, Nilay Shroff wrote:
> 
> 
> On 4/10/25 7:00 PM, Ming Lei wrote:
> > scheduler's ->exit() is called with queue frozen and elevator lock is held, and
> > wbt_enable_default() can't be called with queue frozen, otherwise the
> > following lockdep warning is triggered:
> > 
> > 	#6 (&q->rq_qos_mutex){+.+.}-{4:4}:
> > 	#5 (&eq->sysfs_lock){+.+.}-{4:4}:
> > 	#4 (&q->elevator_lock){+.+.}-{4:4}:
> > 	#3 (&q->q_usage_counter(io)#3){++++}-{0:0}:
> > 	#2 (fs_reclaim){+.+.}-{0:0}:
> > 	#1 (&sb->s_type->i_mutex_key#3){+.+.}-{4:4}:
> > 	#0 (&q->debugfs_mutex){+.+.}-{4:4}:
> > 
> > Fix the issue by moving wbt_enable_default() out of bfq's exit(), and
> > call it from elevator_change_done().
> 
> [...]
> 
> > diff --git a/block/elevator.c b/block/elevator.c
> > index 1cc640a9db3e..9cf78db4d6a4 100644
> > --- a/block/elevator.c
> > +++ b/block/elevator.c
> > @@ -676,8 +676,13 @@ int elevator_change_done(struct request_queue *q, struct elev_change_ctx *ctx)
> >  	int ret = 0;
> >  
> >  	if (ctx->old) {
> > +		bool enable_wbt = test_bit(ELEVATOR_FLAG_ENABLE_WBT_ON_EXIT,
> > +				&ctx->old->flags);
> > +
> >  		elv_unregister_queue(q, ctx->old);
> >  		kobject_put(&ctx->old->kobj);
> > +		if (enable_wbt)
> > +			wbt_enable_default(q->disk);
> >  	}
> wbt_enable_default is also called from ioc_qos_write and blk_register_queue 
> with ->elevator_lock protection. So avoiding protection here doesn't seem
> correct.

The only code which needs the protection should be:

```
wbt_enable_default():
	...
	if (q->elevator && test_bit(ELEVATOR_FLAG_DISABLE_WBT, &q->elevator->flags))
		enable = false;

```

The flag can be moved to request queue from elevator queue, will do it in
V2.


Thanks,
Ming