I think the the requeue in refill_work is not the problem here. In
virtnet_rx_pause[_all](), we use cancel_work_sync() which is safe to
use "even if the work re-queues itself". AFAICS, cancel_work_sync()
will disable work -> flush work -> enable again. So if the work requeue
itself in flush work, the requeue will fail because the work is already
disabled.
I think what triggers the deadlock here is a bug in
virtnet_rx_resume_all(). virtnet_rx_resume_all() calls to
__virtnet_rx_resume() which calls napi_enable() and may schedule
refill. It schedules the refill work right after napi_enable the first
receive queue. The correct way must be napi_enable all receive queues
before scheduling refill work.
The fix is like this (there are quite duplicated code, I will clean up
and send patches later if it is correct)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 8e04adb57f52..892aa0805d1b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3482,20 +3482,25 @@ static void __virtnet_rx_resume(struct virtnet_info *vi,
static void virtnet_rx_resume_all(struct virtnet_info *vi)
{
int i;
+ bool schedule_refill = false;
+
+ for (i = 0; i < vi->max_queue_pairs; i++)
+ __virtnet_rx_resume(vi, &vi->rq[i], false);
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);
- else
- __virtnet_rx_resume(vi, &vi->rq[i], false);
- }
+
+ for (i = 0; i < vi->curr_queue_pairs; i++)
+ if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
+ schedule_refill = true;
+
+ if (schedule_refill)
+ schedule_delayed_work(&vi->refill, 0);
}
static void virtnet_rx_resume(struct virtnet_info *vi, struct receive_queue
*rq)
{
- enable_delayed_refill(vi);
__virtnet_rx_resume(vi, rq, true);
+ enable_delayed_refill(vi);
}
static int virtnet_rx_resize(struct virtnet_info *vi,
I also move the enable_delayed_refill() after we __virtnet_rx_resume()
to ensure no refill is scheduled before napi_enable().
What do you think?
Thanks,
Quang Minh
