virtio_net: enhance wake/stop tx queue statistics accounting

[PATCH] virtio_net: enhance wake/stop tx queue statistics accounting

Posted by liming.wu@jaguarmicro.com 1 week, 6 days ago

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

Re: [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting

Posted by Jason Wang 1 week, 5 days ago

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