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
>
