Skip to content

Commit 5989bfe

Browse files
shroffniaxboe
shroffni
authored and
axboe
committed
block: restore two stage elevator switch while running nr_hw_queue update
The kmemleak reports memory leaks related to elevator resources that were originally allocated in the ->init_hctx() method. The following leak traces are observed after running blktests block/040: unreferenced object 0xffff8881b82f7400 (size 512): comm "check", pid 68454, jiffies 4310588881 hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace (crc 5bac8b34): __kvmalloc_node_noprof+0x55d/0x7a0 sbitmap_init_node+0x15a/0x6a0 kyber_init_hctx+0x316/0xb90 blk_mq_init_sched+0x419/0x580 elevator_switch+0x18b/0x630 elv_update_nr_hw_queues+0x219/0x2c0 __blk_mq_update_nr_hw_queues+0x36a/0x6f0 blk_mq_update_nr_hw_queues+0x3a/0x60 0xffffffffc09ceb80 0xffffffffc09d7e0b configfs_write_iter+0x2b1/0x470 vfs_write+0x527/0xe70 ksys_write+0xff/0x200 do_syscall_64+0x98/0x3c0 entry_SYSCALL_64_after_hwframe+0x76/0x7e unreferenced object 0xffff8881b82f6000 (size 512): comm "check", pid 68454, jiffies 4310588881 hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace (crc 5bac8b34): __kvmalloc_node_noprof+0x55d/0x7a0 sbitmap_init_node+0x15a/0x6a0 kyber_init_hctx+0x316/0xb90 blk_mq_init_sched+0x419/0x580 elevator_switch+0x18b/0x630 elv_update_nr_hw_queues+0x219/0x2c0 __blk_mq_update_nr_hw_queues+0x36a/0x6f0 blk_mq_update_nr_hw_queues+0x3a/0x60 0xffffffffc09ceb80 0xffffffffc09d7e0b configfs_write_iter+0x2b1/0x470 vfs_write+0x527/0xe70 ksys_write+0xff/0x200 do_syscall_64+0x98/0x3c0 entry_SYSCALL_64_after_hwframe+0x76/0x7e unreferenced object 0xffff8881b82f5800 (size 512): comm "check", pid 68454, jiffies 4310588881 hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ backtrace (crc 5bac8b34): __kvmalloc_node_noprof+0x55d/0x7a0 sbitmap_init_node+0x15a/0x6a0 kyber_init_hctx+0x316/0xb90 blk_mq_init_sched+0x419/0x580 elevator_switch+0x18b/0x630 elv_update_nr_hw_queues+0x219/0x2c0 __blk_mq_update_nr_hw_queues+0x36a/0x6f0 blk_mq_update_nr_hw_queues+0x3a/0x60 0xffffffffc09ceb80 0xffffffffc09d7e0b configfs_write_iter+0x2b1/0x470 vfs_write+0x527/0xe70 ksys_write+0xff/0x200 do_syscall_64+0x98/0x3c0 entry_SYSCALL_64_after_hwframe+0x76/0x7e The issue arises while we run nr_hw_queue update, Specifically, we first reallocate hardware contexts (hctx) via __blk_mq_realloc_hw_ctxs(), and then later invoke elevator_switch() (assuming q->elevator is not NULL). The elevator switch code would first exit old elevator (elevator_exit) and then switches to the new elevator. The elevator_exit loops through each hctx and invokes the elevator’s per-hctx exit method ->exit_hctx(), which releases resources allocated during ->init_hctx(). This memleak manifests when we reduce the num of h/w queues - for example, when the initial update sets the number of queues to X, and a later update reduces it to Y, where Y < X. In this case, we'd loose the access to old hctxs while we get to elevator exit code because __blk_mq_realloc_hw_ctxs would have already released the old hctxs. As we don't now have any reference left to the old hctxs, we don't have any way to free the scheduler resources (which are allocate in ->init_hctx()) and kmemleak complains about it. This issue was caused due to the commit 596dce1 ("block: simplify elevator reattachment for updating nr_hw_queues"). That change unified the two-stage elevator teardown and reattachment into a single call that occurs after __blk_mq_realloc_hw_ctxs() has already freed the hctxs. This patch restores the previous two-stage elevator switch logic during nr_hw_queues updates. First, the elevator is switched to 'none', which ensures all scheduler resources are properly freed. Then, the hardware contexts (hctxs) are reallocated, and the software-to-hardware queue mappings are updated. Finally, the original elevator is reattached. This sequence prevents loss of references to old hctxs and avoids the scheduler resource leaks reported by kmemleak. Reported-by : Yi Zhang <yi.zhang@redhat.com> Fixes: 596dce1 ("block: simplify elevator reattachment for updating nr_hw_queues") Closes: https://lore.kernel.org/all/CAHj4cs8oJFvz=daCvjHM5dYCNQH4UXwSySPPU4v-WHce_kZXZA@mail.gmail.com/ Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> Reviewed-by: Yu Kuai <yukuai3@huawei.com> Link: https://lore.kernel.org/r/20250724102540.1366308-1-nilay@linux.ibm.com Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 5ec9d26 commit 5989bfe

