> 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
