[PATCH net v2 0/3] virtio-net: fix the deadlock when disabling rx NAPI

[PATCH net v2 0/3] virtio-net: fix the deadlock when disabling rx NAPI

[Bui Quang Minh]

Calling napi_disable() on an already disabled napi can cause the
deadlock. In commit 4bc12818b363 ("virtio-net: disable delayed refill
when pausing rx"), to avoid the deadlock, when pausing the RX in
virtnet_rx_pause[_all](), we disable and cancel the delayed refill work.
However, in the virtnet_rx_resume_all(), we enable the delayed refill
work too early before enabling all the receive queue napis.

The deadlock can be reproduced by running
selftests/drivers/net/hw/xsk_reconfig.py with multiqueue virtio-net
device and inserting a cond_resched() inside the for loop in
virtnet_rx_resume_all() to increase the success rate. Because the worker
processing the delayed refilled work runs on the same CPU as
virtnet_rx_resume_all(), a reschedule is needed to cause the deadlock.
In real scenario, the contention on netdev_lock can cause the
reschedule.

Due to the complexity of delayed refill worker, in this series, we remove
it. When we fail to refill the receive buffer, we will retry in the next
NAPI poll instead.
- Patch 1: removes delayed refill worker schedule and retry refill in next
NAPI
- Patch 2, 3: removes and clean up unused delayed refill worker code

For testing, I've run the following tests with no issue so far
- selftests/drivers/net/hw/xsk_reconfig.py which sets up the XDP zerocopy
without providing any descriptors to the fill ring. As a result,
try_fill_recv will always fail.
- Send TCP packets from host to guest while guest is nearly OOM and some
try_fill_recv calls fail.

Changes in v2:
- Remove the delayed refill worker to simplify the logic instead of trying
to fix it
- Link to v1:
https://lore.kernel.org/netdev/20251223152533.24364-1-minhquangbui99@gmail.com/

Link to the previous approach and discussion:
https://lore.kernel.org/netdev/20251212152741.11656-1-minhquangbui99@gmail.com/

Thanks,
Quang Minh.

Bui Quang Minh (3):
  virtio-net: don't schedule delayed refill worker
  virtio-net: remove unused delayed refill worker
  virtio-net: clean up __virtnet_rx_pause/resume

 drivers/net/virtio_net.c | 171 +++++++++------------------------------
 1 file changed, 40 insertions(+), 131 deletions(-)

-- 
2.43.0

[PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Bui Quang Minh]

When we fail to refill the receive buffers, we schedule a delayed worker
to retry later. However, this worker creates some concurrency issues
such as races and deadlocks. To simplify the logic and avoid further
problems, we will instead retry refilling in the next NAPI poll.

Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
Reported-by: Paolo Abeni <pabeni@redhat.com>
Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
Cc: stable@vger.kernel.org
Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
---
 drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1bb3aeca66c6..ac514c9383ae 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
 }
 
 static int virtnet_receive(struct receive_queue *rq, int budget,
-			   unsigned int *xdp_xmit)
+			   unsigned int *xdp_xmit, bool *retry_refill)
 {
 	struct virtnet_info *vi = rq->vq->vdev->priv;
 	struct virtnet_rq_stats stats = {};
@@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
 		packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
 
 	if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
-		if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
-			spin_lock(&vi->refill_lock);
-			if (vi->refill_enabled)
-				schedule_delayed_work(&vi->refill, 0);
-			spin_unlock(&vi->refill_lock);
-		}
+		if (!try_fill_recv(vi, rq, GFP_ATOMIC))
+			*retry_refill = true;
 	}
 
 	u64_stats_set(&stats.packets, packets);
@@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
 	struct send_queue *sq;
 	unsigned int received;
 	unsigned int xdp_xmit = 0;
-	bool napi_complete;
+	bool napi_complete, retry_refill = false;
 
 	virtnet_poll_cleantx(rq, budget);
 
-	received = virtnet_receive(rq, budget, &xdp_xmit);
+	received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
 	rq->packets_in_napi += received;
 
 	if (xdp_xmit & VIRTIO_XDP_REDIR)
 		xdp_do_flush();
 
 	/* Out of packets? */
-	if (received < budget) {
+	if (received < budget && !retry_refill) {
 		napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
 		/* Intentionally not taking dim_lock here. This may result in a
 		 * spurious net_dim call. But if that happens virtnet_rx_dim_work
@@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
 		virtnet_xdp_put_sq(vi, sq);
 	}
 
-	return received;
+	return retry_refill ? budget : received;
 }
 
 static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
@@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		if (i < vi->curr_queue_pairs)
-			/* Make sure we have some buffers: if oom use wq. */
-			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
-				schedule_delayed_work(&vi->refill, 0);
+			/* If this fails, we will retry later in
+			 * NAPI poll, which is scheduled in the below
+			 * virtnet_enable_queue_pair
+			 */
+			try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
 
 		err = virtnet_enable_queue_pair(vi, i);
 		if (err < 0)
@@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
 				bool refill)
 {
 	bool running = netif_running(vi->dev);
-	bool schedule_refill = false;
 
-	if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
-		schedule_refill = true;
+	if (refill)
+		/* If this fails, we will retry later in NAPI poll, which is
+		 * scheduled in the below virtnet_napi_enable
+		 */
+		try_fill_recv(vi, rq, GFP_KERNEL);
+
 	if (running)
 		virtnet_napi_enable(rq);
-
-	if (schedule_refill)
-		schedule_delayed_work(&vi->refill, 0);
 }
 
 static void virtnet_rx_resume_all(struct virtnet_info *vi)
@@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
 	struct virtio_net_rss_config_trailer old_rss_trailer;
 	struct net_device *dev = vi->dev;
 	struct scatterlist sg;
+	int i;
 
 	if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
 		return 0;
@@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
 	}
 succ:
 	vi->curr_queue_pairs = queue_pairs;
-	/* virtnet_open() will refill when device is going to up. */
-	spin_lock_bh(&vi->refill_lock);
-	if (dev->flags & IFF_UP && vi->refill_enabled)
-		schedule_delayed_work(&vi->refill, 0);
-	spin_unlock_bh(&vi->refill_lock);
+	if (dev->flags & IFF_UP) {
+		/* Let the NAPI poll refill the receive buffer for us. We can't
+		 * safely call try_fill_recv() here because the NAPI might be
+		 * enabled already.
+		 */
+		local_bh_disable();
+		for (i = 0; i < vi->curr_queue_pairs; i++)
+			virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
+
+		local_bh_enable();
+	}
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Michael S. Tsirkin]

On Fri, Jan 02, 2026 at 10:20:21PM +0700, Bui Quang Minh wrote:
> When we fail to refill the receive buffers, we schedule a delayed worker
> to retry later. However, this worker creates some concurrency issues
> such as races and deadlocks.

include at least one example here, pls.

> To simplify the logic and avoid further
> problems, we will instead retry refilling in the next NAPI poll.
> 
> Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
> Reported-by: Paolo Abeni <pabeni@redhat.com>
> Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
> Cc: stable@vger.kernel.org
> Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> ---
>  drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1bb3aeca66c6..ac514c9383ae 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
>  }
>  
>  static int virtnet_receive(struct receive_queue *rq, int budget,
> -			   unsigned int *xdp_xmit)
> +			   unsigned int *xdp_xmit, bool *retry_refill)
>  {
>  	struct virtnet_info *vi = rq->vq->vdev->priv;
>  	struct virtnet_rq_stats stats = {};
> @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>  		packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
>  
>  	if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
> -		if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
> -			spin_lock(&vi->refill_lock);
> -			if (vi->refill_enabled)
> -				schedule_delayed_work(&vi->refill, 0);
> -			spin_unlock(&vi->refill_lock);
> -		}
> +		if (!try_fill_recv(vi, rq, GFP_ATOMIC))
> +			*retry_refill = true;
>  	}
>  
>  	u64_stats_set(&stats.packets, packets);

So this function sets retry_refill to true but assumes caller
will set it to false? seems unnecessarily complex.
just have to always set retry_refill correctly
and not rely on the caller.


> @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>  	struct send_queue *sq;
>  	unsigned int received;
>  	unsigned int xdp_xmit = 0;
> -	bool napi_complete;
> +	bool napi_complete, retry_refill = false;
>  
>  	virtnet_poll_cleantx(rq, budget);
>  
> -	received = virtnet_receive(rq, budget, &xdp_xmit);
> +	received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
>  	rq->packets_in_napi += received;
>  
>  	if (xdp_xmit & VIRTIO_XDP_REDIR)
>  		xdp_do_flush();
>  
>  	/* Out of packets? */
> -	if (received < budget) {
> +	if (received < budget && !retry_refill) {
>  		napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
>  		/* Intentionally not taking dim_lock here. This may result in a
>  		 * spurious net_dim call. But if that happens virtnet_rx_dim_work
> @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>  		virtnet_xdp_put_sq(vi, sq);
>  	}
>  
> -	return received;
> +	return retry_refill ? budget : received;

a comment can't hurt here, to document what is going on.

>  }
>  
>  static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
> @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
>  
>  	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		if (i < vi->curr_queue_pairs)
> -			/* Make sure we have some buffers: if oom use wq. */
> -			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> -				schedule_delayed_work(&vi->refill, 0);
> +			/* If this fails, we will retry later in
> +			 * NAPI poll, which is scheduled in the below
> +			 * virtnet_enable_queue_pair
> +			 */
> +			try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
>  
>  		err = virtnet_enable_queue_pair(vi, i);
>  		if (err < 0)
> @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
>  				bool refill)
>  {
>  	bool running = netif_running(vi->dev);
> -	bool schedule_refill = false;
>  
> -	if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
> -		schedule_refill = true;
> +	if (refill)
> +		/* If this fails, we will retry later in NAPI poll, which is
> +		 * scheduled in the below virtnet_napi_enable
> +		 */
> +		try_fill_recv(vi, rq, GFP_KERNEL);
> +
>  	if (running)
>  		virtnet_napi_enable(rq);
> -
> -	if (schedule_refill)
> -		schedule_delayed_work(&vi->refill, 0);
>  }
>  
>  static void virtnet_rx_resume_all(struct virtnet_info *vi)
> @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>  	struct virtio_net_rss_config_trailer old_rss_trailer;
>  	struct net_device *dev = vi->dev;
>  	struct scatterlist sg;
> +	int i;
>  
>  	if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
>  		return 0;
> @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>  	}
>  succ:
>  	vi->curr_queue_pairs = queue_pairs;
> -	/* virtnet_open() will refill when device is going to up. */
> -	spin_lock_bh(&vi->refill_lock);
> -	if (dev->flags & IFF_UP && vi->refill_enabled)
> -		schedule_delayed_work(&vi->refill, 0);
> -	spin_unlock_bh(&vi->refill_lock);
> +	if (dev->flags & IFF_UP) {
> +		/* Let the NAPI poll refill the receive buffer for us. We can't
> +		 * safely call try_fill_recv() here because the NAPI might be
> +		 * enabled already.
> +		 */
> +		local_bh_disable();
> +		for (i = 0; i < vi->curr_queue_pairs; i++)

you cam declare i here in the for loop.
and ++i is a bit clearer.

> +			virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
> +
> +		local_bh_enable();
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.43.0

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Michael S. Tsirkin]

