| Message ID | 20240423035746.699466-1-danielj@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Remove RTNL lock protection of CVQ | expand |
On Tue, 2024-04-23 at 06:57 +0300, Daniel Jurgens wrote: > 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 dependency by introducing a mutex > to protect the control buffer and writing SGs to the control VQ. > > v5: > - Changed cvq_lock to a mutex. > - Changed dim_lock to mutex, because it's held taking > the cvq_lock. > - Use spin/mutex_lock/unlock vs guard macros. > v4: > - Protect dim_enabled with same lock as well intr_coal. > - Rename intr_coal_lock to dim_lock. > - Remove some scoped_guard where the error path doesn't > have to be in the lock. > v3: > - Changed type of _offloads to __virtio16 to fix static > analysis warning. > - Moved a misplaced hunk to the correct patch. > v2: > - New patch to only process the provided queue in > virtnet_dim_work > - New patch to lock per queue rx coalescing structure. I had only some minor comments, possibly overall worth another iteration. More importantly, this deserves an explicit ack from the virtio crew. @Jason, @Michael: could you please have a look? Thanks! Paolo
在 2024/4/23 上午11:57, 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 dependency by introducing a mutex > to protect the control buffer and writing SGs to the control VQ. I have done functional and performance tests on this set with dim enabled, and it works well. Please taking Paolo's tips into consideration. For the series: Reviewed-by: Heng Qi <hengqi@linux.alibaba.com> Tested-by: Heng Qi <hengqi@linux.alibaba.com> Thanks! > v5: > - Changed cvq_lock to a mutex. > - Changed dim_lock to mutex, because it's held taking > the cvq_lock. > - Use spin/mutex_lock/unlock vs guard macros. > v4: > - Protect dim_enabled with same lock as well intr_coal. > - Rename intr_coal_lock to dim_lock. > - Remove some scoped_guard where the error path doesn't > have to be in the lock. > v3: > - Changed type of _offloads to __virtio16 to fix static > analysis warning. > - Moved a misplaced hunk to the correct patch. > v2: > - New patch to only process the provided queue in > virtnet_dim_work > - New patch to lock per queue rx coalescing structure. > > Daniel Jurgens (6): > 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: Do DIM update for specified queue only > virtio_net: Add a lock for per queue RX coalesce > virtio_net: Remove rtnl lock protection of command buffers > > drivers/net/virtio_net.c | 276 +++++++++++++++++++++++---------------- > 1 file changed, 163 insertions(+), 113 deletions(-) >
> From: Paolo Abeni <pabeni@redhat.com> > Sent: Friday, April 26, 2024 4:54 AM > To: Dan Jurgens <danielj@nvidia.com>; netdev@vger.kernel.org; > jasowang@redhat.com; mst@redhat.com > Cc: xuanzhuo@linux.alibaba.com; virtualization@lists.linux.dev; > davem@davemloft.net; edumazet@google.com; kuba@kernel.org; Jiri Pirko > <jiri@nvidia.com> > Subject: Re: [PATCH net-next v5 0/6] Remove RTNL lock protection of CVQ > > On Tue, 2024-04-23 at 06:57 +0300, Daniel Jurgens wrote: > > 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 dependency by introducing a mutex to > > protect the control buffer and writing SGs to the control VQ. > > > > v5: > > - Changed cvq_lock to a mutex. > > - Changed dim_lock to mutex, because it's held taking > > the cvq_lock. > > - Use spin/mutex_lock/unlock vs guard macros. > > v4: > > - Protect dim_enabled with same lock as well intr_coal. > > - Rename intr_coal_lock to dim_lock. > > - Remove some scoped_guard where the error path doesn't > > have to be in the lock. > > v3: > > - Changed type of _offloads to __virtio16 to fix static > > analysis warning. > > - Moved a misplaced hunk to the correct patch. > > v2: > > - New patch to only process the provided queue in > > virtnet_dim_work > > - New patch to lock per queue rx coalescing structure. > > I had only some minor comments, possibly overall worth another iteration. > > More importantly, this deserves an explicit ack from the virtio crew. > @Jason, @Michael: could you please have a look? Thanks for the review, Paolo. I'll give Jaso and Michael a chance to respond before sending again to address your comments. > > Thanks! > > Paolo