github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-11-07 06:17:02 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

Revert "nvme-tcp: lockdep: use dynamic lockdep keys per socket instance"

Browse Source 
This reverts commit 19bdb70c77.

The commit 19bdb70c77 ("nvme-tcp: lockdep: use dynamic lockdep keys
per socket instance") addressed the lockdep WARN caused by the circular
lock dependency among six locks:

  set->srcu -> sk_lock -> cpu_hotplug_lock -> fs_reclaim -> q_usage_counter -> elevator_lock -> set->srcu

As its title says, the commit cut the dependency by introducing the
dynamic lockdep keys per socket instance. However, as described in the
Link tag URL, the commit made a wrong assumption: it assumed that
__fput_sync(queue->sock->file) in nvme_tcp_free_queue() would
synchronously destroy the socket. This is wrong: when in-flight packets
cause delayed free of the socket, the prematurely freed lockdep key is
referred to and causes another WARN. The commit is an imperfect fix.
Hence revert it.

To address the circular dependency among the six locks, another solution
is required. It is provided by the commit 0ba6912f7e ("Revert "once:
don't use a work queue to reset sleepable static key""). It cuts the
dependency between sk_lock and cpu_hotplug_lock. This solution is
simpler, and reduces the complexity in nvme-tcp.

Link: https://lore.kernel.org/lkml/CANn89i+wnTLC==UnXCpjsS4YxvEfhe5oK0N7fttbqr1zKyqdug@mail.gmail.com/
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
This commit is contained in:
Shin'ichiro Kawasaki 2026-09-14 16:48:52 +09:00 committed by Keith Busch
parent 2f271b8121
commit 7fc61cdab1
1 changed files with 12 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
drivers/nvme/host/tcp.c
View File

@ -144,11 +144,6 @@ 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
};
static DEFINE_MUTEX(nvme_tcp_ctrl_mutex);
@ -185,9 +180,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 void nvme_tcp_reclassify_socket(struct nvme_tcp_queue *queue)
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 = queue->sock->sk;
struct sock *sk = sock->sk;
if (WARN_ON_ONCE(!sock_allow_reclassification(sk)))
return;
@ -195,20 +193,22 @@ static void nvme_tcp_reclassify_socket(struct nvme_tcp_queue *queue)
switch (sk->sk_family) {
case AF_INET:
sock_lock_init_class_and_name(sk, "slock-AF_INET-NVME",
&queue->nvme_tcp_slock_key,
&nvme_tcp_slock_key[0],
"sk_lock-AF_INET-NVME",
&queue->nvme_tcp_sk_key);
&nvme_tcp_sk_key[0]);
break;
case AF_INET6:
sock_lock_init_class_and_name(sk, "slock-AF_INET6-NVME",
&queue->nvme_tcp_slock_key,
&nvme_tcp_slock_key[1],
"sk_lock-AF_INET6-NVME",
&queue->nvme_tcp_sk_key);
&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)
@ -1523,11 +1523,6 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
mutex_destroy(&queue->send_mutex);
mutex_destroy(&queue->queue_lock);
mutex_destroy(&queue->pf_cache_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)
@ -1874,12 +1869,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
}
sk_net_refcnt_upgrade(queue->sock->sk);
#ifdef CONFIG_DEBUG_LOCK_ALLOC
lockdep_register_key(&queue->nvme_tcp_sk_key);
lockdep_register_key(&queue->nvme_tcp_slock_key);
nvme_tcp_reclassify_socket(queue);
#endif
nvme_tcp_reclassify_socket(queue->sock);
/* Single syn retry */
tcp_sock_set_syncnt(queue->sock->sk, 1);
@ -1984,10 +1974,6 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
/* Use sync variant - see nvme_tcp_free_queue() for explanation */
__fput_sync(queue->sock->file);
queue->sock = NULL;
#ifdef CONFIG_DEBUG_LOCK_ALLOC
lockdep_unregister_key(&queue->nvme_tcp_sk_key);
lockdep_unregister_key(&queue->nvme_tcp_slock_key);
#endif
err_destroy_mutex:
mutex_destroy(&queue->send_mutex);
mutex_destroy(&queue->queue_lock);