On Fri, Jan 02, 2026 at 10:20:21PM +0700, Bui Quang Minh wrote:
> When we fail to refill the receive buffers, we schedule a delayed worker
> to retry later. However, this worker creates some concurrency issues
> such as races and deadlocks. To simplify the logic and avoid further
> problems, we will instead retry refilling in the next NAPI poll.
> 
> Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
> Reported-by: Paolo Abeni <pabeni@redhat.com>
> Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
> Cc: stable@vger.kernel.org
> Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> ---
>  drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1bb3aeca66c6..ac514c9383ae 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
>  }
>  
>  static int virtnet_receive(struct receive_queue *rq, int budget,
> -			   unsigned int *xdp_xmit)
> +			   unsigned int *xdp_xmit, bool *retry_refill)
>  {
>  	struct virtnet_info *vi = rq->vq->vdev->priv;
>  	struct virtnet_rq_stats stats = {};
> @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>  		packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
>  
>  	if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
> -		if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
> -			spin_lock(&vi->refill_lock);
> -			if (vi->refill_enabled)
> -				schedule_delayed_work(&vi->refill, 0);
> -			spin_unlock(&vi->refill_lock);
> -		}
> +		if (!try_fill_recv(vi, rq, GFP_ATOMIC))
> +			*retry_refill = true;
>  	}
>  
>  	u64_stats_set(&stats.packets, packets);
> @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>  	struct send_queue *sq;
>  	unsigned int received;
>  	unsigned int xdp_xmit = 0;
> -	bool napi_complete;
> +	bool napi_complete, retry_refill = false;
>  
>  	virtnet_poll_cleantx(rq, budget);
>  
> -	received = virtnet_receive(rq, budget, &xdp_xmit);
> +	received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
>  	rq->packets_in_napi += received;
>  
>  	if (xdp_xmit & VIRTIO_XDP_REDIR)
>  		xdp_do_flush();
>  
>  	/* Out of packets? */
> -	if (received < budget) {
> +	if (received < budget && !retry_refill) {
>  		napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
>  		/* Intentionally not taking dim_lock here. This may result in a
>  		 * spurious net_dim call. But if that happens virtnet_rx_dim_work
> @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>  		virtnet_xdp_put_sq(vi, sq);
>  	}
>  
> -	return received;
> +	return retry_refill ? budget : received;
>  }
>  
>  static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
> @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
>  
>  	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		if (i < vi->curr_queue_pairs)
> -			/* Make sure we have some buffers: if oom use wq. */
> -			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> -				schedule_delayed_work(&vi->refill, 0);
> +			/* If this fails, we will retry later in
> +			 * NAPI poll, which is scheduled in the below
> +			 * virtnet_enable_queue_pair

hmm do we even need this, then?

> +			 */
> +			try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
>  
>  		err = virtnet_enable_queue_pair(vi, i);
>  		if (err < 0)
> @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
>  				bool refill)
>  {
>  	bool running = netif_running(vi->dev);
> -	bool schedule_refill = false;
>  
> -	if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
> -		schedule_refill = true;
> +	if (refill)
> +		/* If this fails, we will retry later in NAPI poll, which is
> +		 * scheduled in the below virtnet_napi_enable
> +		 */
> +		try_fill_recv(vi, rq, GFP_KERNEL);


hmm do we even need this, then?

> +
>  	if (running)
>  		virtnet_napi_enable(rq);
> -
> -	if (schedule_refill)
> -		schedule_delayed_work(&vi->refill, 0);
>  }
>  
>  static void virtnet_rx_resume_all(struct virtnet_info *vi)
> @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>  	struct virtio_net_rss_config_trailer old_rss_trailer;
>  	struct net_device *dev = vi->dev;
>  	struct scatterlist sg;
> +	int i;
>  
>  	if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
>  		return 0;
> @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>  	}
>  succ:
>  	vi->curr_queue_pairs = queue_pairs;
> -	/* virtnet_open() will refill when device is going to up. */
> -	spin_lock_bh(&vi->refill_lock);
> -	if (dev->flags & IFF_UP && vi->refill_enabled)
> -		schedule_delayed_work(&vi->refill, 0);
> -	spin_unlock_bh(&vi->refill_lock);
> +	if (dev->flags & IFF_UP) {
> +		/* Let the NAPI poll refill the receive buffer for us. We can't
> +		 * safely call try_fill_recv() here because the NAPI might be
> +		 * enabled already.
> +		 */

I'd drop this comment, it does not clarify much.

> +		local_bh_disable();
> +		for (i = 0; i < vi->curr_queue_pairs; i++)
> +			virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
> +
> +		local_bh_enable();
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.43.0

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Michael S. Tsirkin]

On Sat, Jan 03, 2026 at 11:57:43AM -0500, Michael S. Tsirkin wrote:
> On Fri, Jan 02, 2026 at 10:20:21PM +0700, Bui Quang Minh wrote:
> > When we fail to refill the receive buffers, we schedule a delayed worker
> > to retry later. However, this worker creates some concurrency issues
> > such as races and deadlocks. To simplify the logic and avoid further
> > problems, we will instead retry refilling in the next NAPI poll.
> > 
> > Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
> > Reported-by: Paolo Abeni <pabeni@redhat.com>
> > Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
> > Cc: stable@vger.kernel.org
> > Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> > ---
> >  drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
> >  1 file changed, 30 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 1bb3aeca66c6..ac514c9383ae 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
> >  }
> >  
> >  static int virtnet_receive(struct receive_queue *rq, int budget,
> > -			   unsigned int *xdp_xmit)
> > +			   unsigned int *xdp_xmit, bool *retry_refill)
> >  {
> >  	struct virtnet_info *vi = rq->vq->vdev->priv;
> >  	struct virtnet_rq_stats stats = {};
> > @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
> >  		packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
> >  
> >  	if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
> > -		if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
> > -			spin_lock(&vi->refill_lock);
> > -			if (vi->refill_enabled)
> > -				schedule_delayed_work(&vi->refill, 0);
> > -			spin_unlock(&vi->refill_lock);
> > -		}
> > +		if (!try_fill_recv(vi, rq, GFP_ATOMIC))
> > +			*retry_refill = true;
> >  	}
> >  
> >  	u64_stats_set(&stats.packets, packets);
> > @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> >  	struct send_queue *sq;
> >  	unsigned int received;
> >  	unsigned int xdp_xmit = 0;
> > -	bool napi_complete;
> > +	bool napi_complete, retry_refill = false;
> >  
> >  	virtnet_poll_cleantx(rq, budget);
> >  
> > -	received = virtnet_receive(rq, budget, &xdp_xmit);
> > +	received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
> >  	rq->packets_in_napi += received;
> >  
> >  	if (xdp_xmit & VIRTIO_XDP_REDIR)
> >  		xdp_do_flush();
> >  
> >  	/* Out of packets? */
> > -	if (received < budget) {
> > +	if (received < budget && !retry_refill) {
> >  		napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
> >  		/* Intentionally not taking dim_lock here. This may result in a
> >  		 * spurious net_dim call. But if that happens virtnet_rx_dim_work
> > @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> >  		virtnet_xdp_put_sq(vi, sq);
> >  	}
> >  
> > -	return received;
> > +	return retry_refill ? budget : received;
> >  }
> >  
> >  static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
> > @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
> >  
> >  	for (i = 0; i < vi->max_queue_pairs; i++) {
> >  		if (i < vi->curr_queue_pairs)
> > -			/* Make sure we have some buffers: if oom use wq. */
> > -			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> > -				schedule_delayed_work(&vi->refill, 0);
> > +			/* If this fails, we will retry later in
> > +			 * NAPI poll, which is scheduled in the below
> > +			 * virtnet_enable_queue_pair
> 
> hmm do we even need this, then?


Oh this one is GFP_KERNEL. So it makes it more likely the ring will
be filled.

> > +			 */
> > +			try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
> >  
> >  		err = virtnet_enable_queue_pair(vi, i);
> >  		if (err < 0)
> > @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
> >  				bool refill)
> >  {
> >  	bool running = netif_running(vi->dev);
> > -	bool schedule_refill = false;
> >  
> > -	if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
> > -		schedule_refill = true;
> > +	if (refill)
> > +		/* If this fails, we will retry later in NAPI poll, which is
> > +		 * scheduled in the below virtnet_napi_enable
> > +		 */
> > +		try_fill_recv(vi, rq, GFP_KERNEL);
> 
> 
> hmm do we even need this, then?
> 
> > +
> >  	if (running)
> >  		virtnet_napi_enable(rq);
> > -
> > -	if (schedule_refill)
> > -		schedule_delayed_work(&vi->refill, 0);
> >  }
> >  
> >  static void virtnet_rx_resume_all(struct virtnet_info *vi)
> > @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> >  	struct virtio_net_rss_config_trailer old_rss_trailer;
> >  	struct net_device *dev = vi->dev;
> >  	struct scatterlist sg;
> > +	int i;
> >  
> >  	if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
> >  		return 0;
> > @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> >  	}
> >  succ:
> >  	vi->curr_queue_pairs = queue_pairs;
> > -	/* virtnet_open() will refill when device is going to up. */
> > -	spin_lock_bh(&vi->refill_lock);
> > -	if (dev->flags & IFF_UP && vi->refill_enabled)
> > -		schedule_delayed_work(&vi->refill, 0);
> > -	spin_unlock_bh(&vi->refill_lock);
> > +	if (dev->flags & IFF_UP) {
> > +		/* Let the NAPI poll refill the receive buffer for us. We can't
> > +		 * safely call try_fill_recv() here because the NAPI might be
> > +		 * enabled already.
> > +		 */
> 
> I'd drop this comment, it does not clarify much.
> 
> > +		local_bh_disable();
> > +		for (i = 0; i < vi->curr_queue_pairs; i++)
> > +			virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
> > +
> > +		local_bh_enable();
> > +	}
> >  
> >  	return 0;
> >  }
> > -- 
> > 2.43.0

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Bui Quang Minh]

