From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 16CFE1D5CEA; Wed, 11 Feb 2026 12:31:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770813119; cv=none; b=nQ6qslHSCpIBrR0B/uo9ggCPWzl2lH692zldPfsDBsJt6mZ7a1pvz5rcaJA6R6bouKgPqJTxseTdsyK70q+qE7AxOM2RlztAvnSt9OM2e8Jt7u/J6/1vkeFQyPNmxvw1iV89qbDG19bQsovfU/Px0fNZUjoCpMY8OmMPeJs/B8E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770813119; c=relaxed/simple; bh=W6lMWwQYpj38AyccgL7rBPPw/3iRhrUGaZC7plCayeY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=ds3dg5MsZJYOeFB+doEk+iKBipNe+BLHQUE2KOG0fETn1ZOBwCt88sBNLSq1RZnlu7Vp08zLv9f6+KOtvvBCPqA+0hzRWluWRDwgt3c/qSKvB7Bsu8diKzV4cz8FIhJYJvy9LQhOB82VbXOQn6QVzko3y3wLiynDx1fDGKtchhk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=FOxXVuty; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="FOxXVuty" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0BAD4C4CEF7; Wed, 11 Feb 2026 12:31:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1770813119; bh=W6lMWwQYpj38AyccgL7rBPPw/3iRhrUGaZC7plCayeY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FOxXVutycOFUrt8I+5ZBkOd3fvL5fgFnznZqxRiL8e68c7ScMBqhKp2+UZIfI6QjT p00GZIJe3cUcNDY8DXBKtVJVuUgeaicRwaPOg1t+72iULSozrBjqfGFfSLplaKubuK 0EEE1dx29Cv4oQ5PWfdQOncKgJypwdOflDJDMvn66XDBNGZ+jS4KjWxwJozabvyb7P Fdb4yOfMRi0mrezhDmfAu6nxV0qLNBTg8u2fmM1ARTfCM/OgOqvbbU0TfgiBD3G/w2 bixd3x8IWiClJ7Q2e3RUoxLFi/HkGhamYNOqGkEtYnpFmyc/2RBIblK716kkJtPZJz vm8obLX5Pv9qw== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Yu Kuai , Nilay Shroff , Ming Lei , Hannes Reinecke , Jens Axboe , Sasha Levin , linux-block@vger.kernel.org Subject: [PATCH AUTOSEL 6.19-5.10] blk-mq-debugfs: add missing debugfs_mutex in blk_mq_debugfs_register_hctxs() Date: Wed, 11 Feb 2026 07:30:33 -0500 Message-ID: <20260211123112.1330287-23-sashal@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260211123112.1330287-1-sashal@kernel.org> References: <20260211123112.1330287-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: linux-block@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.19 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Yu Kuai [ Upstream commit 9d20fd6ce1ba9733cd5ac96fcab32faa9fc404dd ] In blk_mq_update_nr_hw_queues(), debugfs_mutex is not held while creating debugfs entries for hctxs. Hence add debugfs_mutex there, it's safe because queue is not frozen. Signed-off-by: Yu Kuai Reviewed-by: Nilay Shroff Reviewed-by: Ming Lei Reviewed-by: Hannes Reinecke Signed-off-by: Jens Axboe Signed-off-by: Sasha Levin --- LLM Generated explanations, may be completely bogus: Now I have a comprehensive picture. Let me compile the full analysis. --- ## Complete Analysis ### 1. COMMIT MESSAGE ANALYSIS The commit title is clear: "add missing debugfs_mutex in blk_mq_debugfs_register_hctxs()". The body explicitly describes the problem: `debugfs_mutex` is not held while creating debugfs entries for hctxs during `blk_mq_update_nr_hw_queues()`. The author (Yu Kuai) notes it's safe to add the mutex there because "queue is not frozen" at that point (meaning there's no deadlock risk with freeze-related locks). Three reviewers signed off: Nilay Shroff (IBM), Ming Lei (Red Hat), and Hannes Reinecke (SUSE) -- all experienced block layer developers. The merge was by Jens Axboe (block layer maintainer). ### 2. CODE CHANGE ANALYSIS The diff is minimal -- **3 lines added**: ```c mutex_lock(&q->debugfs_mutex); queue_for_each_hw_ctx(q, hctx, i) blk_mq_debugfs_register_hctx(q, hctx); mutex_unlock(&q->debugfs_mutex); ``` #### The Bug Mechanism The `debugfs_mutex` was introduced in commit `5cf9c91ba927` ("block: serialize all debugfs operations using q->debugfs_mutex", v5.19) by Christoph Hellwig. That commit explicitly stated the goal: *"Use the existing debugfs_mutex to serialize all debugfs operations that rely on q->debugfs_dir or the directories hanging off it."* That commit added `lockdep_assert_held(&q->debugfs_mutex)` in many functions: - `blk_mq_debugfs_register_sched()` (line 706) - `blk_mq_debugfs_unregister_sched()` (line 725) - `blk_mq_debugfs_unregister_rqos()` (line 746) - `blk_mq_debugfs_register_rqos()` (line 759) - `blk_mq_debugfs_register_sched_hctx()` (line 777) - `blk_mq_debugfs_unregister_sched_hctx()` (line 798) But it missed `blk_mq_debugfs_register_hctxs()`, which is called from `__blk_mq_update_nr_hw_queues()` at line 5165 of `block/blk-mq.c` **without** holding `debugfs_mutex`. All other callers properly hold the mutex: - `blk_register_queue()` in `blk-sysfs.c` holds `debugfs_mutex` when calling `blk_mq_debugfs_register()` (which internally calls `blk_mq_debugfs_register_hctx()`) - `blk_mq_sched_reg_debugfs()` in `blk-mq-sched.c` holds `debugfs_mutex` when calling sched debugfs registration - `rq_qos_add()` in `blk-rq-qos.c` holds `debugfs_mutex` when registering rqos debugfs #### The Race The race is between two concurrent paths: **Thread A** -- `__blk_mq_update_nr_hw_queues()` → `blk_mq_debugfs_register_hctxs()` → `blk_mq_debugfs_register_hctx()`: ```656:673:block/blk-mq-debugfs.c void blk_mq_debugfs_register_hctx(struct request_queue *q, struct blk_mq_hw_ctx *hctx) { struct blk_mq_ctx *ctx; char name[20]; int i; if (!q->debugfs_dir) return; snprintf(name, sizeof(name), "hctx%u", hctx->queue_num); hctx->debugfs_dir = debugfs_create_dir(name, q->debugfs_dir); // ... creates more debugfs files ... } ``` **Thread B** -- `blk_unregister_queue()` → `blk_debugfs_remove()`: ```884:895:block/blk-sysfs.c static void blk_debugfs_remove(struct gendisk *disk) { struct request_queue *q = disk->queue; mutex_lock(&q->debugfs_mutex); blk_trace_shutdown(q); debugfs_remove_recursive(q->debugfs_dir); q->debugfs_dir = NULL; // ... mutex_unlock(&q->debugfs_mutex); } ``` Without the mutex in Thread A: 1. Thread A checks `q->debugfs_dir` (line 663) -- not NULL, proceeds 2. Thread B acquires `debugfs_mutex`, removes `q->debugfs_dir`, sets it to NULL 3. Thread A uses the now-stale/freed `q->debugfs_dir` to create child entries (line 667) This can result in orphaned debugfs entries, inconsistent debugfs state, and potentially use of a freed dentry. ### 3. CLASSIFICATION This is a **synchronization bug fix** -- adding a missing lock acquisition in a path that was accidentally omitted when the locking scheme was introduced. It completes the locking protocol established in commit `5cf9c91ba927`. ### 4. SCOPE AND RISK - **Size**: 3 lines added (+2 mutex_lock/unlock, consistent with existing pattern) - **Files touched**: 1 file (`block/blk-mq-debugfs.c`) - **Subsystem**: Block layer (blk-mq) -- core infrastructure used by all block device drivers - **Risk**: Extremely low. The mutex is already held by all other paths. The commit message explicitly addresses deadlock safety ("queue is not frozen") - **Pattern match**: 100% consistent with how every other debugfs operation in the block layer acquires this lock ### 5. USER IMPACT The `__blk_mq_update_nr_hw_queues()` path is triggered during: - CPU hotplug events (adding/removing CPUs changes hw queue count) - Driver-initiated hw queue resizing (e.g., `null_blk` via configfs `submit_queues`) - SCSI/NVMe driver queue changes This means any system doing CPU hotplug or block device reconfiguration could hit this race. The impact is concurrent with disk registration/unregistration, which happens during device discovery and removal -- both common operations. ### 6. STABILITY INDICATORS - **3 Reviewed-by tags** from major Linux storage developers - **Merged by Jens Axboe** (block layer maintainer) - Part of a series of debugfs locking improvements to the block layer (see commit `0e94ed336814` protecting `hctx_busy_show`) ### 7. DEPENDENCY CHECK - **Dependencies**: Requires `debugfs_mutex` in `struct request_queue`, which has been present since v5.19 (commit `5cf9c91ba927`) - **Self-contained**: The fix is entirely self-contained -- no other patches required - **Applies to**: All stable trees from 5.19 onwards (6.1.y, 6.6.y, 6.12.y, etc.) ### Summary This commit fixes a missing lock acquisition in `blk_mq_debugfs_register_hctxs()` that was overlooked when the `debugfs_mutex` serialization scheme was introduced in v5.19. Every other debugfs operation in the block layer properly holds this lock. The fix is: - **3 lines** of trivially correct code - **Zero risk** of regression (follows established pattern exactly) - **Well-reviewed** by 3 experienced block layer developers - **Fixes a real race** between hw queue updates and debugfs teardown - **Self-contained** with no dependencies beyond what's in all affected stable trees The fix is small, surgical, obviously correct, and meets all stable kernel criteria. **YES** block/blk-mq-debugfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c index 4896525b1c054..553d93b88e194 100644 --- a/block/blk-mq-debugfs.c +++ b/block/blk-mq-debugfs.c @@ -686,8 +686,10 @@ void blk_mq_debugfs_register_hctxs(struct request_queue *q) struct blk_mq_hw_ctx *hctx; unsigned long i; + mutex_lock(&q->debugfs_mutex); queue_for_each_hw_ctx(q, hctx, i) blk_mq_debugfs_register_hctx(q, hctx); + mutex_unlock(&q->debugfs_mutex); } void blk_mq_debugfs_unregister_hctxs(struct request_queue *q) -- 2.51.0