在 2024/3/26 下午12:11, Dan Jurgens 写道:
From: Heng Qi <hengqi@xxxxxxxxxxxxxxxxx>
Sent: Monday, March 25, 2024 9:54 PM
To: Dan Jurgens <danielj@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx
Cc: mst@xxxxxxxxxx; jasowang@xxxxxxxxxx; xuanzhuo@xxxxxxxxxxxxxxxxx;
virtualization@xxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx;
edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; Jiri Pirko
<jiri@xxxxxxxxxx>
Subject: Re: [PATCH net-next 0/4] Remove RTNL lock protection of CVQ
在 2024/3/26 上午5:49, Daniel Jurgens 写道:
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@xxxxxxxxxx/ (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.
Hi DanielJ.
Jason now supports this and wants to make changes to ctrlq.
Yes, our requirements for ctrlq have become higher and we need to make
updates as expected.
So your patches look good:
Reviewed-by: Heng Qi <hengqi@xxxxxxxxxxxxxxxxx>
I will make further modifications on top of these.
Regards,
Heng
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
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(-)