On 1/3/26 23:57, Michael S. Tsirkin wrote:
> On Fri, Jan 02, 2026 at 10:20:21PM +0700, Bui Quang Minh wrote:
>> When we fail to refill the receive buffers, we schedule a delayed worker
>> to retry later. However, this worker creates some concurrency issues
>> such as races and deadlocks. To simplify the logic and avoid further
>> problems, we will instead retry refilling in the next NAPI poll.
>>
>> Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
>> Reported-by: Paolo Abeni <pabeni@redhat.com>
>> Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
>> Cc: stable@vger.kernel.org
>> Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
>> ---
>>   drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
>>   1 file changed, 30 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 1bb3aeca66c6..ac514c9383ae 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
>>   }
>>   
>>   static int virtnet_receive(struct receive_queue *rq, int budget,
>> -			   unsigned int *xdp_xmit)
>> +			   unsigned int *xdp_xmit, bool *retry_refill)
>>   {
>>   	struct virtnet_info *vi = rq->vq->vdev->priv;
>>   	struct virtnet_rq_stats stats = {};
>> @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>>   		packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
>>   
>>   	if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
>> -		if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
>> -			spin_lock(&vi->refill_lock);
>> -			if (vi->refill_enabled)
>> -				schedule_delayed_work(&vi->refill, 0);
>> -			spin_unlock(&vi->refill_lock);
>> -		}
>> +		if (!try_fill_recv(vi, rq, GFP_ATOMIC))
>> +			*retry_refill = true;
>>   	}
>>   
>>   	u64_stats_set(&stats.packets, packets);
>> @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>>   	struct send_queue *sq;
>>   	unsigned int received;
>>   	unsigned int xdp_xmit = 0;
>> -	bool napi_complete;
>> +	bool napi_complete, retry_refill = false;
>>   
>>   	virtnet_poll_cleantx(rq, budget);
>>   
>> -	received = virtnet_receive(rq, budget, &xdp_xmit);
>> +	received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
>>   	rq->packets_in_napi += received;
>>   
>>   	if (xdp_xmit & VIRTIO_XDP_REDIR)
>>   		xdp_do_flush();
>>   
>>   	/* Out of packets? */
>> -	if (received < budget) {
>> +	if (received < budget && !retry_refill) {
>>   		napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
>>   		/* Intentionally not taking dim_lock here. This may result in a
>>   		 * spurious net_dim call. But if that happens virtnet_rx_dim_work
>> @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>>   		virtnet_xdp_put_sq(vi, sq);
>>   	}
>>   
>> -	return received;
>> +	return retry_refill ? budget : received;
>>   }
>>   
>>   static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
>> @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
>>   
>>   	for (i = 0; i < vi->max_queue_pairs; i++) {
>>   		if (i < vi->curr_queue_pairs)
>> -			/* Make sure we have some buffers: if oom use wq. */
>> -			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
>> -				schedule_delayed_work(&vi->refill, 0);
>> +			/* If this fails, we will retry later in
>> +			 * NAPI poll, which is scheduled in the below
>> +			 * virtnet_enable_queue_pair
> hmm do we even need this, then?
>
>> +			 */
>> +			try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
>>   
>>   		err = virtnet_enable_queue_pair(vi, i);
>>   		if (err < 0)
>> @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
>>   				bool refill)
>>   {
>>   	bool running = netif_running(vi->dev);
>> -	bool schedule_refill = false;
>>   
>> -	if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
>> -		schedule_refill = true;
>> +	if (refill)
>> +		/* If this fails, we will retry later in NAPI poll, which is
>> +		 * scheduled in the below virtnet_napi_enable
>> +		 */
>> +		try_fill_recv(vi, rq, GFP_KERNEL);
>
> hmm do we even need this, then?
>
>> +
>>   	if (running)
>>   		virtnet_napi_enable(rq);
>> -
>> -	if (schedule_refill)
>> -		schedule_delayed_work(&vi->refill, 0);
>>   }
>>   
>>   static void virtnet_rx_resume_all(struct virtnet_info *vi)
>> @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>>   	struct virtio_net_rss_config_trailer old_rss_trailer;
>>   	struct net_device *dev = vi->dev;
>>   	struct scatterlist sg;
>> +	int i;
>>   
>>   	if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
>>   		return 0;
>> @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>>   	}
>>   succ:
>>   	vi->curr_queue_pairs = queue_pairs;
>> -	/* virtnet_open() will refill when device is going to up. */
>> -	spin_lock_bh(&vi->refill_lock);
>> -	if (dev->flags & IFF_UP && vi->refill_enabled)
>> -		schedule_delayed_work(&vi->refill, 0);
>> -	spin_unlock_bh(&vi->refill_lock);
>> +	if (dev->flags & IFF_UP) {
>> +		/* Let the NAPI poll refill the receive buffer for us. We can't
>> +		 * safely call try_fill_recv() here because the NAPI might be
>> +		 * enabled already.
>> +		 */
> I'd drop this comment, it does not clarify much.

Here I want to note on why we can't use try_fill_recv like in other places.

>
>> +		local_bh_disable();
>> +		for (i = 0; i < vi->curr_queue_pairs; i++)
>> +			virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
>> +
>> +		local_bh_enable();
>> +	}
>>   
>>   	return 0;
>>   }
>> -- 
>> 2.43.0

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Michael S. Tsirkin]

On Sun, Jan 04, 2026 at 11:33:20AM +0700, Bui Quang Minh wrote:
> On 1/3/26 23:57, Michael S. Tsirkin wrote:
> > On Fri, Jan 02, 2026 at 10:20:21PM +0700, Bui Quang Minh wrote:
> > > When we fail to refill the receive buffers, we schedule a delayed worker
> > > to retry later. However, this worker creates some concurrency issues
> > > such as races and deadlocks. To simplify the logic and avoid further
> > > problems, we will instead retry refilling in the next NAPI poll.
> > > 
> > > Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
> > > Reported-by: Paolo Abeni <pabeni@redhat.com>
> > > Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
> > > Cc: stable@vger.kernel.org
> > > Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> > > ---
> > >   drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
> > >   1 file changed, 30 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 1bb3aeca66c6..ac514c9383ae 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
> > >   }
> > >   static int virtnet_receive(struct receive_queue *rq, int budget,
> > > -			   unsigned int *xdp_xmit)
> > > +			   unsigned int *xdp_xmit, bool *retry_refill)
> > >   {
> > >   	struct virtnet_info *vi = rq->vq->vdev->priv;
> > >   	struct virtnet_rq_stats stats = {};
> > > @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
> > >   		packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
> > >   	if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
> > > -		if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
> > > -			spin_lock(&vi->refill_lock);
> > > -			if (vi->refill_enabled)
> > > -				schedule_delayed_work(&vi->refill, 0);
> > > -			spin_unlock(&vi->refill_lock);
> > > -		}
> > > +		if (!try_fill_recv(vi, rq, GFP_ATOMIC))
> > > +			*retry_refill = true;
> > >   	}
> > >   	u64_stats_set(&stats.packets, packets);
> > > @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> > >   	struct send_queue *sq;
> > >   	unsigned int received;
> > >   	unsigned int xdp_xmit = 0;
> > > -	bool napi_complete;
> > > +	bool napi_complete, retry_refill = false;
> > >   	virtnet_poll_cleantx(rq, budget);
> > > -	received = virtnet_receive(rq, budget, &xdp_xmit);
> > > +	received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
> > >   	rq->packets_in_napi += received;
> > >   	if (xdp_xmit & VIRTIO_XDP_REDIR)
> > >   		xdp_do_flush();
> > >   	/* Out of packets? */
> > > -	if (received < budget) {
> > > +	if (received < budget && !retry_refill) {
> > >   		napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
> > >   		/* Intentionally not taking dim_lock here. This may result in a
> > >   		 * spurious net_dim call. But if that happens virtnet_rx_dim_work
> > > @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> > >   		virtnet_xdp_put_sq(vi, sq);
> > >   	}
> > > -	return received;
> > > +	return retry_refill ? budget : received;
> > >   }
> > >   static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
> > > @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
> > >   	for (i = 0; i < vi->max_queue_pairs; i++) {
> > >   		if (i < vi->curr_queue_pairs)
> > > -			/* Make sure we have some buffers: if oom use wq. */
> > > -			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> > > -				schedule_delayed_work(&vi->refill, 0);
> > > +			/* If this fails, we will retry later in
> > > +			 * NAPI poll, which is scheduled in the below
> > > +			 * virtnet_enable_queue_pair
> > hmm do we even need this, then?
> > 
> > > +			 */
> > > +			try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
> > >   		err = virtnet_enable_queue_pair(vi, i);
> > >   		if (err < 0)
> > > @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
> > >   				bool refill)
> > >   {
> > >   	bool running = netif_running(vi->dev);
> > > -	bool schedule_refill = false;
> > > -	if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
> > > -		schedule_refill = true;
> > > +	if (refill)
> > > +		/* If this fails, we will retry later in NAPI poll, which is
> > > +		 * scheduled in the below virtnet_napi_enable
> > > +		 */
> > > +		try_fill_recv(vi, rq, GFP_KERNEL);
> > 
> > hmm do we even need this, then?
> > 
> > > +
> > >   	if (running)
> > >   		virtnet_napi_enable(rq);
> > > -
> > > -	if (schedule_refill)
> > > -		schedule_delayed_work(&vi->refill, 0);
> > >   }
> > >   static void virtnet_rx_resume_all(struct virtnet_info *vi)
> > > @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> > >   	struct virtio_net_rss_config_trailer old_rss_trailer;
> > >   	struct net_device *dev = vi->dev;
> > >   	struct scatterlist sg;
> > > +	int i;
> > >   	if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
> > >   		return 0;
> > > @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> > >   	}
> > >   succ:
> > >   	vi->curr_queue_pairs = queue_pairs;
> > > -	/* virtnet_open() will refill when device is going to up. */
> > > -	spin_lock_bh(&vi->refill_lock);
> > > -	if (dev->flags & IFF_UP && vi->refill_enabled)
> > > -		schedule_delayed_work(&vi->refill, 0);
> > > -	spin_unlock_bh(&vi->refill_lock);
> > > +	if (dev->flags & IFF_UP) {
> > > +		/* Let the NAPI poll refill the receive buffer for us. We can't
> > > +		 * safely call try_fill_recv() here because the NAPI might be
> > > +		 * enabled already.
> > > +		 */
> > I'd drop this comment, it does not clarify much.
> 
> Here I want to note on why we can't use try_fill_recv like in other places.

It does not really answer the question why, you could just disable napi
if you wanted.

> > 
> > > +		local_bh_disable();
> > > +		for (i = 0; i < vi->curr_queue_pairs; i++)
> > > +			virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
> > > +
> > > +		local_bh_enable();
> > > +	}
> > >   	return 0;
> > >   }
> > > -- 
> > > 2.43.0

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Jason Wang]

On Fri, Jan 2, 2026 at 11:20 PM Bui Quang Minh <minhquangbui99@gmail.com> wrote:
>
> When we fail to refill the receive buffers, we schedule a delayed worker
> to retry later. However, this worker creates some concurrency issues
> such as races and deadlocks. To simplify the logic and avoid further
> problems, we will instead retry refilling in the next NAPI poll.
>
> Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
> Reported-by: Paolo Abeni <pabeni@redhat.com>
> Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
> Cc: stable@vger.kernel.org
> Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> ---
>  drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1bb3aeca66c6..ac514c9383ae 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
>  }
>
>  static int virtnet_receive(struct receive_queue *rq, int budget,
> -                          unsigned int *xdp_xmit)
> +                          unsigned int *xdp_xmit, bool *retry_refill)
>  {
>         struct virtnet_info *vi = rq->vq->vdev->priv;
>         struct virtnet_rq_stats stats = {};
> @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>                 packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
>
>         if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
> -               if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
> -                       spin_lock(&vi->refill_lock);
> -                       if (vi->refill_enabled)
> -                               schedule_delayed_work(&vi->refill, 0);
> -                       spin_unlock(&vi->refill_lock);
> -               }
> +               if (!try_fill_recv(vi, rq, GFP_ATOMIC))
> +                       *retry_refill = true;
>         }
>
>         u64_stats_set(&stats.packets, packets);
> @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>         struct send_queue *sq;
>         unsigned int received;
>         unsigned int xdp_xmit = 0;
> -       bool napi_complete;
> +       bool napi_complete, retry_refill = false;
>
>         virtnet_poll_cleantx(rq, budget);
>
> -       received = virtnet_receive(rq, budget, &xdp_xmit);
> +       received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);

I think we can simply let virtnet_receive() to return the budget when
reill fails.

