[PATCH 0/2] nvme-tcp: do not use dynamic lockdep key for socket instances

[Nilay Shroff]

On 9/14/26 1:18 PM, Shin'ichiro Kawasaki wrote:
> From: Shin'ichiro Kawasaki <shinichiro.kawasaki at wdc.com>
> 
> Keith and the maintainers, please consider this series for upstream.
> The commit 19bdb70c77d3 tried to avoid a lockdep WARN issue, but it
> was imperfect. With Eric's help, I propose this series as a better
> solution.
> 
> Eric, FYI, I prepared the second patch with your authorship and your
> SoB tag. Thanks for the fix idea.
> 
> 
> Commit 19bdb70c77d3 ("nvme-tcp: lockdep: use dynamic lockdep keys per
> socket instance") introduced the dynamic lockdep key to avoid a lockdep
> WARN. However, it had a bug in lockdep key lifetime management and
> caused another WARN [1]. The first patch in this series reverts the
> commit to avoid the WARN.
> 
> Reverting 19bdb70c77d3 re-exposes two lockdep WARNs that it had
> suppressed. The first one was observed with the blktests test case
> nvme/005, which was caused by the lock chain below:
> 
>    set->srcu -> sk_lock -> cpu_hotplug_lock -> fs_reclaim -> q_usage_counter -> elevator_lock -> set->srcu
> 
> This WARN needs no patch in this series: it is already cut by the merged
> commit 0ba6912f7e97 by Eric in the v7.3-rc2 tag, which removes the
> "sk_lock -> cpu_hotplug_lock" dependency. This series therefore depends
> on that commit being present.
> 
> The second WARN was observed with the blktests test case nvme/062, which
> was caused by the dependency newly added for TLS support. The second
> patch in this series delays the socket reclassification timing to cut
> the dependency.
> 
> With this series applied on v7.3-rc2, nvme/005 and nvme/062 pass with no
> lockdep splat.
> 
I have just reviewed both the patch in the series and both looks good to me.

I also looked at Eric's commits 18666c73afe9 ("tcp: use GFP_ATOMIC in
tcp_send_active_reset()") and 0ba6912f7e97 ("Revert once: don't use a work
queue to reset sleepable static key"). Both look good to me.

However, reverting commit 19bdb70c77d3 ("nvme-tcp: lockdep: use dynamic lockdep
keys per socket instance") means that lockdep will no longer be able to distinguish
between the socket locks of different nvme-tcp queues. This could unnecessarily
create dependency chains between socket locks belonging to different queues,
even though those sockets are independent in practice.

So, while Eric's two commits above should address the lockdep splat reported by
blktests nvme/005 (as well as syzbot reported warning), IMO we should still teach
lockdep that sockets belonging to different nvme-tcp queues have different lock
classes. This would prevent lockdep from constructing dependency chains between
unrelated socket instances.

I agree with Eric's initial assessment that the lifetime of the nvme-tcp queue and
the lifetime of the lockdep key associated with its socket need to be tracked separately.
Based on that, perhaps we should consider introducing a separate object to track the
socket lockdep key lifetime (separate from nvme-tcp queue object), similar to what
Shin'ichiro proposed here:
https://lore.kernel.org/lkml/ao2QHIDrGeYjltX9@shinhome/

Any thoughts?

Thanks,
--Nilay