Re: [syzbot] [net?] WARNING: locking bug in tcp_tsq_handler
From: Shin'ichiro Kawasaki <hidden>
Date: 2026-08-26 02:24:05
Also in:
lkml
On Aug 25, 2026 / 16:27, Eric Dumazet wrote:
On Tue, Aug 25, 2026 at 3:53 PM Eric Dumazet [off-list ref] wrote:quoted
On Tue, Aug 25, 2026 at 3:26 PM Shin'ichiro Kawasaki [off-list ref] wrote:quoted
On Aug 25, 2026 / 14:45, Hillf Danton wrote:quoted
On Tue, 25 Aug 2026 14:44:35 +0900 Shin'ichiro Kawasaki wrote:quoted
On Aug 25, 2026 / 03:50, Eric Dumazet wrote:quoted
I think 19bdb70c77d3 should be reverted.Just reverting the commit will reintroduce the other lockdep WARN that the commit addressed. I hope to have another fix to avoid the WARN.I do not think you know the root cause of that lockdep warn, so reverting 19bdb70c77d3 is the right thing to do because of [11, 12]. [11] ffa1e7ada456 ("block: Make request_queue lockdep splats show up earlier") [12] Subject: Re: [PATCH] nbd: don't warn when reclassifying a busy socket lock https://lore.kernel.org/lkml/20260816014906.1149-1-hdanton@sina.com/ (local)I took a look in the commit [11] and the discussion [12], but couldn't follow why the commit 19bdb70c77d3 should be reverted. Could you elaborate a bit more about the reason you suggest the revert?I think you missed one problem in lib/once.c Revert commit e8eef69a99f1 (or restore once_disable_jump(once_key, mod) inside __do_once_sleepable_done()) so static_branch_disable() is never called synchronously under caller locks. Previously, once_disable_jump() deferred static_branch_disable() to a worker thread via schedule_work(). Commit e8eef69a99f1 made it run synchronously in the caller's context. Because __inet_hash_connect() is called under lock_sock(sk), calling static_branch_disable() directly inside DO_ONCE_SLEEPABLE() takes cpus_read_lock() (cpu_hotplug_lock) while holding lock_sock(sk). This created the bogus lock dependency sk_lock -> cpu_hotplug_lock. Every other storage/networking client (sunrpc, nbd, cifs, iscsi_tcp, rxe, siw) uses static lockdep keys (static struct lock_class_key ...[2]) without issue.I sent the revert request for review : https://lore.kernel.org/lkml/20260825142515.1965654-1-edumazet@google.com/T/#u (local)
Thank you for the clarification. Now I have better understanding. When I looked at the lock dependency chain among six locks below, I assumed that any of the dependencies could not be cut. set->srcu -> sk_lock -> cpu_hotplug_lock -> fs_reclaim -> q_usage_counter -> elevator_lock -> set->srcu So I thought that the idea to introduce the dynamic lockdep key for nvme-tcp sk_lock instances would be the only one solution. But you pointed out that the dependency sk_lock -> cpu_hotplug_lock can be cut by reverting the commit e8eef69a99f1. I did trial and confirmed that your suggestion works. With the kerenl v7.2 + revert 19bdb70c77d3 + revert e8eef69a99f1, I confirmed the blktests test case nvme/005 passed with tcp transport. The lockdep WARN due to the six locks goes away. Great :) Assuming e8eef69a99f1 will be reverted, I agree to revert 19bdb70c77d3 also.
quoted
Use my pending TCP patch, which is a no brainer : https://lore.kernel.org/netdev/20260825023614.1228551-1-edumazet@google.com/ (local)
IIUC, this patch uses GFP_ATOMIC to cut the dependency sk_lock -> fs_reclaim. I did another trial: using the kerenl v7.2 + revert 19bdb70c77d3 + the TCP patch "tcp: use GFP_ATOMIC in tcp_send_active_reset()", I still observed the blktests test case nvme/005 failed with the lockdep WARN due to the six locks. Hmm. The TCP patch cuts the direct dependency sk_lock -> fs_reclaim, but the depdency chain I face has indirect dependency between sk_lock and fs_reclaim: ... sk_lock -> cpu_hotplug_lock -> fs_reclaim ... I think this explains why the TCP patch does not fix the lockdep WARN due to the six locks.
quoted
Revert 19bdb70c77d3 in drivers/nvme/host/tcp.c : Restore static lockdep keys nvme_tcp_sk_key[2] and nvme_tcp_slock_key[2]. And we should be good. If you think each nvme-tcp socket MUST have its own LOCKDEP class, please elaborate, because I have no idea why this would be needed.
As I noted above, if e8eef69a99f1 will be reverted, I do not have reason to make nvme-tcp socket have its owy dynamic lockdep key and class. Let me wait and see how the e8eef69a99f1 revert patch will go. If it goes well, I will send out the patch to revert 19bdb70c77d3.