3 files changed

Lines changed: 81 additions & 15 deletions

File tree

‎block/blk-mq.c‎

Lines changed: 75 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4970,13 +4970,68 @@ int blk_mq_update_nr_requests(struct request_queue *q, unsigned int nr)
49704970
return ret;
49714971
}
49724972

4973+
/*
4974+
* Switch back to the elevator type stored in the xarray.
4975+
*/
4976+
static void blk_mq_elv_switch_back(struct request_queue *q,
4977+
struct xarray *elv_tbl)
4978+
{
4979+
struct elevator_type *e = xa_load(elv_tbl, q->id);
4980+
4981+
/* The elv_update_nr_hw_queues unfreezes the queue. */
4982+
elv_update_nr_hw_queues(q, e);
4983+
4984+
/* Drop the reference acquired in blk_mq_elv_switch_none. */
4985+
if (e)
4986+
elevator_put(e);
4987+
}
4988+
4989+
/*
4990+
* Stores elevator type in xarray and set current elevator to none. It uses
4991+
* q->id as an index to store the elevator type into the xarray.
4992+
*/
4993+
static int blk_mq_elv_switch_none(struct request_queue *q,
4994+
struct xarray *elv_tbl)
4995+
{
4996+
int ret = 0;
4997+
4998+
lockdep_assert_held_write(&q->tag_set->update_nr_hwq_lock);
4999+
5000+
/*
5001+
* Accessing q->elevator without holding q->elevator_lock is safe here
5002+
* because we're called from nr_hw_queue update which is protected by
5003+
* set->update_nr_hwq_lock in the writer context. So, scheduler update/
5004+
* switch code (which acquires the same lock in the reader context)
5005+
* can't run concurrently.
5006+
*/
5007+
if (q->elevator) {
5008+
5009+
ret = xa_insert(elv_tbl, q->id, q->elevator->type, GFP_KERNEL);
5010+
if (WARN_ON_ONCE(ret))
5011+
return ret;
5012+
5013+
/*
5014+
* Before we switch elevator to 'none', take a reference to
5015+
* the elevator module so that while nr_hw_queue update is
5016+
* running, no one can remove elevator module. We'd put the
5017+
* reference to elevator module later when we switch back
5018+
* elevator.
5019+
*/
5020+
__elevator_get(q->elevator->type);
5021+
5022+
elevator_set_none(q);
5023+
}
5024+
return ret;
5025+
}
5026+
49735027
static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
49745028
int nr_hw_queues)
49755029
{
49765030
struct request_queue *q;
49775031
int prev_nr_hw_queues = set->nr_hw_queues;
49785032
unsigned int memflags;
49795033
int i;
5034+
struct xarray elv_tbl;
49805035

49815036
lockdep_assert_held(&set->tag_list_lock);
49825037

@@ -4988,6 +5043,9 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
49885043
return;
49895044

49905045
memflags = memalloc_noio_save();
5046+
5047+
xa_init(&elv_tbl);
5048+
49915049
list_for_each_entry(q, &set->tag_list, tag_set_list) {
49925050
blk_mq_debugfs_unregister_hctxs(q);
49935051
blk_mq_sysfs_unregister_hctxs(q);
@@ -4996,11 +5054,17 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
49965054
list_for_each_entry(q, &set->tag_list, tag_set_list)
49975055
blk_mq_freeze_queue_nomemsave(q);
49985056

4999-
if (blk_mq_realloc_tag_set_tags(set, nr_hw_queues) < 0) {
5000-
list_for_each_entry(q, &set->tag_list, tag_set_list)
5001-
blk_mq_unfreeze_queue_nomemrestore(q);
5002-
goto reregister;
5003-
}
5057+
/*
5058+
* Switch IO scheduler to 'none', cleaning up the data associated
5059+
* with the previous scheduler. We will switch back once we are done
5060+
* updating the new sw to hw queue mappings.
5061+
*/
5062+
list_for_each_entry(q, &set->tag_list, tag_set_list)
5063+
if (blk_mq_elv_switch_none(q, &elv_tbl))
5064+
goto switch_back;
5065+
5066+
if (blk_mq_realloc_tag_set_tags(set, nr_hw_queues) < 0)
5067+
goto switch_back;
50045068

50055069
fallback:
50065070
blk_mq_update_queue_map(set);
@@ -5020,19 +5084,21 @@ static void __blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set,
50205084
}
50215085
blk_mq_map_swqueue(q);
50225086
}
5023-
5024-
/* elv_update_nr_hw_queues() unfreeze queue for us */
5087+
switch_back:
5088+
/* The blk_mq_elv_switch_back unfreezes queue for us. */
50255089
list_for_each_entry(q, &set->tag_list, tag_set_list)
5026-
elv_update_nr_hw_queues(q);
5090+
blk_mq_elv_switch_back(q, &elv_tbl);
50275091

