[PATCH 0/2] nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

[PATCH 0/2] nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

[Shin'ichiro Kawasaki]

When NVMe-TCP controller setup and teardown are repeated with lockdep
enabled, lockdep reports false-positive WARNs. This was observed when
running blktests nvme/005 on v7.1-rc1 with a patch [1]. Analysis by
Nilay [2] confirmed that the WARNs are false positives caused by lockdep
confusing different socket instances due to shared static lockdep keys.

This series resolves the issue by using dynamically allocated lockdep
keys per socket instance instead of static keys. This ensures lockdep
correctly tracks locks across different socket instances. For that
purpose, the first patch moves a function as preparation. The second
patch introduces the dynamic lockdep keys.

[1] https://lore.kernel.org/linux-nvme/afB5syZbUrppgsDQ@shinmob/
[2] https://lore.kernel.org/linux-nvme/c4ddc101-184a-4e4f-82ca-c3123bce5e34@linux.ibm.com/

Shin'ichiro Kawasaki (2):
  nvme-tcp: move nvme_tcp_reclassify_socket()
  nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

 drivers/nvme/host/tcp.c | 88 +++++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 39 deletions(-)

-- 
2.54.0

[PATCH 1/2] nvme-tcp: move nvme_tcp_reclassify_socket()

[Shin'ichiro Kawasaki]

Move nvme_tcp_reclassify_socket() in tcp.c after the struct
nvme_tcp_queue definition. This is preparation for adding a reference
to struct nvme_tcp_queue in the function, which would otherwise cause a
compile failure due to the struct being defined after the function.

Move the entire CONFIG_DEBUG_LOCK_ALLOC block along with the function
to maintain the code organization.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 drivers/nvme/host/tcp.c | 76 ++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 15d36d6a728e..0e5764f09f31 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -56,44 +56,6 @@ MODULE_PARM_DESC(tls_handshake_timeout,
 
 static atomic_t nvme_tcp_cpu_queues[NR_CPUS];
 
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
-/* lockdep can detect a circular dependency of the form
- *   sk_lock -> mmap_lock (page fault) -> fs locks -> sk_lock
- * because dependencies are tracked for both nvme-tcp and user contexts. Using
- * a separate class prevents lockdep from conflating nvme-tcp socket use with
- * user-space socket API use.
- */
-static struct lock_class_key nvme_tcp_sk_key[2];
-static struct lock_class_key nvme_tcp_slock_key[2];
-
-static void nvme_tcp_reclassify_socket(struct socket *sock)
-{
-	struct sock *sk = sock->sk;
-
-	if (WARN_ON_ONCE(!sock_allow_reclassification(sk)))
-		return;
-
-	switch (sk->sk_family) {
-	case AF_INET:
-		sock_lock_init_class_and_name(sk, "slock-AF_INET-NVME",
-					      &nvme_tcp_slock_key[0],
-					      "sk_lock-AF_INET-NVME",
-					      &nvme_tcp_sk_key[0]);
-		break;
-	case AF_INET6:
-		sock_lock_init_class_and_name(sk, "slock-AF_INET6-NVME",
-					      &nvme_tcp_slock_key[1],
-					      "sk_lock-AF_INET6-NVME",
-					      &nvme_tcp_sk_key[1]);
-		break;
-	default:
-		WARN_ON_ONCE(1);
-	}
-}
-#else
-static void nvme_tcp_reclassify_socket(struct socket *sock) { }
-#endif
-
 enum nvme_tcp_send_state {
 	NVME_TCP_SEND_CMD_PDU = 0,
 	NVME_TCP_SEND_H2C_PDU,
@@ -207,6 +169,44 @@ static const struct blk_mq_ops nvme_tcp_mq_ops;
 static const struct blk_mq_ops nvme_tcp_admin_mq_ops;
 static int nvme_tcp_try_send(struct nvme_tcp_queue *queue);
 
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+/* lockdep can detect a circular dependency of the form
+ *   sk_lock -> mmap_lock (page fault) -> fs locks -> sk_lock
+ * because dependencies are tracked for both nvme-tcp and user contexts. Using
+ * a separate class prevents lockdep from conflating nvme-tcp socket use with
+ * user-space socket API use.
+ */
+static struct lock_class_key nvme_tcp_sk_key[2];
+static struct lock_class_key nvme_tcp_slock_key[2];
+
+static void nvme_tcp_reclassify_socket(struct socket *sock)
+{
+	struct sock *sk = sock->sk;
+
+	if (WARN_ON_ONCE(!sock_allow_reclassification(sk)))
+		return;
+
+	switch (sk->sk_family) {
+	case AF_INET:
+		sock_lock_init_class_and_name(sk, "slock-AF_INET-NVME",
+					      &nvme_tcp_slock_key[0],
+					      "sk_lock-AF_INET-NVME",
+					      &nvme_tcp_sk_key[0]);
+		break;
+	case AF_INET6:
+		sock_lock_init_class_and_name(sk, "slock-AF_INET6-NVME",
+					      &nvme_tcp_slock_key[1],
+					      "sk_lock-AF_INET6-NVME",
+					      &nvme_tcp_sk_key[1]);
+		break;
+	default:
+		WARN_ON_ONCE(1);
+	}
+}
+#else
+static void nvme_tcp_reclassify_socket(struct socket *sock) { }
+#endif
+
 static inline struct nvme_tcp_ctrl *to_tcp_ctrl(struct nvme_ctrl *ctrl)
 {
 	return container_of(ctrl, struct nvme_tcp_ctrl, ctrl);
-- 
2.54.0

[PATCH 2/2] nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

[Shin'ichiro Kawasaki]

When NVMe-TCP controller setup and teardown are repeated with lockdep
enabled, lockdep reports false positives WARN for the following locks:

  1) &q->elevator_lock        : IO scheduler change context
  2) &q->q_usage_counter(io)  : SCSI disk probe context
  3) fs_reclaim               : CPU hotplug bring-up context
  4) cpu_hotplug_lock         : socket establishment context
  5) sk_lock-AF_INET-NVME     : MQ sched dispatch context for the socket
  6) set->srcu                : NVMe controller delete context

