netdev - Re: [PATCH net v2 2/2] Revert "virtio_net: Add a lock for per queue RX coalesce"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 27 May 2024 12:42:43 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Heng Qi <hengqi@...ux.alibaba.com>, netdev@...r.kernel.org, 
	virtualization@...ts.linux.dev
Cc: Jason Wang <jasowang@...hat.com>, "Michael S. Tsirkin" <mst@...hat.com>,
  Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
 <kuba@...nel.org>, Jiri Pirko <jiri@...nulli.us>
Subject: Re: [PATCH net v2 2/2] Revert "virtio_net: Add a lock for per queue
 RX coalesce"

On Thu, 2024-05-23 at 15:46 +0800, Heng Qi wrote:
> This reverts commit 4d4ac2ececd3c42a08dd32a6e3a4aaf25f7efe44.
> 
> 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, and we
> can not make each lock a separate class because there can be more
> than MAX_LOCKDEP_SUBCLASSES of vqs.
> 
> However, dropping the lock is harmless:
>   1. If dim is enabled, modifications made by dim worker to coalescing
>      params may cause the user's query results to be dirty data.

It looks like the above can confuse the user-space/admin?

Have you considered instead re-factoring
virtnet_send_rx_notf_coal_cmds() to avoid acquiring all the mutex in
sequence?

Thanks!

Paolo