>         rq->packets_in_napi += received;
>
>         if (xdp_xmit & VIRTIO_XDP_REDIR)
>                 xdp_do_flush();
>
>         /* Out of packets? */
> -       if (received < budget) {
> +       if (received < budget && !retry_refill) {
>                 napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
>                 /* Intentionally not taking dim_lock here. This may result in a
>                  * spurious net_dim call. But if that happens virtnet_rx_dim_work
> @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>                 virtnet_xdp_put_sq(vi, sq);
>         }
>
> -       return received;
> +       return retry_refill ? budget : received;
>  }
>
>  static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
> @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
>
>         for (i = 0; i < vi->max_queue_pairs; i++) {
>                 if (i < vi->curr_queue_pairs)
> -                       /* Make sure we have some buffers: if oom use wq. */
> -                       if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> -                               schedule_delayed_work(&vi->refill, 0);
> +                       /* If this fails, we will retry later in
> +                        * NAPI poll, which is scheduled in the below
> +                        * virtnet_enable_queue_pair
> +                        */
> +                       try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);

Consider NAPI will be eventually scheduled, I wonder if it's still
worth to refill here.

>
>                 err = virtnet_enable_queue_pair(vi, i);
>                 if (err < 0)
> @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
>                                 bool refill)
>  {
>         bool running = netif_running(vi->dev);
> -       bool schedule_refill = false;
>
> -       if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
> -               schedule_refill = true;
> +       if (refill)
> +               /* If this fails, we will retry later in NAPI poll, which is
> +                * scheduled in the below virtnet_napi_enable
> +                */
> +               try_fill_recv(vi, rq, GFP_KERNEL);

and here.

> +
>         if (running)
>                 virtnet_napi_enable(rq);
> -
> -       if (schedule_refill)
> -               schedule_delayed_work(&vi->refill, 0);
>  }
>
>  static void virtnet_rx_resume_all(struct virtnet_info *vi)
> @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>         struct virtio_net_rss_config_trailer old_rss_trailer;
>         struct net_device *dev = vi->dev;
>         struct scatterlist sg;
> +       int i;
>
>         if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
>                 return 0;
> @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>         }
>  succ:
>         vi->curr_queue_pairs = queue_pairs;
> -       /* virtnet_open() will refill when device is going to up. */
> -       spin_lock_bh(&vi->refill_lock);
> -       if (dev->flags & IFF_UP && vi->refill_enabled)
> -               schedule_delayed_work(&vi->refill, 0);
> -       spin_unlock_bh(&vi->refill_lock);
> +       if (dev->flags & IFF_UP) {
> +               /* Let the NAPI poll refill the receive buffer for us. We can't
> +                * safely call try_fill_recv() here because the NAPI might be
> +                * enabled already.
> +                */
> +               local_bh_disable();
> +               for (i = 0; i < vi->curr_queue_pairs; i++)
> +                       virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
> +
> +               local_bh_enable();
> +       }
>
>         return 0;
>  }
> --
> 2.43.0
>

Thanks

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Bui Quang Minh]

On 1/4/26 13:09, Jason Wang wrote:
> On Fri, Jan 2, 2026 at 11:20 PM Bui Quang Minh <minhquangbui99@gmail.com> wrote:
>> When we fail to refill the receive buffers, we schedule a delayed worker
>> to retry later. However, this worker creates some concurrency issues
>> such as races and deadlocks. To simplify the logic and avoid further
>> problems, we will instead retry refilling in the next NAPI poll.
>>
>> Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
>> Reported-by: Paolo Abeni <pabeni@redhat.com>
>> Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
>> Cc: stable@vger.kernel.org
>> Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
>> ---
>>   drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
>>   1 file changed, 30 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 1bb3aeca66c6..ac514c9383ae 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
>>   }
>>
>>   static int virtnet_receive(struct receive_queue *rq, int budget,
>> -                          unsigned int *xdp_xmit)
>> +                          unsigned int *xdp_xmit, bool *retry_refill)
>>   {
>>          struct virtnet_info *vi = rq->vq->vdev->priv;
>>          struct virtnet_rq_stats stats = {};
>> @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>>                  packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
>>
>>          if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
>> -               if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
>> -                       spin_lock(&vi->refill_lock);
>> -                       if (vi->refill_enabled)
>> -                               schedule_delayed_work(&vi->refill, 0);
>> -                       spin_unlock(&vi->refill_lock);
>> -               }
>> +               if (!try_fill_recv(vi, rq, GFP_ATOMIC))
>> +                       *retry_refill = true;
>>          }
>>
>>          u64_stats_set(&stats.packets, packets);
>> @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>>          struct send_queue *sq;
>>          unsigned int received;
>>          unsigned int xdp_xmit = 0;
>> -       bool napi_complete;
>> +       bool napi_complete, retry_refill = false;
>>
>>          virtnet_poll_cleantx(rq, budget);
>>
>> -       received = virtnet_receive(rq, budget, &xdp_xmit);
>> +       received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
> I think we can simply let virtnet_receive() to return the budget when
> reill fails.

That makes sense, I'll change it.

>
>>          rq->packets_in_napi += received;
>>
>>          if (xdp_xmit & VIRTIO_XDP_REDIR)
>>                  xdp_do_flush();
>>
>>          /* Out of packets? */
>> -       if (received < budget) {
>> +       if (received < budget && !retry_refill) {
>>                  napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
>>                  /* Intentionally not taking dim_lock here. This may result in a
>>                   * spurious net_dim call. But if that happens virtnet_rx_dim_work
>> @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>>                  virtnet_xdp_put_sq(vi, sq);
>>          }
>>
>> -       return received;
>> +       return retry_refill ? budget : received;
>>   }
>>
>>   static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
>> @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
>>
>>          for (i = 0; i < vi->max_queue_pairs; i++) {
>>                  if (i < vi->curr_queue_pairs)
>> -                       /* Make sure we have some buffers: if oom use wq. */
>> -                       if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
>> -                               schedule_delayed_work(&vi->refill, 0);
>> +                       /* If this fails, we will retry later in
>> +                        * NAPI poll, which is scheduled in the below
>> +                        * virtnet_enable_queue_pair
>> +                        */
>> +                       try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
> Consider NAPI will be eventually scheduled, I wonder if it's still
> worth to refill here.

With GFP_KERNEL here, I think it's more likely to succeed than 
GFP_ATOMIC in NAPI poll. Another small benefit is that the actual packet 
can happen earlier. In case the receive buffer is empty and we don't 
refill here, the #1 NAPI poll refill the buffer and the #2 NAPI poll can 
receive packets. The #2 NAPI poll is scheduled in the interrupt handler 
because the #1 NAPI poll will deschedule the NAPI and enable the device 
interrupt. In case we successfully refill here, the #1 NAPI poll can 
receive packets right away.


>
>>                  err = virtnet_enable_queue_pair(vi, i);
>>                  if (err < 0)
>> @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
>>                                  bool refill)
>>   {
>>          bool running = netif_running(vi->dev);
>> -       bool schedule_refill = false;
>>
>> -       if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
>> -               schedule_refill = true;
>> +       if (refill)
>> +               /* If this fails, we will retry later in NAPI poll, which is
>> +                * scheduled in the below virtnet_napi_enable
>> +                */
>> +               try_fill_recv(vi, rq, GFP_KERNEL);
> and here.
>
>> +
>>          if (running)
>>                  virtnet_napi_enable(rq);
>> -
>> -       if (schedule_refill)
>> -               schedule_delayed_work(&vi->refill, 0);
>>   }
>>
>>   static void virtnet_rx_resume_all(struct virtnet_info *vi)
>> @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>>          struct virtio_net_rss_config_trailer old_rss_trailer;
>>          struct net_device *dev = vi->dev;
>>          struct scatterlist sg;
>> +       int i;
>>
>>          if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
>>                  return 0;
>> @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>>          }
>>   succ:
>>          vi->curr_queue_pairs = queue_pairs;
>> -       /* virtnet_open() will refill when device is going to up. */
>> -       spin_lock_bh(&vi->refill_lock);
>> -       if (dev->flags & IFF_UP && vi->refill_enabled)
>> -               schedule_delayed_work(&vi->refill, 0);
>> -       spin_unlock_bh(&vi->refill_lock);
>> +       if (dev->flags & IFF_UP) {
>> +               /* Let the NAPI poll refill the receive buffer for us. We can't
>> +                * safely call try_fill_recv() here because the NAPI might be
>> +                * enabled already.
>> +                */
>> +               local_bh_disable();
>> +               for (i = 0; i < vi->curr_queue_pairs; i++)
>> +                       virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
>> +
>> +               local_bh_enable();
>> +       }
>>
>>          return 0;
>>   }
>> --
>> 2.43.0
>>
> Thanks
>

Thanks,
Quang Minh.

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Michael S. Tsirkin]

On Sun, Jan 04, 2026 at 03:34:52PM +0700, Bui Quang Minh wrote:
> On 1/4/26 13:09, Jason Wang wrote:
> > On Fri, Jan 2, 2026 at 11:20 PM Bui Quang Minh <minhquangbui99@gmail.com> wrote:
> > > When we fail to refill the receive buffers, we schedule a delayed worker
> > > to retry later. However, this worker creates some concurrency issues
> > > such as races and deadlocks. To simplify the logic and avoid further
> > > problems, we will instead retry refilling in the next NAPI poll.
> > > 
> > > Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
> > > Reported-by: Paolo Abeni <pabeni@redhat.com>
> > > Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
> > > Cc: stable@vger.kernel.org
> > > Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> > > ---
> > >   drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
> > >   1 file changed, 30 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 1bb3aeca66c6..ac514c9383ae 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
> > >   }
> > > 
> > >   static int virtnet_receive(struct receive_queue *rq, int budget,
> > > -                          unsigned int *xdp_xmit)
> > > +                          unsigned int *xdp_xmit, bool *retry_refill)
> > >   {
> > >          struct virtnet_info *vi = rq->vq->vdev->priv;
> > >          struct virtnet_rq_stats stats = {};
> > > @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
> > >                  packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
> > > 
> > >          if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
> > > -               if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
> > > -                       spin_lock(&vi->refill_lock);
> > > -                       if (vi->refill_enabled)
> > > -                               schedule_delayed_work(&vi->refill, 0);
> > > -                       spin_unlock(&vi->refill_lock);
> > > -               }
> > > +               if (!try_fill_recv(vi, rq, GFP_ATOMIC))
> > > +                       *retry_refill = true;
> > >          }
> > > 
> > >          u64_stats_set(&stats.packets, packets);
> > > @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> > >          struct send_queue *sq;
> > >          unsigned int received;
> > >          unsigned int xdp_xmit = 0;
> > > -       bool napi_complete;
> > > +       bool napi_complete, retry_refill = false;
> > > 
> > >          virtnet_poll_cleantx(rq, budget);
> > > 
> > > -       received = virtnet_receive(rq, budget, &xdp_xmit);
> > > +       received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
> > I think we can simply let virtnet_receive() to return the budget when
> > reill fails.
> 
> That makes sense, I'll change it.
> 
> > 
> > >          rq->packets_in_napi += received;
> > > 
> > >          if (xdp_xmit & VIRTIO_XDP_REDIR)
> > >                  xdp_do_flush();
> > > 
> > >          /* Out of packets? */
> > > -       if (received < budget) {
> > > +       if (received < budget && !retry_refill) {
> > >                  napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
> > >                  /* Intentionally not taking dim_lock here. This may result in a
> > >                   * spurious net_dim call. But if that happens virtnet_rx_dim_work
> > > @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> > >                  virtnet_xdp_put_sq(vi, sq);
> > >          }
> > > 
> > > -       return received;
> > > +       return retry_refill ? budget : received;
> > >   }
> > > 
> > >   static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
> > > @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
> > > 
> > >          for (i = 0; i < vi->max_queue_pairs; i++) {
> > >                  if (i < vi->curr_queue_pairs)
> > > -                       /* Make sure we have some buffers: if oom use wq. */
> > > -                       if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> > > -                               schedule_delayed_work(&vi->refill, 0);
> > > +                       /* If this fails, we will retry later in
> > > +                        * NAPI poll, which is scheduled in the below
> > > +                        * virtnet_enable_queue_pair
> > > +                        */
> > > +                       try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
> > Consider NAPI will be eventually scheduled, I wonder if it's still
> > worth to refill here.
> 
> With GFP_KERNEL here, I think it's more likely to succeed than GFP_ATOMIC in
> NAPI poll. Another small benefit is that the actual packet can happen
> earlier. In case the receive buffer is empty and we don't refill here, the
> #1 NAPI poll refill the buffer and the #2 NAPI poll can receive packets. The
> #2 NAPI poll is scheduled in the interrupt handler because the #1 NAPI poll
> will deschedule the NAPI and enable the device interrupt. In case we
> successfully refill here, the #1 NAPI poll can receive packets right away.
> 

Right. But I think this is a part that needs elucidating, not
error handling.

/* Pre-fill rq agressively, to make sure we are ready to get packets
 * immediately.
 * */

> > 
> > >                  err = virtnet_enable_queue_pair(vi, i);
> > >                  if (err < 0)
> > > @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
> > >                                  bool refill)
> > >   {
> > >          bool running = netif_running(vi->dev);
> > > -       bool schedule_refill = false;
> > > 
> > > -       if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
> > > -               schedule_refill = true;
> > > +       if (refill)
> > > +               /* If this fails, we will retry later in NAPI poll, which is
> > > +                * scheduled in the below virtnet_napi_enable
> > > +                */
> > > +               try_fill_recv(vi, rq, GFP_KERNEL);
> > and here.
> > 
> > > +
> > >          if (running)
> > >                  virtnet_napi_enable(rq);

here the part that isn't clear is why are we refilling if !running
and what handles failures in that case.

> > > -
> > > -       if (schedule_refill)
> > > -               schedule_delayed_work(&vi->refill, 0);
> > >   }
> > > 
> > >   static void virtnet_rx_resume_all(struct virtnet_info *vi)
> > > @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> > >          struct virtio_net_rss_config_trailer old_rss_trailer;
> > >          struct net_device *dev = vi->dev;
> > >          struct scatterlist sg;
> > > +       int i;
> > > 
> > >          if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
> > >                  return 0;
> > > @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> > >          }
> > >   succ:
> > >          vi->curr_queue_pairs = queue_pairs;
> > > -       /* virtnet_open() will refill when device is going to up. */
> > > -       spin_lock_bh(&vi->refill_lock);
> > > -       if (dev->flags & IFF_UP && vi->refill_enabled)
> > > -               schedule_delayed_work(&vi->refill, 0);
> > > -       spin_unlock_bh(&vi->refill_lock);
> > > +       if (dev->flags & IFF_UP) {
> > > +               /* Let the NAPI poll refill the receive buffer for us. We can't
> > > +                * safely call try_fill_recv() here because the NAPI might be
> > > +                * enabled already.
> > > +                */
> > > +               local_bh_disable();
> > > +               for (i = 0; i < vi->curr_queue_pairs; i++)
> > > +                       virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
> > > +
> > > +               local_bh_enable();
> > > +       }
> > > 
> > >          return 0;
> > >   }
> > > --
> > > 2.43.0
> > > 
> > Thanks
> > 
> 
> Thanks,
> Quang Minh.

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Bui Quang Minh]