The lockdep WARN was observed by running blktests test case nvme/005 for
tcp transport on v7.1-rc1 kernel with a patch. Refer to the Link tag for
the details of the WARN.

This is a false positive because lockdep confuses lock 4) (socket
establishment) with lock 5) (socket in use) for different socket
instances. The locks belong to different sockets, but lockdep treats
them as the same due to shared static lockdep keys.

Fix this by using dynamically allocated lockdep keys per socket instance
instead of static keys nvme_tcp_sk_key[] and nvme_tcp_slock_key[]. Add
nvme_tcp_sk_key and nvme_tcp_slock_key fields to struct nvme_tcp_queue
and pass them to sock_lock_init_class_and_name() for proper lockdep
tracking. Change the arguement of nvme_tcp_reclassify_socket() from
'struct socket *' to 'struct nvme_tcp_queue *' to pass both the socket
and the keys.

Link: https://lore.kernel.org/linux-nvme/afB5syZbUrppgsDQ@shinmob/
Suggested-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 drivers/nvme/host/tcp.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 0e5764f09f31..51d496f414a1 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -142,6 +142,11 @@ struct nvme_tcp_queue {
 	void (*state_change)(struct sock *);
 	void (*data_ready)(struct sock *);
 	void (*write_space)(struct sock *);
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	struct lock_class_key nvme_tcp_sk_key;
+	struct lock_class_key nvme_tcp_slock_key;
+#endif
 };
 
 struct nvme_tcp_ctrl {
@@ -176,12 +181,12 @@ static int nvme_tcp_try_send(struct nvme_tcp_queue *queue);
  * a separate class prevents lockdep from conflating nvme-tcp socket use with
  * user-space socket API use.
  */
-static struct lock_class_key nvme_tcp_sk_key[2];
-static struct lock_class_key nvme_tcp_slock_key[2];
-
-static void nvme_tcp_reclassify_socket(struct socket *sock)
+static void nvme_tcp_reclassify_socket(struct nvme_tcp_queue *queue)
 {
-	struct sock *sk = sock->sk;
+	struct sock *sk = queue->sock->sk;
+
+	lockdep_register_key(&queue->nvme_tcp_sk_key);
+	lockdep_register_key(&queue->nvme_tcp_slock_key);
 
 	if (WARN_ON_ONCE(!sock_allow_reclassification(sk)))
 		return;
@@ -189,22 +194,22 @@ static void nvme_tcp_reclassify_socket(struct socket *sock)
 	switch (sk->sk_family) {
 	case AF_INET:
 		sock_lock_init_class_and_name(sk, "slock-AF_INET-NVME",
-					      &nvme_tcp_slock_key[0],
+					      &queue->nvme_tcp_slock_key,
 					      "sk_lock-AF_INET-NVME",
-					      &nvme_tcp_sk_key[0]);
+					      &queue->nvme_tcp_sk_key);
 		break;
 	case AF_INET6:
 		sock_lock_init_class_and_name(sk, "slock-AF_INET6-NVME",
-					      &nvme_tcp_slock_key[1],
+					      &queue->nvme_tcp_slock_key,
 					      "sk_lock-AF_INET6-NVME",
-					      &nvme_tcp_sk_key[1]);
+					      &queue->nvme_tcp_sk_key);
 		break;
 	default:
 		WARN_ON_ONCE(1);
 	}
 }
 #else
