Commit 9869d3a6 authored by Ming Lei's avatar Ming Lei Committed by Jens Axboe
Browse files

block: fix race between wbt_enable_default and IO submission 



When wbt_enable_default() is moved out of queue freezing in elevator_change(),
it can cause the wbt inflight counter to become negative (-1), leading to hung
tasks in the writeback path. Tasks get stuck in wbt_wait() because the counter
is in an inconsistent state.

The issue occurs because wbt_enable_default() could race with IO submission,
allowing the counter to be decremented before proper initialization. This manifests
as:

  rq_wait[0]:
    inflight:             -1
    has_waiters:        True

rwb_enabled() checks the state, which can be updated exactly between wbt_wait()
(rq_qos_throttle()) and wbt_track()(rq_qos_track()), then the inflight counter
will become negative.

And results in hung task warnings like:
  task:kworker/u24:39 state:D stack:0 pid:14767
  Call Trace:
    rq_qos_wait+0xb4/0x150
    wbt_wait+0xa9/0x100
    __rq_qos_throttle+0x24/0x40
    blk_mq_submit_bio+0x672/0x7b0
    ...

Fix this by:

1. Splitting wbt_enable_default() into:
   - __wbt_enable_default(): Returns true if wbt_init() should be called
   - wbt_enable_default(): Wrapper for existing callers (no init)
   - wbt_init_enable_default(): New function that checks and inits WBT

2. Using wbt_init_enable_default() in blk_register_queue() to ensure
   proper initialization during queue registration

3. Move wbt_init() out of wbt_enable_default() which is only for enabling
   disabled wbt from bfq and iocost, and wbt_init() isn't needed. Then the
   original lock warning can be avoided.

4. Removing the ELEVATOR_FLAG_ENABLE_WBT_ON_EXIT flag and its handling
   code since it's no longer needed

This ensures WBT is properly initialized before any IO can be submitted,
preventing the counter from going negative.

Cc: Nilay Shroff <nilay@linux.ibm.com>
Cc: Yu Kuai <yukuai@fnnas.com>
Cc: Guangwu Zhang <guazhang@redhat.com>
Fixes: 78c27134 ("block: move wbt_enable_default() out of queue freezing from sched ->exit()")
Signed-off-by: default avatarMing Lei <ming.lei@redhat.com>
Reviewed-by: default avatarNilay Shroff <nilay@linux.ibm.com>
Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 63276182

block/bfq-iosched.c

+1 −1
Original line number Diff line number Diff line
@@ -7181,7 +7181,7 @@ static void bfq_exit_queue(struct elevator_queue *e) 


	blk_stat_disable_accounting(bfqd->queue);

	blk_queue_flag_clear(QUEUE_FLAG_DISABLE_WBT_DEF, bfqd->queue);

	set_bit(ELEVATOR_FLAG_ENABLE_WBT_ON_EXIT, &e->flags);

	wbt_enable_default(bfqd->queue->disk);



	kfree(bfqd);

}

block/blk-sysfs.c

+1 −1
Original line number Diff line number Diff line
@@ -932,7 +932,7 @@ int blk_register_queue(struct gendisk *disk) 
		elevator_set_default(q);



	blk_queue_flag_set(QUEUE_FLAG_REGISTERED, q);

	wbt_enable_default(disk);

	wbt_init_enable_default(disk);



	/* Now everything is ready and send out KOBJ_ADD uevent */

	kobject_uevent(&disk->queue_kobj, KOBJ_ADD);

block/blk-wbt.c

+16 −4
Original line number Diff line number Diff line
@@ -699,7 +699,7 @@ static void wbt_requeue(struct rq_qos *rqos, struct request *rq) 
/*

 * Enable wbt if defaults are configured that way

 */

void wbt_enable_default(struct gendisk *disk)

static bool __wbt_enable_default(struct gendisk *disk)

{

	struct request_queue *q = disk->queue;

	struct rq_qos *rqos;
@@ -716,19 +716,31 @@ void wbt_enable_default(struct gendisk *disk) 
		if (enable && RQWB(rqos)->enable_state == WBT_STATE_OFF_DEFAULT)

			RQWB(rqos)->enable_state = WBT_STATE_ON_DEFAULT;

		mutex_unlock(&disk->rqos_state_mutex);

		return;

		return false;

	}

	mutex_unlock(&disk->rqos_state_mutex);



	/* Queue not registered? Maybe shutting down... */

	if (!blk_queue_registered(q))

		return;

		return false;



	if (queue_is_mq(q) && enable)

		wbt_init(disk);

		return true;

	return false;

}



void wbt_enable_default(struct gendisk *disk)

{

	__wbt_enable_default(disk);

}

EXPORT_SYMBOL_GPL(wbt_enable_default);



void wbt_init_enable_default(struct gendisk *disk)

{

	if (__wbt_enable_default(disk))

		WARN_ON_ONCE(wbt_init(disk));

}



u64 wbt_default_latency_nsec(struct request_queue *q)

{

	/*

block/blk-wbt.h

+5 −0
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ 
#ifdef CONFIG_BLK_WBT



int wbt_init(struct gendisk *disk);

void wbt_init_enable_default(struct gendisk *disk);

void wbt_disable_default(struct gendisk *disk);

void wbt_enable_default(struct gendisk *disk);
@@ -16,6 +17,10 @@ u64 wbt_default_latency_nsec(struct request_queue *); 


#else



static inline void wbt_init_enable_default(struct gendisk *disk)

{

}



static inline void wbt_disable_default(struct gendisk *disk)

{

}

block/elevator.c

+0 −4
Original line number Diff line number Diff line
@@ -633,14 +633,10 @@ static int elevator_change_done(struct request_queue *q, 
			.et = ctx->old->et,

			.data = ctx->old->elevator_data

		};

		bool enable_wbt = test_bit(ELEVATOR_FLAG_ENABLE_WBT_ON_EXIT,

				&ctx->old->flags);



		elv_unregister_queue(q, ctx->old);

		blk_mq_free_sched_res(&res, ctx->old->type, q->tag_set);

		kobject_put(&ctx->old->kobj);

		if (enable_wbt)

			wbt_enable_default(q->disk);

	}

	if (ctx->new) {

		ret = elv_register_queue(q, ctx->new, !ctx->no_uevent);