5028-
reregister:
50295092
list_for_each_entry(q, &set->tag_list, tag_set_list) {
50305093
blk_mq_sysfs_register_hctxs(q);
50315094
blk_mq_debugfs_register_hctxs(q);
50325095

50335096
blk_mq_remove_hw_queues_cpuhp(q);
50345097
blk_mq_add_hw_queues_cpuhp(q);
50355098
}
5099+
5100+
xa_destroy(&elv_tbl);
5101+
50365102
memalloc_noio_restore(memflags);
50375103

50385104
/* Free the excess tags when nr_hw_queues shrink. */

‎block/blk.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ bool blk_bio_list_merge(struct request_queue *q, struct list_head *list,
330330

331331
bool blk_insert_flush(struct request *rq);
332332

333-
void elv_update_nr_hw_queues(struct request_queue *q);
333+
void elv_update_nr_hw_queues(struct request_queue *q, struct elevator_type *e);
334334
void elevator_set_default(struct request_queue *q);
335335
void elevator_set_none(struct request_queue *q);
336336

‎block/elevator.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -689,21 +689,21 @@ static int elevator_change(struct request_queue *q, struct elv_change_ctx *ctx)
689689
* The I/O scheduler depends on the number of hardware queues, this forces a
690690
* reattachment when nr_hw_queues changes.
691691
*/
692-
void elv_update_nr_hw_queues(struct request_queue *q)
692+
void elv_update_nr_hw_queues(struct request_queue *q, struct elevator_type *e)
693693
{
694694
struct elv_change_ctx ctx = {};
695695
int ret = -ENODEV;
696696

697697
WARN_ON_ONCE(q->mq_freeze_depth == 0);
698698

699-
mutex_lock(&q->elevator_lock);
700-
if (q->elevator && !blk_queue_dying(q) && blk_queue_registered(q)) {
701-
ctx.name = q->elevator->type->elevator_name;
699+
if (e && !blk_queue_dying(q) && blk_queue_registered(q)) {
700+
ctx.name = e->elevator_name;
702701

702+
mutex_lock(&q->elevator_lock);
703703
/* force to reattach elevator after nr_hw_queue is updated */
704704
ret = elevator_switch(q, &ctx);
705+
mutex_unlock(&q->elevator_lock);
705706
}
706-
mutex_unlock(&q->elevator_lock);
707707
blk_mq_unfreeze_queue_nomemrestore(q);
708708
if (!ret)
709709
WARN_ON_ONCE(elevator_change_done(q, &ctx));

0 commit comments

Comments
 (0)