On 1/4/26 21:03, Michael S. Tsirkin wrote:
> On Sun, Jan 04, 2026 at 03:34:52PM +0700, Bui Quang Minh wrote:
>> On 1/4/26 13:09, Jason Wang wrote:
>>> On Fri, Jan 2, 2026 at 11:20 PM Bui Quang Minh <minhquangbui99@gmail.com> wrote:
>>>> When we fail to refill the receive buffers, we schedule a delayed worker
>>>> to retry later. However, this worker creates some concurrency issues
>>>> such as races and deadlocks. To simplify the logic and avoid further
>>>> problems, we will instead retry refilling in the next NAPI poll.
>>>>
>>>> Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
>>>> Reported-by: Paolo Abeni <pabeni@redhat.com>
>>>> Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
>>>> Cc: stable@vger.kernel.org
>>>> Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>>> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
>>>> ---
>>>>    drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
>>>>    1 file changed, 30 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>>> index 1bb3aeca66c6..ac514c9383ae 100644
>>>> --- a/drivers/net/virtio_net.c
>>>> +++ b/drivers/net/virtio_net.c
>>>> @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
>>>>    }
>>>>
>>>>    static int virtnet_receive(struct receive_queue *rq, int budget,
>>>> -                          unsigned int *xdp_xmit)
>>>> +                          unsigned int *xdp_xmit, bool *retry_refill)
>>>>    {
>>>>           struct virtnet_info *vi = rq->vq->vdev->priv;
>>>>           struct virtnet_rq_stats stats = {};
>>>> @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
>>>>                   packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
>>>>
>>>>           if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
>>>> -               if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
>>>> -                       spin_lock(&vi->refill_lock);
>>>> -                       if (vi->refill_enabled)
>>>> -                               schedule_delayed_work(&vi->refill, 0);
>>>> -                       spin_unlock(&vi->refill_lock);
>>>> -               }
>>>> +               if (!try_fill_recv(vi, rq, GFP_ATOMIC))
>>>> +                       *retry_refill = true;
>>>>           }
>>>>
>>>>           u64_stats_set(&stats.packets, packets);
>>>> @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>>>>           struct send_queue *sq;
>>>>           unsigned int received;
>>>>           unsigned int xdp_xmit = 0;
>>>> -       bool napi_complete;
>>>> +       bool napi_complete, retry_refill = false;
>>>>
>>>>           virtnet_poll_cleantx(rq, budget);
>>>>
>>>> -       received = virtnet_receive(rq, budget, &xdp_xmit);
>>>> +       received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
>>> I think we can simply let virtnet_receive() to return the budget when
>>> reill fails.
>> That makes sense, I'll change it.
>>
>>>>           rq->packets_in_napi += received;
>>>>
>>>>           if (xdp_xmit & VIRTIO_XDP_REDIR)
>>>>                   xdp_do_flush();
>>>>
>>>>           /* Out of packets? */
>>>> -       if (received < budget) {
>>>> +       if (received < budget && !retry_refill) {
>>>>                   napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
>>>>                   /* Intentionally not taking dim_lock here. This may result in a
>>>>                    * spurious net_dim call. But if that happens virtnet_rx_dim_work
>>>> @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
>>>>                   virtnet_xdp_put_sq(vi, sq);
>>>>           }
>>>>
>>>> -       return received;
>>>> +       return retry_refill ? budget : received;
>>>>    }
>>>>
>>>>    static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
>>>> @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
>>>>
>>>>           for (i = 0; i < vi->max_queue_pairs; i++) {
>>>>                   if (i < vi->curr_queue_pairs)
>>>> -                       /* Make sure we have some buffers: if oom use wq. */
>>>> -                       if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
>>>> -                               schedule_delayed_work(&vi->refill, 0);
>>>> +                       /* If this fails, we will retry later in
>>>> +                        * NAPI poll, which is scheduled in the below
>>>> +                        * virtnet_enable_queue_pair
>>>> +                        */
>>>> +                       try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
>>> Consider NAPI will be eventually scheduled, I wonder if it's still
>>> worth to refill here.
>> With GFP_KERNEL here, I think it's more likely to succeed than GFP_ATOMIC in
>> NAPI poll. Another small benefit is that the actual packet can happen
>> earlier. In case the receive buffer is empty and we don't refill here, the
>> #1 NAPI poll refill the buffer and the #2 NAPI poll can receive packets. The
>> #2 NAPI poll is scheduled in the interrupt handler because the #1 NAPI poll
>> will deschedule the NAPI and enable the device interrupt. In case we
>> successfully refill here, the #1 NAPI poll can receive packets right away.
>>
> Right. But I think this is a part that needs elucidating, not
> error handling.
>
> /* Pre-fill rq agressively, to make sure we are ready to get packets
>   * immediately.
>   * */
>
>>>>                   err = virtnet_enable_queue_pair(vi, i);
>>>>                   if (err < 0)
>>>> @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
>>>>                                   bool refill)
>>>>    {
>>>>           bool running = netif_running(vi->dev);
>>>> -       bool schedule_refill = false;
>>>>
>>>> -       if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
>>>> -               schedule_refill = true;
>>>> +       if (refill)
>>>> +               /* If this fails, we will retry later in NAPI poll, which is
>>>> +                * scheduled in the below virtnet_napi_enable
>>>> +                */
>>>> +               try_fill_recv(vi, rq, GFP_KERNEL);
>>> and here.
>>>
>>>> +
>>>>           if (running)
>>>>                   virtnet_napi_enable(rq);
> here the part that isn't clear is why are we refilling if !running
> and what handles failures in that case.

You are right, we should not refill when !running. I'll move the if 
(refill) inside the if (running).

>
>>>> -
>>>> -       if (schedule_refill)
>>>> -               schedule_delayed_work(&vi->refill, 0);
>>>>    }
>>>>
>>>>    static void virtnet_rx_resume_all(struct virtnet_info *vi)
>>>> @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>>>>           struct virtio_net_rss_config_trailer old_rss_trailer;
>>>>           struct net_device *dev = vi->dev;
>>>>           struct scatterlist sg;
>>>> +       int i;
>>>>
>>>>           if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
>>>>                   return 0;
>>>> @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
>>>>           }
>>>>    succ:
>>>>           vi->curr_queue_pairs = queue_pairs;
>>>> -       /* virtnet_open() will refill when device is going to up. */
>>>> -       spin_lock_bh(&vi->refill_lock);
>>>> -       if (dev->flags & IFF_UP && vi->refill_enabled)
>>>> -               schedule_delayed_work(&vi->refill, 0);
>>>> -       spin_unlock_bh(&vi->refill_lock);
>>>> +       if (dev->flags & IFF_UP) {
>>>> +               /* Let the NAPI poll refill the receive buffer for us. We can't
>>>> +                * safely call try_fill_recv() here because the NAPI might be
>>>> +                * enabled already.
>>>> +                */
>>>> +               local_bh_disable();
>>>> +               for (i = 0; i < vi->curr_queue_pairs; i++)
>>>> +                       virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
>>>> +
>>>> +               local_bh_enable();
>>>> +       }
>>>>
>>>>           return 0;
>>>>    }
>>>> --
>>>> 2.43.0
>>>>
>>> Thanks
>>>
>> Thanks,
>> Quang Minh.

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Michael S. Tsirkin]

