Re: [PATCH net v3] net: sched: fix packet stuck problem for lockless qdisc

On Mon, 12 Apr 2021 09:24:30  Yunsheng Lin wrote:

quoted

On 2021/4/9 17:09, Hillf Danton wrote:

quoted

On Fri, 9 Apr 2021 07:31:03  Juergen Gross wrote:

quoted

On 25.03.21 04:13, Yunsheng Lin wrote:
I have a setup which is able to reproduce the issue quite reliably:

In a Xen guest I'm mounting 8 NFS shares and run sysbench fileio on
each of them. The average latency reported by sysbench is well below
1 msec, but at least once per hour I get latencies in the minute
range.

With this patch I don't see these high latencies any longer (test
is running for more than 20 hours now).

So you can add my:

Tested-by: Juergen Gross <jgross@suse.com>

If retry is allowed in the dequeue method then a simple seqcount can do the
work of serializing enqueuer and dequeuer. IIUC it was not attempted last year.

At the first glance, I do not think the below patch fix the data race

Thanks for taking a look.

quoted

described in the commit log, as it does not handle the time window
between dequeuing and q->seqlock releasing, as below:

Yes the time window does exist.

quoted

The cpu1 may not see the qdisc->pad changed after pfifo_fast_dequeue(),
and cpu2 is not able to take the q->seqlock yet because cpu1 do not
release the q->seqlock.

It's now covered by extending the seqcount aperture a bit.

--- x/net/sched/sch_generic.c
+++ y/net/sched/sch_generic.c

@@ -380,14 +380,23 @@ void __qdisc_run(struct Qdisc *q)
 {
 	int quota = dev_tx_weight;
 	int packets;
+	int seq;
+
+again:
+	seq = READ_ONCE(q->pad);
+	smp_rmb();
 
 	while (qdisc_restart(q, &packets)) {
 		quota -= packets;
 		if (quota <= 0) {
 			__netif_schedule(q);
-			break;
+			return;
 		}
 	}
+
+	smp_rmb();
+	if (seq != READ_ONCE(q->pad))
+		goto again;

 }
 
 unsigned long dev_trans_start(struct net_device *dev)

@@ -632,6 +641,9 @@ static int pfifo_fast_enqueue(struct sk_
 			return qdisc_drop(skb, qdisc, to_free);
 	}
 
+	qdisc->pad++;
+	smp_wmb();
+
 	qdisc_update_stats_at_enqueue(qdisc, pkt_len);
 	return NET_XMIT_SUCCESS;
 }

.