From: Heng Qi <redacted>
Sent: Monday, March 25, 2024 9:54 PM
To: Dan Jurgens <redacted>; netdev@vger.kernel.org
Cc: mst@redhat.com; jasowang@redhat.com; xuanzhuo@linux.alibaba.com;
virtualization@lists.linux.dev; davem@davemloft.net;
edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Jiri Pirko
[off-list ref]
Subject: Re: [PATCH net-next 0/4] Remove RTNL lock protection of CVQ



在 2024/3/26 上午5:49, Daniel Jurgens 写道:

quoted

Currently the buffer used for control VQ commands is protected by the
RTNL lock. Previously this wasn't a major concern because the control
VQ was only used during device setup and user interaction. With the
recent addition of dynamic interrupt moderation the control VQ may be
used frequently during normal operation.

This series removes the RNTL lock dependancy by introducing a spin
lock to protect the control buffer and writing SGs to the control VQ.

Hi Daniel.

It's a nice piece of work, but now that we're talking about ctrlq adding
interrupts, spin lock has some conflicts with its goals. For example, we expect
the ethtool command to be blocked.
Therefore, a mutex lock may be more suitable.

Any how, the final conclusion may require some waiting.

Thanks, Heng

I took this a step further and made the ctrlq interrupt driven, but an internal reviewer pointed me to this:
https://lore.kernel.org/lkml/20230413064027.13267-1-jasowang@redhat.com/ (local) (sorry if it gets safelinked)

It seemed there was little appetite to go that route last year, because of set RX mode behavior change, and consumption of an additional IRQ.

Either way, I think the spin lock is still needed. In my interrupt driven implantation I was allocating a new control buffer instead of just the data fields. The spin lock was tighter around virtqueue_add_sgs, after the kick it would unlock and wait for a completion that would be triggered from the cvq callback. 

Regards,
Heng

quoted

Daniel Jurgens (4):
   virtio_net: Store RSS setting in virtnet_info
   virtio_net: Remove command data from control_buf
   virtio_net: Add a lock for the command VQ.
   virtio_net: Remove rtnl lock protection of command buffers

  drivers/net/virtio_net.c | 185 ++++++++++++++++++++++-----------------
  1 file changed, 104 insertions(+), 81 deletions(-)