On Sun, Jan 04, 2026 at 09:54:30PM +0700, Bui Quang Minh wrote:
> On 1/4/26 21:03, Michael S. Tsirkin wrote:
> > On Sun, Jan 04, 2026 at 03:34:52PM +0700, Bui Quang Minh wrote:
> > > On 1/4/26 13:09, Jason Wang wrote:
> > > > On Fri, Jan 2, 2026 at 11:20 PM Bui Quang Minh <minhquangbui99@gmail.com> wrote:
> > > > > When we fail to refill the receive buffers, we schedule a delayed worker
> > > > > to retry later. However, this worker creates some concurrency issues
> > > > > such as races and deadlocks. To simplify the logic and avoid further
> > > > > problems, we will instead retry refilling in the next NAPI poll.
> > > > > 
> > > > > Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
> > > > > Reported-by: Paolo Abeni <pabeni@redhat.com>
> > > > > Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
> > > > > Cc: stable@vger.kernel.org
> > > > > Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> > > > > ---
> > > > >    drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
> > > > >    1 file changed, 30 insertions(+), 25 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > index 1bb3aeca66c6..ac514c9383ae 100644
> > > > > --- a/drivers/net/virtio_net.c
> > > > > +++ b/drivers/net/virtio_net.c
> > > > > @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
> > > > >    }
> > > > > 
> > > > >    static int virtnet_receive(struct receive_queue *rq, int budget,
> > > > > -                          unsigned int *xdp_xmit)
> > > > > +                          unsigned int *xdp_xmit, bool *retry_refill)
> > > > >    {
> > > > >           struct virtnet_info *vi = rq->vq->vdev->priv;
> > > > >           struct virtnet_rq_stats stats = {};
> > > > > @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
> > > > >                   packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
> > > > > 
> > > > >           if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
> > > > > -               if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
> > > > > -                       spin_lock(&vi->refill_lock);
> > > > > -                       if (vi->refill_enabled)
> > > > > -                               schedule_delayed_work(&vi->refill, 0);
> > > > > -                       spin_unlock(&vi->refill_lock);
> > > > > -               }
> > > > > +               if (!try_fill_recv(vi, rq, GFP_ATOMIC))
> > > > > +                       *retry_refill = true;
> > > > >           }
> > > > > 
> > > > >           u64_stats_set(&stats.packets, packets);
> > > > > @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> > > > >           struct send_queue *sq;
> > > > >           unsigned int received;
> > > > >           unsigned int xdp_xmit = 0;
> > > > > -       bool napi_complete;
> > > > > +       bool napi_complete, retry_refill = false;
> > > > > 
> > > > >           virtnet_poll_cleantx(rq, budget);
> > > > > 
> > > > > -       received = virtnet_receive(rq, budget, &xdp_xmit);
> > > > > +       received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
> > > > I think we can simply let virtnet_receive() to return the budget when
> > > > reill fails.
> > > That makes sense, I'll change it.
> > > 
> > > > >           rq->packets_in_napi += received;
> > > > > 
> > > > >           if (xdp_xmit & VIRTIO_XDP_REDIR)
> > > > >                   xdp_do_flush();
> > > > > 
> > > > >           /* Out of packets? */
> > > > > -       if (received < budget) {
> > > > > +       if (received < budget && !retry_refill) {
> > > > >                   napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
> > > > >                   /* Intentionally not taking dim_lock here. This may result in a
> > > > >                    * spurious net_dim call. But if that happens virtnet_rx_dim_work
> > > > > @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> > > > >                   virtnet_xdp_put_sq(vi, sq);
> > > > >           }
> > > > > 
> > > > > -       return received;
> > > > > +       return retry_refill ? budget : received;
> > > > >    }
> > > > > 
> > > > >    static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
> > > > > @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
> > > > > 
> > > > >           for (i = 0; i < vi->max_queue_pairs; i++) {
> > > > >                   if (i < vi->curr_queue_pairs)
> > > > > -                       /* Make sure we have some buffers: if oom use wq. */
> > > > > -                       if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> > > > > -                               schedule_delayed_work(&vi->refill, 0);
> > > > > +                       /* If this fails, we will retry later in
> > > > > +                        * NAPI poll, which is scheduled in the below
> > > > > +                        * virtnet_enable_queue_pair
> > > > > +                        */
> > > > > +                       try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
> > > > Consider NAPI will be eventually scheduled, I wonder if it's still
> > > > worth to refill here.
> > > With GFP_KERNEL here, I think it's more likely to succeed than GFP_ATOMIC in
> > > NAPI poll. Another small benefit is that the actual packet can happen
> > > earlier. In case the receive buffer is empty and we don't refill here, the
> > > #1 NAPI poll refill the buffer and the #2 NAPI poll can receive packets. The
> > > #2 NAPI poll is scheduled in the interrupt handler because the #1 NAPI poll
> > > will deschedule the NAPI and enable the device interrupt. In case we
> > > successfully refill here, the #1 NAPI poll can receive packets right away.
> > > 
> > Right. But I think this is a part that needs elucidating, not
> > error handling.
> > 
> > /* Pre-fill rq agressively, to make sure we are ready to get packets
> >   * immediately.
> >   * */
> > 
> > > > >                   err = virtnet_enable_queue_pair(vi, i);
> > > > >                   if (err < 0)
> > > > > @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
> > > > >                                   bool refill)
> > > > >    {
> > > > >           bool running = netif_running(vi->dev);
> > > > > -       bool schedule_refill = false;
> > > > > 
> > > > > -       if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
> > > > > -               schedule_refill = true;
> > > > > +       if (refill)
> > > > > +               /* If this fails, we will retry later in NAPI poll, which is
> > > > > +                * scheduled in the below virtnet_napi_enable
> > > > > +                */
> > > > > +               try_fill_recv(vi, rq, GFP_KERNEL);
> > > > and here.
> > > > 
> > > > > +
> > > > >           if (running)
> > > > >                   virtnet_napi_enable(rq);
> > here the part that isn't clear is why are we refilling if !running
> > and what handles failures in that case.
> 
> You are right, we should not refill when !running. I'll move the if (refill)
> inside the if (running).

Sounds like a helper that does refill+virtnet_napi_enable
would be in order then?  fill_recv_aggressively(vi, rq) ?

> > 
> > > > > -
> > > > > -       if (schedule_refill)
> > > > > -               schedule_delayed_work(&vi->refill, 0);
> > > > >    }
> > > > > 
> > > > >    static void virtnet_rx_resume_all(struct virtnet_info *vi)
> > > > > @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> > > > >           struct virtio_net_rss_config_trailer old_rss_trailer;
> > > > >           struct net_device *dev = vi->dev;
> > > > >           struct scatterlist sg;
> > > > > +       int i;
> > > > > 
> > > > >           if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
> > > > >                   return 0;
> > > > > @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> > > > >           }
> > > > >    succ:
> > > > >           vi->curr_queue_pairs = queue_pairs;
> > > > > -       /* virtnet_open() will refill when device is going to up. */
> > > > > -       spin_lock_bh(&vi->refill_lock);
> > > > > -       if (dev->flags & IFF_UP && vi->refill_enabled)
> > > > > -               schedule_delayed_work(&vi->refill, 0);
> > > > > -       spin_unlock_bh(&vi->refill_lock);
> > > > > +       if (dev->flags & IFF_UP) {
> > > > > +               /* Let the NAPI poll refill the receive buffer for us. We can't
> > > > > +                * safely call try_fill_recv() here because the NAPI might be
> > > > > +                * enabled already.
> > > > > +                */
> > > > > +               local_bh_disable();
> > > > > +               for (i = 0; i < vi->curr_queue_pairs; i++)
> > > > > +                       virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
> > > > > +
> > > > > +               local_bh_enable();
> > > > > +       }
> > > > > 
> > > > >           return 0;
> > > > >    }
> > > > > --
> > > > > 2.43.0
> > > > > 
> > > > Thanks
> > > > 
> > > Thanks,
> > > Quang Minh.

Re: [PATCH net v2 1/3] virtio-net: don't schedule delayed refill worker

[Michael S. Tsirkin]

