Re: [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting
On Wed, Nov 19, 2025 at 07:54:07AM +, 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
>
RE: [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting
> 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
Re: [PATCH] virtio_net: enhance wake/stop tx queue statistics accounting
On Tue, Nov 18, 2025 at 5:10 PM wrote:
>
> From: Liming Wu
>
> 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
> ---
> 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