-static void nvme_tcp_reclassify_socket(struct socket *sock) { }
+static void nvme_tcp_reclassify_socket(struct nvme_tcp_queue *queue) { }
 #endif
 
 static inline struct nvme_tcp_ctrl *to_tcp_ctrl(struct nvme_ctrl *ctrl)
@@ -1468,6 +1473,11 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
 	kfree(queue->pdu);
 	mutex_destroy(&queue->send_mutex);
 	mutex_destroy(&queue->queue_lock);
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+	lockdep_unregister_key(&queue->nvme_tcp_sk_key);
+	lockdep_unregister_key(&queue->nvme_tcp_slock_key);
+#endif
 }
 
 static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
@@ -1813,7 +1823,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
 	}
 
 	sk_net_refcnt_upgrade(queue->sock->sk);
-	nvme_tcp_reclassify_socket(queue->sock);
+	nvme_tcp_reclassify_socket(queue);
 
 	/* Single syn retry */
 	tcp_sock_set_syncnt(queue->sock->sk, 1);
-- 
2.54.0

Re: [PATCH 1/2] nvme-tcp: move nvme_tcp_reclassify_socket()

[Nilay Shroff]

On 5/30/26 10:50 AM, Shin'ichiro Kawasaki wrote:
> Move nvme_tcp_reclassify_socket() in tcp.c after the struct
> nvme_tcp_queue definition. This is preparation for adding a reference
> to struct nvme_tcp_queue in the function, which would otherwise cause a
> compile failure due to the struct being defined after the function.
> 
> Move the entire CONFIG_DEBUG_LOCK_ALLOC block along with the function
> to maintain the code organization.
> 
> Signed-off-by: Shin'ichiro Kawasaki<shinichiro.kawasaki@wdc.com>
> ---
>   drivers/nvme/host/tcp.c | 76 ++++++++++++++++++++---------------------
>   1 file changed, 38 insertions(+), 38 deletions(-)

Looks good to me.
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

Re: [PATCH 2/2] nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

[Nilay Shroff]

On 5/30/26 10:50 AM, Shin'ichiro Kawasaki wrote:
> When NVMe-TCP controller setup and teardown are repeated with lockdep
> enabled, lockdep reports false positives WARN for the following locks:
> 
>    1) &q->elevator_lock        : IO scheduler change context
>    2) &q->q_usage_counter(io)  : SCSI disk probe context
>    3) fs_reclaim               : CPU hotplug bring-up context
>    4) cpu_hotplug_lock         : socket establishment context
>    5) sk_lock-AF_INET-NVME     : MQ sched dispatch context for the socket
>    6) set->srcu                : NVMe controller delete context
> 
> The lockdep WARN was observed by running blktests test case nvme/005 for
> tcp transport on v7.1-rc1 kernel with a patch. Refer to the Link tag for
> the details of the WARN.
> 
> This is a false positive because lockdep confuses lock 4) (socket
> establishment) with lock 5) (socket in use) for different socket
> instances. The locks belong to different sockets, but lockdep treats
> them as the same due to shared static lockdep keys.
> 
> Fix this by using dynamically allocated lockdep keys per socket instance
> instead of static keys nvme_tcp_sk_key[] and nvme_tcp_slock_key[]. Add
> nvme_tcp_sk_key and nvme_tcp_slock_key fields to struct nvme_tcp_queue
> and pass them to sock_lock_init_class_and_name() for proper lockdep
> tracking. Change the arguement of nvme_tcp_reclassify_socket() from
> 'struct socket *' to 'struct nvme_tcp_queue *' to pass both the socket
> and the keys.
> 
> Link:https://lore.kernel.org/linux-nvme/afB5syZbUrppgsDQ@shinmob/
> Suggested-by: Nilay Shroff<nilay@linux.ibm.com>
> Signed-off-by: Shin'ichiro Kawasaki<shinichiro.kawasaki@wdc.com>
> ---
>   drivers/nvme/host/tcp.c | 32 +++++++++++++++++++++-----------
>   1 file changed, 21 insertions(+), 11 deletions(-)

Looks good to me.
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>

Re: [PATCH 1/2] nvme-tcp: move nvme_tcp_reclassify_socket()

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

[Keith Busch]