On Sun, Jan 04, 2026 at 02:09:06PM +0800, Jason Wang wrote:
> On Fri, Jan 2, 2026 at 11:20 PM Bui Quang Minh <minhquangbui99@gmail.com> wrote:
> >
> > When we fail to refill the receive buffers, we schedule a delayed worker
> > to retry later. However, this worker creates some concurrency issues
> > such as races and deadlocks. To simplify the logic and avoid further
> > problems, we will instead retry refilling in the next NAPI poll.
> >
> > Fixes: 4bc12818b363 ("virtio-net: disable delayed refill when pausing rx")
> > Reported-by: Paolo Abeni <pabeni@redhat.com>
> > Closes: https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/400961/3-xdp-py/stderr
> > Cc: stable@vger.kernel.org
> > Suggested-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> > ---
> >  drivers/net/virtio_net.c | 55 ++++++++++++++++++++++------------------
> >  1 file changed, 30 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 1bb3aeca66c6..ac514c9383ae 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -3035,7 +3035,7 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
> >  }
> >
> >  static int virtnet_receive(struct receive_queue *rq, int budget,
> > -                          unsigned int *xdp_xmit)
> > +                          unsigned int *xdp_xmit, bool *retry_refill)
> >  {
> >         struct virtnet_info *vi = rq->vq->vdev->priv;
> >         struct virtnet_rq_stats stats = {};
> > @@ -3047,12 +3047,8 @@ static int virtnet_receive(struct receive_queue *rq, int budget,
> >                 packets = virtnet_receive_packets(vi, rq, budget, xdp_xmit, &stats);
> >
> >         if (rq->vq->num_free > min((unsigned int)budget, virtqueue_get_vring_size(rq->vq)) / 2) {
> > -               if (!try_fill_recv(vi, rq, GFP_ATOMIC)) {
> > -                       spin_lock(&vi->refill_lock);
> > -                       if (vi->refill_enabled)
> > -                               schedule_delayed_work(&vi->refill, 0);
> > -                       spin_unlock(&vi->refill_lock);
> > -               }
> > +               if (!try_fill_recv(vi, rq, GFP_ATOMIC))
> > +                       *retry_refill = true;
> >         }
> >
> >         u64_stats_set(&stats.packets, packets);
> > @@ -3129,18 +3125,18 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> >         struct send_queue *sq;
> >         unsigned int received;
> >         unsigned int xdp_xmit = 0;
> > -       bool napi_complete;
> > +       bool napi_complete, retry_refill = false;
> >
> >         virtnet_poll_cleantx(rq, budget);
> >
> > -       received = virtnet_receive(rq, budget, &xdp_xmit);
> > +       received = virtnet_receive(rq, budget, &xdp_xmit, &retry_refill);
> 
> I think we can simply let virtnet_receive() to return the budget when
> reill fails.

Great idea.

> >         rq->packets_in_napi += received;
> >
> >         if (xdp_xmit & VIRTIO_XDP_REDIR)
> >                 xdp_do_flush();
> >
> >         /* Out of packets? */
> > -       if (received < budget) {
> > +       if (received < budget && !retry_refill) {
> >                 napi_complete = virtqueue_napi_complete(napi, rq->vq, received);
> >                 /* Intentionally not taking dim_lock here. This may result in a
> >                  * spurious net_dim call. But if that happens virtnet_rx_dim_work
> > @@ -3160,7 +3156,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> >                 virtnet_xdp_put_sq(vi, sq);
> >         }
> >
> > -       return received;
> > +       return retry_refill ? budget : received;
> >  }
> >
> >  static void virtnet_disable_queue_pair(struct virtnet_info *vi, int qp_index)
> > @@ -3230,9 +3226,11 @@ static int virtnet_open(struct net_device *dev)
> >
> >         for (i = 0; i < vi->max_queue_pairs; i++) {
> >                 if (i < vi->curr_queue_pairs)
> > -                       /* Make sure we have some buffers: if oom use wq. */
> > -                       if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> > -                               schedule_delayed_work(&vi->refill, 0);
> > +                       /* If this fails, we will retry later in
> > +                        * NAPI poll, which is scheduled in the below
> > +                        * virtnet_enable_queue_pair
> > +                        */
> > +                       try_fill_recv(vi, &vi->rq[i], GFP_KERNEL);
> 
> Consider NAPI will be eventually scheduled, I wonder if it's still
> worth to refill here.
> 
> >
> >                 err = virtnet_enable_queue_pair(vi, i);
> >                 if (err < 0)
> > @@ -3473,15 +3471,15 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
> >                                 bool refill)
> >  {
> >         bool running = netif_running(vi->dev);
> > -       bool schedule_refill = false;
> >
> > -       if (refill && !try_fill_recv(vi, rq, GFP_KERNEL))
> > -               schedule_refill = true;
> > +       if (refill)
> > +               /* If this fails, we will retry later in NAPI poll, which is
> > +                * scheduled in the below virtnet_napi_enable
> > +                */
> > +               try_fill_recv(vi, rq, GFP_KERNEL);
> 
> and here.
> 
> > +
> >         if (running)
> >                 virtnet_napi_enable(rq);
> > -
> > -       if (schedule_refill)
> > -               schedule_delayed_work(&vi->refill, 0);
> >  }
> >
> >  static void virtnet_rx_resume_all(struct virtnet_info *vi)
> > @@ -3777,6 +3775,7 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> >         struct virtio_net_rss_config_trailer old_rss_trailer;
> >         struct net_device *dev = vi->dev;
> >         struct scatterlist sg;
> > +       int i;
> >
> >         if (!vi->has_cvq || !virtio_has_feature(vi->vdev, VIRTIO_NET_F_MQ))
> >                 return 0;
> > @@ -3829,11 +3828,17 @@ static int virtnet_set_queues(struct virtnet_info *vi, u16 queue_pairs)
> >         }
> >  succ:
> >         vi->curr_queue_pairs = queue_pairs;
> > -       /* virtnet_open() will refill when device is going to up. */
> > -       spin_lock_bh(&vi->refill_lock);
> > -       if (dev->flags & IFF_UP && vi->refill_enabled)
> > -               schedule_delayed_work(&vi->refill, 0);
> > -       spin_unlock_bh(&vi->refill_lock);
> > +       if (dev->flags & IFF_UP) {
> > +               /* Let the NAPI poll refill the receive buffer for us. We can't
> > +                * safely call try_fill_recv() here because the NAPI might be
> > +                * enabled already.
> > +                */
> > +               local_bh_disable();
> > +               for (i = 0; i < vi->curr_queue_pairs; i++)
> > +                       virtqueue_napi_schedule(&vi->rq[i].napi, vi->rq[i].vq);
> > +
> > +               local_bh_enable();
> > +       }
> >
> >         return 0;
> >  }
> > --
> > 2.43.0
> >
> 
> Thanks

[PATCH net v2 2/3] virtio-net: remove unused delayed refill worker

[Bui Quang Minh]

Since we change to retry refilling receive buffer in NAPI poll instead
of delayed worker, remove all unused delayed refill worker code.

Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
---
 drivers/net/virtio_net.c | 86 ----------------------------------------
 1 file changed, 86 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ac514c9383ae..7e77a05b5662 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -441,9 +441,6 @@ struct virtnet_info {
 	/* Packet virtio header size */
 	u8 hdr_len;
 
-	/* Work struct for delayed refilling if we run low on memory. */
-	struct delayed_work refill;
-
 	/* UDP tunnel support */
 	bool tx_tnl;
 
@@ -451,12 +448,6 @@ struct virtnet_info {
 
 	bool rx_tnl_csum;
 
-	/* Is delayed refill enabled? */
-	bool refill_enabled;
-
-	/* The lock to synchronize the access to refill_enabled */
-	spinlock_t refill_lock;
-
 	/* Work struct for config space updates */
 	struct work_struct config_work;
 
@@ -720,20 +711,6 @@ static void virtnet_rq_free_buf(struct virtnet_info *vi,
 		put_page(virt_to_head_page(buf));
 }
 
-static void enable_delayed_refill(struct virtnet_info *vi)
-{
-	spin_lock_bh(&vi->refill_lock);
-	vi->refill_enabled = true;
-	spin_unlock_bh(&vi->refill_lock);
-}
-
-static void disable_delayed_refill(struct virtnet_info *vi)
-{
-	spin_lock_bh(&vi->refill_lock);
-	vi->refill_enabled = false;
-	spin_unlock_bh(&vi->refill_lock);
-}
-
 static void enable_rx_mode_work(struct virtnet_info *vi)
 {
 	rtnl_lock();
@@ -2948,42 +2925,6 @@ static void virtnet_napi_disable(struct receive_queue *rq)
 	napi_disable(napi);
 }
 
-static void refill_work(struct work_struct *work)
-{
-	struct virtnet_info *vi =
-		container_of(work, struct virtnet_info, refill.work);
-	bool still_empty;
-	int i;
-
-	for (i = 0; i < vi->curr_queue_pairs; i++) {
-		struct receive_queue *rq = &vi->rq[i];
-
-		/*
-		 * When queue API support is added in the future and the call
-		 * below becomes napi_disable_locked, this driver will need to
-		 * be refactored.
-		 *
-		 * One possible solution would be to:
-		 *   - cancel refill_work with cancel_delayed_work (note:
-		 *     non-sync)
-		 *   - cancel refill_work with cancel_delayed_work_sync in
-		 *     virtnet_remove after the netdev is unregistered
-		 *   - wrap all of the work in a lock (perhaps the netdev
-		 *     instance lock)
-		 *   - check netif_running() and return early to avoid a race
-		 */
-		napi_disable(&rq->napi);
-		still_empty = !try_fill_recv(vi, rq, GFP_KERNEL);
-		virtnet_napi_do_enable(rq->vq, &rq->napi);
-
-		/* In theory, this can happen: if we don't get any buffers in
-		 * we will *never* try to fill again.
-		 */
-		if (still_empty)
-			schedule_delayed_work(&vi->refill, HZ/2);
-	}
-}
-
 static int virtnet_receive_xsk_bufs(struct virtnet_info *vi,
 				    struct receive_queue *rq,
 				    int budget,
@@ -3222,8 +3163,6 @@ static int virtnet_open(struct net_device *dev)
 	struct virtnet_info *vi = netdev_priv(dev);
 	int i, err;
 
-	enable_delayed_refill(vi);
-
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		if (i < vi->curr_queue_pairs)
 			/* If this fails, we will retry later in
@@ -3249,9 +3188,6 @@ static int virtnet_open(struct net_device *dev)
 	return 0;
 
 err_enable_qp:
-	disable_delayed_refill(vi);
-	cancel_delayed_work_sync(&vi->refill);
-
 	for (i--; i >= 0; i--) {
 		virtnet_disable_queue_pair(vi, i);
 		virtnet_cancel_dim(vi, &vi->rq[i].dim);
@@ -3445,24 +3381,12 @@ static void virtnet_rx_pause_all(struct virtnet_info *vi)
 {
 	int i;
 
-	/*
-	 * Make sure refill_work does not run concurrently to
-	 * avoid napi_disable race which leads to deadlock.
-	 */
-	disable_delayed_refill(vi);
-	cancel_delayed_work_sync(&vi->refill);
 	for (i = 0; i < vi->max_queue_pairs; i++)
 		__virtnet_rx_pause(vi, &vi->rq[i]);
 }
 
 static void virtnet_rx_pause(struct virtnet_info *vi, struct receive_queue *rq)
 {
-	/*
-	 * Make sure refill_work does not run concurrently to
-	 * avoid napi_disable race which leads to deadlock.
-	 */
-	disable_delayed_refill(vi);
-	cancel_delayed_work_sync(&vi->refill);
 	__virtnet_rx_pause(vi, rq);
 }
 
@@ -3486,7 +3410,6 @@ static void virtnet_rx_resume_all(struct virtnet_info *vi)
 {
 	int i;
 
-	enable_delayed_refill(vi);
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		if (i < vi->curr_queue_pairs)
 			__virtnet_rx_resume(vi, &vi->rq[i], true);
@@ -3497,7 +3420,6 @@ static void virtnet_rx_resume_all(struct virtnet_info *vi)
 
 static void virtnet_rx_resume(struct virtnet_info *vi, struct receive_queue *rq)
 {
-	enable_delayed_refill(vi);
 	__virtnet_rx_resume(vi, rq, true);
 }
 
@@ -3848,10 +3770,6 @@ static int virtnet_close(struct net_device *dev)
 	struct virtnet_info *vi = netdev_priv(dev);
 	int i;
 
-	/* Make sure NAPI doesn't schedule refill work */
-	disable_delayed_refill(vi);
-	/* Make sure refill_work doesn't re-enable napi! */
-	cancel_delayed_work_sync(&vi->refill);
 	/* Prevent the config change callback from changing carrier
 	 * after close
 	 */
@@ -5807,7 +5725,6 @@ static int virtnet_restore_up(struct virtio_device *vdev)
 
 	virtio_device_ready(vdev);
 
-	enable_delayed_refill(vi);
 	enable_rx_mode_work(vi);
 
 	if (netif_running(vi->dev)) {
@@ -6564,7 +6481,6 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
 	if (!vi->rq)
 		goto err_rq;
 
-	INIT_DELAYED_WORK(&vi->refill, refill_work);
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		vi->rq[i].pages = NULL;
 		netif_napi_add_config(vi->dev, &vi->rq[i].napi, virtnet_poll,
@@ -6906,7 +6822,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 	INIT_WORK(&vi->config_work, virtnet_config_changed_work);
 	INIT_WORK(&vi->rx_mode_work, virtnet_rx_mode_work);
-	spin_lock_init(&vi->refill_lock);
 
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) {
 		vi->mergeable_rx_bufs = true;
@@ -7170,7 +7085,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 	net_failover_destroy(vi->failover);
 free_vqs:
 	virtio_reset_device(vdev);
-	cancel_delayed_work_sync(&vi->refill);
 	free_receive_page_frags(vi);
 	virtnet_del_vqs(vi);
 free:
-- 
2.43.0

Re: [PATCH net v2 2/3] virtio-net: remove unused delayed refill worker

[Michael S. Tsirkin]

On Fri, Jan 02, 2026 at 10:20:22PM +0700, Bui Quang Minh wrote:
> Since we change to retry refilling receive buffer in NAPI poll instead

change->switched

> of delayed worker, remove all unused delayed refill worker code.

unused -> now unused

> 
> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> ---
>  drivers/net/virtio_net.c | 86 ----------------------------------------
>  1 file changed, 86 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index ac514c9383ae..7e77a05b5662 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -441,9 +441,6 @@ struct virtnet_info {
>  	/* Packet virtio header size */
>  	u8 hdr_len;
>  
> -	/* Work struct for delayed refilling if we run low on memory. */
> -	struct delayed_work refill;
> -
>  	/* UDP tunnel support */
>  	bool tx_tnl;
>  
> @@ -451,12 +448,6 @@ struct virtnet_info {
>  
>  	bool rx_tnl_csum;
>  
> -	/* Is delayed refill enabled? */
> -	bool refill_enabled;
> -
> -	/* The lock to synchronize the access to refill_enabled */
> -	spinlock_t refill_lock;
> -
>  	/* Work struct for config space updates */
>  	struct work_struct config_work;
>  
> @@ -720,20 +711,6 @@ static void virtnet_rq_free_buf(struct virtnet_info *vi,
>  		put_page(virt_to_head_page(buf));
>  }
>  
> -static void enable_delayed_refill(struct virtnet_info *vi)
> -{
> -	spin_lock_bh(&vi->refill_lock);
> -	vi->refill_enabled = true;
> -	spin_unlock_bh(&vi->refill_lock);
> -}
> -
> -static void disable_delayed_refill(struct virtnet_info *vi)
> -{
> -	spin_lock_bh(&vi->refill_lock);
> -	vi->refill_enabled = false;
> -	spin_unlock_bh(&vi->refill_lock);
> -}
> -
>  static void enable_rx_mode_work(struct virtnet_info *vi)
>  {
>  	rtnl_lock();
> @@ -2948,42 +2925,6 @@ static void virtnet_napi_disable(struct receive_queue *rq)
>  	napi_disable(napi);
>  }
>  
> -static void refill_work(struct work_struct *work)
> -{
> -	struct virtnet_info *vi =
> -		container_of(work, struct virtnet_info, refill.work);
> -	bool still_empty;
> -	int i;
> -
> -	for (i = 0; i < vi->curr_queue_pairs; i++) {
> -		struct receive_queue *rq = &vi->rq[i];
> -
> -		/*
> -		 * When queue API support is added in the future and the call
> -		 * below becomes napi_disable_locked, this driver will need to
> -		 * be refactored.
> -		 *
> -		 * One possible solution would be to:
> -		 *   - cancel refill_work with cancel_delayed_work (note:
> -		 *     non-sync)
> -		 *   - cancel refill_work with cancel_delayed_work_sync in
> -		 *     virtnet_remove after the netdev is unregistered
> -		 *   - wrap all of the work in a lock (perhaps the netdev
> -		 *     instance lock)
> -		 *   - check netif_running() and return early to avoid a race
> -		 */
> -		napi_disable(&rq->napi);
> -		still_empty = !try_fill_recv(vi, rq, GFP_KERNEL);
> -		virtnet_napi_do_enable(rq->vq, &rq->napi);
> -
> -		/* In theory, this can happen: if we don't get any buffers in
> -		 * we will *never* try to fill again.
> -		 */
> -		if (still_empty)
> -			schedule_delayed_work(&vi->refill, HZ/2);
> -	}
> -}
> -
>  static int virtnet_receive_xsk_bufs(struct virtnet_info *vi,
>  				    struct receive_queue *rq,
>  				    int budget,
> @@ -3222,8 +3163,6 @@ static int virtnet_open(struct net_device *dev)
>  	struct virtnet_info *vi = netdev_priv(dev);
>  	int i, err;
>  
> -	enable_delayed_refill(vi);
> -
>  	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		if (i < vi->curr_queue_pairs)
>  			/* If this fails, we will retry later in
> @@ -3249,9 +3188,6 @@ static int virtnet_open(struct net_device *dev)
>  	return 0;
>  
>  err_enable_qp:
> -	disable_delayed_refill(vi);
> -	cancel_delayed_work_sync(&vi->refill);
> -
>  	for (i--; i >= 0; i--) {
>  		virtnet_disable_queue_pair(vi, i);
>  		virtnet_cancel_dim(vi, &vi->rq[i].dim);
> @@ -3445,24 +3381,12 @@ static void virtnet_rx_pause_all(struct virtnet_info *vi)
>  {
>  	int i;
>  
> -	/*
> -	 * Make sure refill_work does not run concurrently to
> -	 * avoid napi_disable race which leads to deadlock.
> -	 */
> -	disable_delayed_refill(vi);
> -	cancel_delayed_work_sync(&vi->refill);
>  	for (i = 0; i < vi->max_queue_pairs; i++)
>  		__virtnet_rx_pause(vi, &vi->rq[i]);
>  }
>  
>  static void virtnet_rx_pause(struct virtnet_info *vi, struct receive_queue *rq)
>  {
> -	/*
> -	 * Make sure refill_work does not run concurrently to
> -	 * avoid napi_disable race which leads to deadlock.
> -	 */
> -	disable_delayed_refill(vi);
> -	cancel_delayed_work_sync(&vi->refill);
>  	__virtnet_rx_pause(vi, rq);
>  }
>  
> @@ -3486,7 +3410,6 @@ static void virtnet_rx_resume_all(struct virtnet_info *vi)
>  {
>  	int i;
>  
> -	enable_delayed_refill(vi);
>  	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		if (i < vi->curr_queue_pairs)
>  			__virtnet_rx_resume(vi, &vi->rq[i], true);
> @@ -3497,7 +3420,6 @@ static void virtnet_rx_resume_all(struct virtnet_info *vi)
>  
>  static void virtnet_rx_resume(struct virtnet_info *vi, struct receive_queue *rq)
>  {
> -	enable_delayed_refill(vi);
>  	__virtnet_rx_resume(vi, rq, true);
>  }
>  
> @@ -3848,10 +3770,6 @@ static int virtnet_close(struct net_device *dev)
>  	struct virtnet_info *vi = netdev_priv(dev);
>  	int i;
>  
> -	/* Make sure NAPI doesn't schedule refill work */
> -	disable_delayed_refill(vi);
> -	/* Make sure refill_work doesn't re-enable napi! */
> -	cancel_delayed_work_sync(&vi->refill);
>  	/* Prevent the config change callback from changing carrier
>  	 * after close
>  	 */
> @@ -5807,7 +5725,6 @@ static int virtnet_restore_up(struct virtio_device *vdev)
>  
>  	virtio_device_ready(vdev);
>  
> -	enable_delayed_refill(vi);
>  	enable_rx_mode_work(vi);
>  
>  	if (netif_running(vi->dev)) {
> @@ -6564,7 +6481,6 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
>  	if (!vi->rq)
>  		goto err_rq;
>  
> -	INIT_DELAYED_WORK(&vi->refill, refill_work);
>  	for (i = 0; i < vi->max_queue_pairs; i++) {
>  		vi->rq[i].pages = NULL;
>  		netif_napi_add_config(vi->dev, &vi->rq[i].napi, virtnet_poll,
> @@ -6906,7 +6822,6 @@ static int virtnet_probe(struct virtio_device *vdev)
>  
>  	INIT_WORK(&vi->config_work, virtnet_config_changed_work);
>  	INIT_WORK(&vi->rx_mode_work, virtnet_rx_mode_work);
> -	spin_lock_init(&vi->refill_lock);
>  
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) {
>  		vi->mergeable_rx_bufs = true;
> @@ -7170,7 +7085,6 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	net_failover_destroy(vi->failover);
>  free_vqs:
>  	virtio_reset_device(vdev);
> -	cancel_delayed_work_sync(&vi->refill);
>  	free_receive_page_frags(vi);
>  	virtnet_del_vqs(vi);
>  free:
> -- 
> 2.43.0

[PATCH net v2 3/3] virtio-net: clean up __virtnet_rx_pause/resume

[Bui Quang Minh]

The delayed refill worker is removed which makes virtnet_rx_pause/resume
quite the same as __virtnet_rx_pause/resume. So remove
__virtnet_rx_pause/resume and move the code to virtnet_rx_pause/resume.

Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
---
 drivers/net/virtio_net.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7e77a05b5662..95c80f55fa9a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3366,8 +3366,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
-static void __virtnet_rx_pause(struct virtnet_info *vi,
-			       struct receive_queue *rq)
+static void virtnet_rx_pause(struct virtnet_info *vi,
+			     struct receive_queue *rq)
 {
 	bool running = netif_running(vi->dev);
 
@@ -3382,17 +3382,12 @@ static void virtnet_rx_pause_all(struct virtnet_info *vi)
 	int i;
 
 	for (i = 0; i < vi->max_queue_pairs; i++)
-		__virtnet_rx_pause(vi, &vi->rq[i]);
+		virtnet_rx_pause(vi, &vi->rq[i]);
 }
 
-static void virtnet_rx_pause(struct virtnet_info *vi, struct receive_queue *rq)
-{
-	__virtnet_rx_pause(vi, rq);
-}
-
-static void __virtnet_rx_resume(struct virtnet_info *vi,
-				struct receive_queue *rq,
-				bool refill)
+static void virtnet_rx_resume(struct virtnet_info *vi,
+			      struct receive_queue *rq,
+			      bool refill)
 {
 	bool running = netif_running(vi->dev);
 
@@ -3412,17 +3407,12 @@ static void virtnet_rx_resume_all(struct virtnet_info *vi)
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
 		if (i < vi->curr_queue_pairs)
-			__virtnet_rx_resume(vi, &vi->rq[i], true);
+			virtnet_rx_resume(vi, &vi->rq[i], true);
 		else
-			__virtnet_rx_resume(vi, &vi->rq[i], false);
+			virtnet_rx_resume(vi, &vi->rq[i], false);
 	}
 }
 
-static void virtnet_rx_resume(struct virtnet_info *vi, struct receive_queue *rq)
-{
-	__virtnet_rx_resume(vi, rq, true);
-}
-
 static int virtnet_rx_resize(struct virtnet_info *vi,
 			     struct receive_queue *rq, u32 ring_num)
 {
@@ -3436,7 +3426,7 @@ static int virtnet_rx_resize(struct virtnet_info *vi,
 	if (err)
 		netdev_err(vi->dev, "resize rx fail: rx queue index: %d err: %d\n", qindex, err);
 
-	virtnet_rx_resume(vi, rq);
+	virtnet_rx_resume(vi, rq, true);
 	return err;
 }
 
@@ -5814,7 +5804,7 @@ static int virtnet_rq_bind_xsk_pool(struct virtnet_info *vi, struct receive_queu
 
 	rq->xsk_pool = pool;
 
-	virtnet_rx_resume(vi, rq);
+	virtnet_rx_resume(vi, rq, true);
 
 	if (pool)
 		return 0;
-- 
2.43.0

Re: [PATCH net v2 0/3] virtio-net: fix the deadlock when disabling rx NAPI

[Michael S. Tsirkin]

On Fri, Jan 02, 2026 at 10:20:20PM +0700, Bui Quang Minh wrote:
> Calling napi_disable() on an already disabled napi can cause the
> deadlock. In commit 4bc12818b363 ("virtio-net: disable delayed refill
> when pausing rx"), to avoid the deadlock, when pausing the RX in
> virtnet_rx_pause[_all](), we disable and cancel the delayed refill work.
> However, in the virtnet_rx_resume_all(), we enable the delayed refill
> work too early before enabling all the receive queue napis.
> 
> The deadlock can be reproduced by running
> selftests/drivers/net/hw/xsk_reconfig.py with multiqueue virtio-net
> device and inserting a cond_resched() inside the for loop in
> virtnet_rx_resume_all() to increase the success rate. Because the worker
> processing the delayed refilled work runs on the same CPU as
> virtnet_rx_resume_all(), a reschedule is needed to cause the deadlock.
> In real scenario, the contention on netdev_lock can cause the
> reschedule.
> 
> Due to the complexity of delayed refill worker, in this series, we remove
> it. When we fail to refill the receive buffer, we will retry in the next
> NAPI poll instead.
> - Patch 1: removes delayed refill worker schedule and retry refill in next
> NAPI
> - Patch 2, 3: removes and clean up unused delayed refill worker code
> 
> For testing, I've run the following tests with no issue so far
> - selftests/drivers/net/hw/xsk_reconfig.py which sets up the XDP zerocopy
> without providing any descriptors to the fill ring. As a result,
> try_fill_recv will always fail.
> - Send TCP packets from host to guest while guest is nearly OOM and some
> try_fill_recv calls fail.

Thanks, the patches look good to me.
Sent some nitpicking comments, with these addressed:

Acked-by: Michael S. Tsirkin <mst@redhat.com>



> Changes in v2:
> - Remove the delayed refill worker to simplify the logic instead of trying
> to fix it
> - Link to v1:
> https://lore.kernel.org/netdev/20251223152533.24364-1-minhquangbui99@gmail.com/
> 
> Link to the previous approach and discussion:
> https://lore.kernel.org/netdev/20251212152741.11656-1-minhquangbui99@gmail.com/
> 
> Thanks,
> Quang Minh.
> 
> Bui Quang Minh (3):
>   virtio-net: don't schedule delayed refill worker
>   virtio-net: remove unused delayed refill worker
>   virtio-net: clean up __virtnet_rx_pause/resume
> 
>  drivers/net/virtio_net.c | 171 +++++++++------------------------------
>  1 file changed, 40 insertions(+), 131 deletions(-)
> 
> -- 
> 2.43.0