Skip to content

Commit d1f0bd0

Browse files
Heng Qikuba-moo
Heng Qi
authored andcommitted
virtio_net: fix a spurious deadlock issue
When the following snippet is run, lockdep will report a deadlock[1]. /* Acquire all queues dim_locks */ for (i = 0; i < vi->max_queue_pairs; i++) mutex_lock(&vi->rq[i].dim_lock); There's no deadlock here because the vq locks are always taken in the same order, but lockdep can not figure it out. So refactoring the code to alleviate the problem. [1] ======================================================== WARNING: possible recursive locking detected 6.9.0-rc7+ #319 Not tainted -------------------------------------------- ethtool/962 is trying to acquire lock: but task is already holding lock: other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&vi->rq[i].dim_lock); lock(&vi->rq[i].dim_lock); *** DEADLOCK *** May be due to missing lock nesting notation 3 locks held by ethtool/962: #0: ffffffff82dbaab0 (cb_lock){++++}-{3:3}, at: genl_rcv+0x19/0x40 #1: ffffffff82dad0a8 (rtnl_mutex){+.+.}-{3:3}, at: ethnl_default_set_doit+0xbe/0x1e0 stack backtrace: CPU: 6 PID: 962 Comm: ethtool Not tainted 6.9.0-rc7+ #319 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x79/0xb0 check_deadlock+0x130/0x220 __lock_acquire+0x861/0x990 lock_acquire.part.0+0x72/0x1d0 ? lock_acquire+0xf8/0x130 __mutex_lock+0x71/0xd50 virtnet_set_coalesce+0x151/0x190 __ethnl_set_coalesce.isra.0+0x3f8/0x4d0 ethnl_set_coalesce+0x34/0x90 ethnl_default_set_doit+0xdd/0x1e0 genl_family_rcv_msg_doit+0xdc/0x130 genl_family_rcv_msg+0x154/0x230 ? __pfx_ethnl_default_set_doit+0x10/0x10 genl_rcv_msg+0x4b/0xa0 ? __pfx_genl_rcv_msg+0x10/0x10 netlink_rcv_skb+0x5a/0x110 genl_rcv+0x28/0x40 netlink_unicast+0x1af/0x280 netlink_sendmsg+0x20e/0x460 __sys_sendto+0x1fe/0x210 ? find_held_lock+0x2b/0x80 ? do_user_addr_fault+0x3a2/0x8a0 ? __lock_release+0x5e/0x160 ? do_user_addr_fault+0x3a2/0x8a0 ? lock_release+0x72/0x140 ? do_user_addr_fault+0x3a7/0x8a0 __x64_sys_sendto+0x29/0x30 do_syscall_64+0x78/0x180 entry_SYSCALL_64_after_hwframe+0x76/0x7e Fixes: 4d4ac2e ("virtio_net: Add a lock for per queue RX coalesce") Signed-off-by: Heng Qi <[email protected]> Acked-by: Michael S. Tsirkin <[email protected]> Reviewed-by: Xuan Zhuo <[email protected]> Acked-by: Jason Wang <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 9e0945b commit d1f0bd0

File tree

1 file changed

+16
-20
lines changed

1 file changed

+16
-20
lines changed

drivers/net/virtio_net.c

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4259,7 +4259,6 @@ static int virtnet_send_rx_notf_coal_cmds(struct virtnet_info *vi,
42594259
struct virtio_net_ctrl_coal_rx *coal_rx __free(kfree) = NULL;
42604260
bool rx_ctrl_dim_on = !!ec->use_adaptive_rx_coalesce;
42614261
struct scatterlist sgs_rx;
4262-
int ret = 0;
42634262
int i;
42644263

42654264
if (rx_ctrl_dim_on && !virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
@@ -4269,27 +4268,27 @@ static int virtnet_send_rx_notf_coal_cmds(struct virtnet_info *vi,
42694268
ec->rx_max_coalesced_frames != vi->intr_coal_rx.max_packets))
42704269
return -EINVAL;
42714270

4272-
/* Acquire all queues dim_locks */
4273-
for (i = 0; i < vi->max_queue_pairs; i++)
4274-
mutex_lock(&vi->rq[i].dim_lock);
4275-
42764271
if (rx_ctrl_dim_on && !vi->rx_dim_enabled) {
42774272
vi->rx_dim_enabled = true;
4278-
for (i = 0; i < vi->max_queue_pairs; i++)
4273+
for (i = 0; i < vi->max_queue_pairs; i++) {
4274+
mutex_lock(&vi->rq[i].dim_lock);
42794275
vi->rq[i].dim_enabled = true;
4280-
goto unlock;
4276+
mutex_unlock(&vi->rq[i].dim_lock);
4277+
}
4278+
return 0;
42814279
}
42824280

42834281
coal_rx = kzalloc(sizeof(*coal_rx), GFP_KERNEL);
4284-
if (!coal_rx) {
4285-
ret = -ENOMEM;
4286-
goto unlock;
4287-
}
4282+
if (!coal_rx)
4283+
return -ENOMEM;
42884284

42894285
if (!rx_ctrl_dim_on && vi->rx_dim_enabled) {
42904286
vi->rx_dim_enabled = false;
4291-
for (i = 0; i < vi->max_queue_pairs; i++)
4287+
for (i = 0; i < vi->max_queue_pairs; i++) {
4288+
mutex_lock(&vi->rq[i].dim_lock);
42924289
vi->rq[i].dim_enabled = false;
4290+
mutex_unlock(&vi->rq[i].dim_lock);
4291+
}
42934292
}
42944293

42954294
/* Since the per-queue coalescing params can be set,
@@ -4302,22 +4301,19 @@ static int virtnet_send_rx_notf_coal_cmds(struct virtnet_info *vi,
43024301

43034302
if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_NOTF_COAL,
43044303
VIRTIO_NET_CTRL_NOTF_COAL_RX_SET,
4305-
&sgs_rx)) {
4306-
ret = -EINVAL;
4307-
goto unlock;
4308-
}
4304+
&sgs_rx))
4305+
return -EINVAL;
43094306

43104307
vi->intr_coal_rx.max_usecs = ec->rx_coalesce_usecs;
43114308
vi->intr_coal_rx.max_packets = ec->rx_max_coalesced_frames;
43124309
for (i = 0; i < vi->max_queue_pairs; i++) {
4310+
mutex_lock(&vi->rq[i].dim_lock);
43134311
vi->rq[i].intr_coal.max_usecs = ec->rx_coalesce_usecs;
43144312
vi->rq[i].intr_coal.max_packets = ec->rx_max_coalesced_frames;
4315-
}
4316-
unlock:
4317-
for (i = vi->max_queue_pairs - 1; i >= 0; i--)
43184313
mutex_unlock(&vi->rq[i].dim_lock);
4314+
}
43194315

4320-
return ret;
4316+
return 0;
43214317
}
43224318

43234319
static int virtnet_send_notf_coal_cmds(struct virtnet_info *vi,

0 commit comments

Comments
 (0)