* [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting
@ 2025-11-18 9:09 liming.wu
2025-11-19 2:56 ` Jason Wang
0 siblings, 1 reply; 4+ messages in thread
From: liming.wu @ 2025-11-18 9:09 UTC (permalink / raw)
To: Michael S . Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez
Cc: kvm, virtualization, netdev, linux-kernel, angus.chen, Liming Wu
From: Liming Wu <liming.wu@jaguarmicro.com>
This patch refines and strengthens the statistics collection of TX queue
wake/stop events introduced by a previous patch.
Previously, the driver only recorded partial wake/stop statistics
for TX queues. Some wake events triggered by 'skb_xmit_done()' or resume
operations were not counted, which made the per-queue metrics incomplete.
Signed-off-by: Liming Wu <liming.wu@jaguarmicro.com>
---
drivers/net/virtio_net.c | 49 ++++++++++++++++++++++------------------
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8e8a179aaa49..f92a90dde2b3 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -775,10 +775,26 @@ static bool virtqueue_napi_complete(struct napi_struct *napi,
return false;
}
+static void virtnet_tx_wake_queue(struct virtnet_info *vi,
+ struct send_queue *sq)
+{
+ unsigned int index = vq2txq(sq->vq);
+ struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, index);
+
+ if (netif_tx_queue_stopped(txq)) {
+ u64_stats_update_begin(&sq->stats.syncp);
+ u64_stats_inc(&sq->stats.wake);
+ u64_stats_update_end(&sq->stats.syncp);
+ netif_tx_wake_queue(txq);
+ }
+}
+
static void skb_xmit_done(struct virtqueue *vq)
{
struct virtnet_info *vi = vq->vdev->priv;
- struct napi_struct *napi = &vi->sq[vq2txq(vq)].napi;
+ unsigned int index = vq2txq(vq);
+ struct send_queue *sq = &vi->sq[index];
+ struct napi_struct *napi = &sq->napi;
/* Suppress further interrupts. */
virtqueue_disable_cb(vq);
@@ -786,8 +802,7 @@ static void skb_xmit_done(struct virtqueue *vq)
if (napi->weight)
virtqueue_napi_schedule(napi, vq);
else
- /* We were probably waiting for more output buffers. */
- netif_wake_subqueue(vi->dev, vq2txq(vq));
+ virtnet_tx_wake_queue(vi, sq);
}
#define MRG_CTX_HEADER_SHIFT 22
@@ -1166,10 +1181,7 @@ static void check_sq_full_and_disable(struct virtnet_info *vi,
/* More just got used, free them then recheck. */
free_old_xmit(sq, txq, false);
if (sq->vq->num_free >= MAX_SKB_FRAGS + 2) {
- netif_start_subqueue(dev, qnum);
- u64_stats_update_begin(&sq->stats.syncp);
- u64_stats_inc(&sq->stats.wake);
- u64_stats_update_end(&sq->stats.syncp);
+ virtnet_tx_wake_queue(vi, sq);
virtqueue_disable_cb(sq->vq);
}
}
@@ -3068,13 +3080,8 @@ static void virtnet_poll_cleantx(struct receive_queue *rq, int budget)
free_old_xmit(sq, txq, !!budget);
} while (unlikely(!virtqueue_enable_cb_delayed(sq->vq)));
- if (sq->vq->num_free >= MAX_SKB_FRAGS + 2 &&
- netif_tx_queue_stopped(txq)) {
- u64_stats_update_begin(&sq->stats.syncp);
- u64_stats_inc(&sq->stats.wake);
- u64_stats_update_end(&sq->stats.syncp);
- netif_tx_wake_queue(txq);
- }
+ if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
+ virtnet_tx_wake_queue(vi, sq);
__netif_tx_unlock(txq);
}
@@ -3264,13 +3271,8 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget)
else
free_old_xmit(sq, txq, !!budget);
- if (sq->vq->num_free >= MAX_SKB_FRAGS + 2 &&
- netif_tx_queue_stopped(txq)) {
- u64_stats_update_begin(&sq->stats.syncp);
- u64_stats_inc(&sq->stats.wake);
- u64_stats_update_end(&sq->stats.syncp);
- netif_tx_wake_queue(txq);
- }
+ if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
+ virtnet_tx_wake_queue(vi, sq);
if (xsk_done >= budget) {
__netif_tx_unlock(txq);
@@ -3521,6 +3523,9 @@ static void virtnet_tx_pause(struct virtnet_info *vi, struct send_queue *sq)
/* Prevent the upper layer from trying to send packets. */
netif_stop_subqueue(vi->dev, qindex);
+ u64_stats_update_begin(&sq->stats.syncp);
+ u64_stats_inc(&sq->stats.stop);
+ u64_stats_update_end(&sq->stats.syncp);
__netif_tx_unlock_bh(txq);
}
@@ -3537,7 +3542,7 @@ static void virtnet_tx_resume(struct virtnet_info *vi, struct send_queue *sq)
__netif_tx_lock_bh(txq);
sq->reset = false;
- netif_tx_wake_queue(txq);
+ virtnet_tx_wake_queue(vi, sq);
__netif_tx_unlock_bh(txq);
if (running)
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting
2025-11-18 9:09 [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting liming.wu
@ 2025-11-19 2:56 ` Jason Wang
2025-11-19 7:54 ` Liming Wu
0 siblings, 1 reply; 4+ messages in thread
From: Jason Wang @ 2025-11-19 2:56 UTC (permalink / raw)
To: liming.wu
Cc: Michael S . Tsirkin, Xuan Zhuo, Eugenio Pérez, kvm,
virtualization, netdev, linux-kernel, angus.chen
On Tue, Nov 18, 2025 at 5:10 PM <liming.wu@jaguarmicro.com> wrote:
>
> From: Liming Wu <liming.wu@jaguarmicro.com>
>
> This patch refines and strengthens the statistics collection of TX queue
> wake/stop events introduced by a previous patch.
It would be better to add commit id here.
>
> Previously, the driver only recorded partial wake/stop statistics
> for TX queues. Some wake events triggered by 'skb_xmit_done()' or resume
> operations were not counted, which made the per-queue metrics incomplete.
>
> Signed-off-by: Liming Wu <liming.wu@jaguarmicro.com>
> ---
> drivers/net/virtio_net.c | 49 ++++++++++++++++++++++------------------
> 1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 8e8a179aaa49..f92a90dde2b3 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -775,10 +775,26 @@ static bool virtqueue_napi_complete(struct napi_struct *napi,
> return false;
> }
>
> +static void virtnet_tx_wake_queue(struct virtnet_info *vi,
> + struct send_queue *sq)
> +{
> + unsigned int index = vq2txq(sq->vq);
> + struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, index);
> +
> + if (netif_tx_queue_stopped(txq)) {
> + u64_stats_update_begin(&sq->stats.syncp);
> + u64_stats_inc(&sq->stats.wake);
> + u64_stats_update_end(&sq->stats.syncp);
> + netif_tx_wake_queue(txq);
> + }
> +}
> +
> static void skb_xmit_done(struct virtqueue *vq)
> {
> struct virtnet_info *vi = vq->vdev->priv;
> - struct napi_struct *napi = &vi->sq[vq2txq(vq)].napi;
> + unsigned int index = vq2txq(vq);
> + struct send_queue *sq = &vi->sq[index];
> + struct napi_struct *napi = &sq->napi;
>
> /* Suppress further interrupts. */
> virtqueue_disable_cb(vq);
> @@ -786,8 +802,7 @@ static void skb_xmit_done(struct virtqueue *vq)
> if (napi->weight)
> virtqueue_napi_schedule(napi, vq);
> else
> - /* We were probably waiting for more output buffers. */
> - netif_wake_subqueue(vi->dev, vq2txq(vq));
> + virtnet_tx_wake_queue(vi, sq);
> }
>
> #define MRG_CTX_HEADER_SHIFT 22
> @@ -1166,10 +1181,7 @@ static void check_sq_full_and_disable(struct virtnet_info *vi,
> /* More just got used, free them then recheck. */
> free_old_xmit(sq, txq, false);
> if (sq->vq->num_free >= MAX_SKB_FRAGS + 2) {
> - netif_start_subqueue(dev, qnum);
> - u64_stats_update_begin(&sq->stats.syncp);
> - u64_stats_inc(&sq->stats.wake);
> - u64_stats_update_end(&sq->stats.syncp);
> + virtnet_tx_wake_queue(vi, sq);
This is suspicious, netif_tx_wake_queue() will schedule qdisc, or is
this intended?
> virtqueue_disable_cb(sq->vq);
> }
> }
> @@ -3068,13 +3080,8 @@ static void virtnet_poll_cleantx(struct receive_queue *rq, int budget)
> free_old_xmit(sq, txq, !!budget);
> } while (unlikely(!virtqueue_enable_cb_delayed(sq->vq)));
>
> - if (sq->vq->num_free >= MAX_SKB_FRAGS + 2 &&
> - netif_tx_queue_stopped(txq)) {
> - u64_stats_update_begin(&sq->stats.syncp);
> - u64_stats_inc(&sq->stats.wake);
> - u64_stats_update_end(&sq->stats.syncp);
> - netif_tx_wake_queue(txq);
> - }
> + if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
> + virtnet_tx_wake_queue(vi, sq);
>
> __netif_tx_unlock(txq);
> }
> @@ -3264,13 +3271,8 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget)
> else
> free_old_xmit(sq, txq, !!budget);
>
> - if (sq->vq->num_free >= MAX_SKB_FRAGS + 2 &&
> - netif_tx_queue_stopped(txq)) {
> - u64_stats_update_begin(&sq->stats.syncp);
> - u64_stats_inc(&sq->stats.wake);
> - u64_stats_update_end(&sq->stats.syncp);
> - netif_tx_wake_queue(txq);
> - }
> + if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
> + virtnet_tx_wake_queue(vi, sq);
>
> if (xsk_done >= budget) {
> __netif_tx_unlock(txq);
> @@ -3521,6 +3523,9 @@ static void virtnet_tx_pause(struct virtnet_info *vi, struct send_queue *sq)
>
> /* Prevent the upper layer from trying to send packets. */
> netif_stop_subqueue(vi->dev, qindex);
> + u64_stats_update_begin(&sq->stats.syncp);
> + u64_stats_inc(&sq->stats.stop);
> + u64_stats_update_end(&sq->stats.syncp);
>
> __netif_tx_unlock_bh(txq);
> }
> @@ -3537,7 +3542,7 @@ static void virtnet_tx_resume(struct virtnet_info *vi, struct send_queue *sq)
>
> __netif_tx_lock_bh(txq);
> sq->reset = false;
> - netif_tx_wake_queue(txq);
> + virtnet_tx_wake_queue(vi, sq);
> __netif_tx_unlock_bh(txq);
>
> if (running)
> --
> 2.34.1
>
Thanks
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting
2025-11-19 2:56 ` Jason Wang
@ 2025-11-19 7:54 ` Liming Wu
2025-11-19 7:58 ` Michael S. Tsirkin
0 siblings, 1 reply; 4+ messages in thread
From: Liming Wu @ 2025-11-19 7:54 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S . Tsirkin, Xuan Zhuo, Eugenio Pérez,
kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Angus Chen
> queue wake/stop events introduced by a previous patch.
>
> It would be better to add commit id here.
OK, thx.
>
eck. */
> > free_old_xmit(sq, txq, false);
> > if (sq->vq->num_free >= MAX_SKB_FRAGS + 2) {
> > - netif_start_subqueue(dev, qnum);
> > -
> u64_stats_update_begin(&sq->stats.syncp);
> > - u64_stats_inc(&sq->stats.wake);
> > -
> u64_stats_update_end(&sq->stats.syncp);
> > + virtnet_tx_wake_queue(vi, sq);
>
> This is suspicious, netif_tx_wake_queue() will schedule qdisc, or is this intended?
Thanks for pointing this out.
You're right — using netif_tx_wake_queue() here would indeed trigger qdisc scheduling, which is not intended in this specific path.
My change tried to unify the wake/stop accounting paths, but replacing netif_start_subqueue() was not the right choice semantically.
I will restore netif_start_subqueue() at this site and keep only the statistic increment, so the behavior stays consistent with the original code while still improving the per-queue metrics.
>
> > virtqueue_disable_cb(sq->vq);
> > }
> > }
> > @@ -3068,13 +3080,8 @@ static void virtnet_poll_cleantx(struct
> receive_queue *rq, int budget)
> > free_old_xmit(sq, txq, !!budget);
> > } while
> > (unlikely(!virtqueue_enable_cb_delayed(sq->vq)));
> >
> > - if (sq->vq->num_free >= MAX_SKB_FRAGS + 2 &&
> > - netif_tx_queue_stopped(txq)) {
> > - u64_stats_update_begin(&sq->stats.syncp);
> > - u64_stats_inc(&sq->stats.wake);
> > - u64_stats_update_end(&sq->stats.syncp);
> > - netif_tx_wake_queue(txq);
> > - }
> > + if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
> > + virtnet_tx_wake_queue(vi, sq);
> >
> > __netif_tx_unlock(txq);
> > }
> > @@ -3264,13 +3271,8 @@ static int virtnet_poll_tx(struct napi_struct *napi,
> int budget)
> > else
> > free_old_xmit(sq, txq, !!budget);
> >
> > - if (sq->vq->num_free >= MAX_SKB_FRAGS + 2 &&
> > - netif_tx_queue_stopped(txq)) {
> > - u64_stats_update_begin(&sq->stats.syncp);
> > - u64_stats_inc(&sq->stats.wake);
> > - u64_stats_update_end(&sq->stats.syncp);
> > - netif_tx_wake_queue(txq);
> > - }
> > + if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
> > + virtnet_tx_wake_queue(vi, sq);
> >
> > if (xsk_done >= budget) {
> > __netif_tx_unlock(txq); @@ -3521,6 +3523,9 @@ static
> > void virtnet_tx_pause(struct virtnet_info *vi, struct send_queue *sq)
> >
> > /* Prevent the upper layer from trying to send packets. */
> > netif_stop_subqueue(vi->dev, qindex);
> > + u64_stats_update_begin(&sq->stats.syncp);
> > + u64_stats_inc(&sq->stats.stop);
> > + u64_stats_update_end(&sq->stats.syncp);
> >
> > __netif_tx_unlock_bh(txq);
> > }
> > @@ -3537,7 +3542,7 @@ static void virtnet_tx_resume(struct
> > virtnet_info *vi, struct send_queue *sq)
> >
> > __netif_tx_lock_bh(txq);
> > sq->reset = false;
> > - netif_tx_wake_queue(txq);
> > + virtnet_tx_wake_queue(vi, sq);
> > __netif_tx_unlock_bh(txq);
> >
> > if (running)
> > --
> > 2.34.1
> >
>
> Thanks
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting
2025-11-19 7:54 ` Liming Wu
@ 2025-11-19 7:58 ` Michael S. Tsirkin
0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2025-11-19 7:58 UTC (permalink / raw)
To: Liming Wu
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, kvm@vger.kernel.org,
virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Angus Chen
On Wed, Nov 19, 2025 at 07:54:07AM +0000, Liming Wu wrote:
> > queue wake/stop events introduced by a previous patch.
> >
> > It would be better to add commit id here.
> OK, thx.
>
> >
> eck. */
> > > free_old_xmit(sq, txq, false);
> > > if (sq->vq->num_free >= MAX_SKB_FRAGS + 2) {
> > > - netif_start_subqueue(dev, qnum);
> > > -
> > u64_stats_update_begin(&sq->stats.syncp);
> > > - u64_stats_inc(&sq->stats.wake);
> > > -
> > u64_stats_update_end(&sq->stats.syncp);
> > > + virtnet_tx_wake_queue(vi, sq);
> >
> > This is suspicious, netif_tx_wake_queue() will schedule qdisc, or is this intended?
> Thanks for pointing this out.
> You're right — using netif_tx_wake_queue() here would indeed trigger qdisc scheduling, which is not intended in this specific path.
> My change tried to unify the wake/stop accounting paths, but replacing netif_start_subqueue() was not the right choice semantically.
>
> I will restore netif_start_subqueue() at this site and keep only the statistic increment, so the behavior stays consistent with the original code while still improving the per-queue metrics.
Please do not send fluff comments like this to the list.
And with em-dashes too, for added flair.
If you can not bother writing email yourself why should
anyone bother reading it?
> >
> > > virtqueue_disable_cb(sq->vq);
> > > }
> > > }
> > > @@ -3068,13 +3080,8 @@ static void virtnet_poll_cleantx(struct
> > receive_queue *rq, int budget)
> > > free_old_xmit(sq, txq, !!budget);
> > > } while
> > > (unlikely(!virtqueue_enable_cb_delayed(sq->vq)));
> > >
> > > - if (sq->vq->num_free >= MAX_SKB_FRAGS + 2 &&
> > > - netif_tx_queue_stopped(txq)) {
> > > - u64_stats_update_begin(&sq->stats.syncp);
> > > - u64_stats_inc(&sq->stats.wake);
> > > - u64_stats_update_end(&sq->stats.syncp);
> > > - netif_tx_wake_queue(txq);
> > > - }
> > > + if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
> > > + virtnet_tx_wake_queue(vi, sq);
> > >
> > > __netif_tx_unlock(txq);
> > > }
> > > @@ -3264,13 +3271,8 @@ static int virtnet_poll_tx(struct napi_struct *napi,
> > int budget)
> > > else
> > > free_old_xmit(sq, txq, !!budget);
> > >
> > > - if (sq->vq->num_free >= MAX_SKB_FRAGS + 2 &&
> > > - netif_tx_queue_stopped(txq)) {
> > > - u64_stats_update_begin(&sq->stats.syncp);
> > > - u64_stats_inc(&sq->stats.wake);
> > > - u64_stats_update_end(&sq->stats.syncp);
> > > - netif_tx_wake_queue(txq);
> > > - }
> > > + if (sq->vq->num_free >= MAX_SKB_FRAGS + 2)
> > > + virtnet_tx_wake_queue(vi, sq);
> > >
> > > if (xsk_done >= budget) {
> > > __netif_tx_unlock(txq); @@ -3521,6 +3523,9 @@ static
> > > void virtnet_tx_pause(struct virtnet_info *vi, struct send_queue *sq)
> > >
> > > /* Prevent the upper layer from trying to send packets. */
> > > netif_stop_subqueue(vi->dev, qindex);
> > > + u64_stats_update_begin(&sq->stats.syncp);
> > > + u64_stats_inc(&sq->stats.stop);
> > > + u64_stats_update_end(&sq->stats.syncp);
> > >
> > > __netif_tx_unlock_bh(txq);
> > > }
> > > @@ -3537,7 +3542,7 @@ static void virtnet_tx_resume(struct
> > > virtnet_info *vi, struct send_queue *sq)
> > >
> > > __netif_tx_lock_bh(txq);
> > > sq->reset = false;
> > > - netif_tx_wake_queue(txq);
> > > + virtnet_tx_wake_queue(vi, sq);
> > > __netif_tx_unlock_bh(txq);
> > >
> > > if (running)
> > > --
> > > 2.34.1
> > >
> >
> > Thanks
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-19 7:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-18 9:09 [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting liming.wu
2025-11-19 2:56 ` Jason Wang
2025-11-19 7:54 ` Liming Wu
2025-11-19 7:58 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).