On Sat, May 30, 2026 at 02:20:45PM +0900, Shin'ichiro Kawasaki wrote:
> -static void nvme_tcp_reclassify_socket(struct socket *sock)
> +static void nvme_tcp_reclassify_socket(struct nvme_tcp_queue *queue)
>  {
> -	struct sock *sk = sock->sk;
> +	struct sock *sk = queue->sock->sk;
> +
> +	lockdep_register_key(&queue->nvme_tcp_sk_key);
> +	lockdep_register_key(&queue->nvme_tcp_slock_key);

...

> @@ -1468,6 +1473,11 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
>  	kfree(queue->pdu);
>  	mutex_destroy(&queue->send_mutex);
>  	mutex_destroy(&queue->queue_lock);
> +
> +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> +	lockdep_unregister_key(&queue->nvme_tcp_sk_key);
> +	lockdep_unregister_key(&queue->nvme_tcp_slock_key);
> +#endif
>  }
>  
>  static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
> @@ -1813,7 +1823,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
>  	}
>  
>  	sk_net_refcnt_upgrade(queue->sock->sk);
> -	nvme_tcp_reclassify_socket(queue->sock);
> +	nvme_tcp_reclassify_socket(queue);

On a subsequent error in this function, I think you need to call
lockdep_unregister_key directly in on of the goto error cases since we
don't call nvme_tcp_free_queue for this failed queue.

Re: [PATCH 0/2] nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

[Keith Busch]

On Sat, May 30, 2026 at 02:20:43PM +0900, Shin'ichiro Kawasaki wrote:
> When NVMe-TCP controller setup and teardown are repeated with lockdep
> enabled, lockdep reports false-positive WARNs. This was observed when
> running blktests nvme/005 on v7.1-rc1 with a patch [1]. Analysis by
> Nilay [2] confirmed that the WARNs are false positives caused by lockdep
> confusing different socket instances due to shared static lockdep keys.
> 
> This series resolves the issue by using dynamically allocated lockdep
> keys per socket instance instead of static keys. This ensures lockdep
> correctly tracks locks across different socket instances. For that
> purpose, the first patch moves a function as preparation. The second
> patch introduces the dynamic lockdep keys.
> 
> [1] https://lore.kernel.org/linux-nvme/afB5syZbUrppgsDQ@shinmob/
> [2] https://lore.kernel.org/linux-nvme/c4ddc101-184a-4e4f-82ca-c3123bce5e34@linux.ibm.com/
> 
> Shin'ichiro Kawasaki (2):
>   nvme-tcp: move nvme_tcp_reclassify_socket()
>   nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

Thanks, patch 1 is applied. Please have a look at my comment on patch 2
and let me know if we need to respin that version for it.

Re: [PATCH 2/2] nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

[Shin'ichiro Kawasaki]

On Jun 02, 2026 / 11:13, Keith Busch wrote:
> On Sat, May 30, 2026 at 02:20:45PM +0900, Shin'ichiro Kawasaki wrote:
> > -static void nvme_tcp_reclassify_socket(struct socket *sock)
> > +static void nvme_tcp_reclassify_socket(struct nvme_tcp_queue *queue)
> >  {
> > -	struct sock *sk = sock->sk;
> > +	struct sock *sk = queue->sock->sk;
> > +
> > +	lockdep_register_key(&queue->nvme_tcp_sk_key);
> > +	lockdep_register_key(&queue->nvme_tcp_slock_key);
> 
> ...
> 
> > @@ -1468,6 +1473,11 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
> >  	kfree(queue->pdu);
> >  	mutex_destroy(&queue->send_mutex);
> >  	mutex_destroy(&queue->queue_lock);
> > +
> > +#ifdef CONFIG_DEBUG_LOCK_ALLOC
> > +	lockdep_unregister_key(&queue->nvme_tcp_sk_key);
> > +	lockdep_unregister_key(&queue->nvme_tcp_slock_key);
> > +#endif
> >  }
> >  
> >  static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
> > @@ -1813,7 +1823,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
> >  	}
> >  
> >  	sk_net_refcnt_upgrade(queue->sock->sk);
> > -	nvme_tcp_reclassify_socket(queue->sock);
> > +	nvme_tcp_reclassify_socket(queue);
> 
> On a subsequent error in this function, I think you need to call
> lockdep_unregister_key directly in on of the goto error cases since we
> don't call nvme_tcp_free_queue for this failed queue.

Thank you for point it out. lockdep_unregister_key() calls are required in the
error paths. I will add them in v2.

Re: [PATCH 0/2] nvme-tcp: lockdep: use dynamic lockdep keys per socket instance

[Shin'ichiro Kawasaki]

On Jun 03, 2026 / 10:44, Keith Busch wrote:
...
> > Shin'ichiro Kawasaki (2):
> >   nvme-tcp: move nvme_tcp_reclassify_socket()
> >   nvme-tcp: lockdep: use dynamic lockdep keys per socket instance
> 
> Thanks, patch 1 is applied. Please have a look at my comment on patch 2
> and let me know if we need to respin that version for it.

Thanks! I will post v2 of the patch 2.