Re: [patch] pktgen: bug when calling ndelay in x86 architectures

Le jeudi 20 octobre 2011 à 16:24 -0400, David Miller a écrit :

From: Eric Dumazet <redacted>
Date: Tue, 18 Oct 2011 16:47:44 +0200

quoted

Le mardi 18 octobre 2011 à 15:00 +0100, Ben Hutchings a écrit :

quoted

AIUI, the reason for limits on delays is not that it's bad practice to
spin for so long, but that the delay calculations may overflow or
otherwise become inaccurate.

OK, I can understand that, then a more appropriate patch would be :

I think doing the udelay/ndelay thing is the way to go for 'net' and
-stable.  We can do something sophisticated with ktime et al. in
'net-next'.

Well, I am not sure a patch is needed for net, since there is no bug,
but maybe small inaccuracies ? Correct me if I misunderstood Daniel !

Eric, could you please formally submit this patch with proper
changelog etc.?

Sure !

[PATCH net-next] pktgen: remove ndelay() call

Daniel Turull reported inaccuracies in pktgen when using low packet
rates, because we call ndelay(val) with values bigger than 20000.

Instead of calling ndelay() for delays < 100us, we can instead loop
calling ktime_now() only.

Reported-by: Daniel Turull <redacted>
Signed-off-by: Eric Dumazet <redacted>
---
 net/core/pktgen.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 6bbf008..0001c24 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c

@@ -2145,9 +2145,12 @@ static void spin(struct pktgen_dev *pkt_dev, ktime_t spin_until)
 	}
 
 	start_time = ktime_now();
-	if (remaining < 100000)
-		ndelay(remaining);	/* really small just spin */
-	else {
+	if (remaining < 100000) {
+		/* for small delays (<100us), just loop until limit is reached */
+		do {
+			end_time = ktime_now();
+		} while (ktime_lt(end_time, spin_until));
+	} else {
 		/* see do_nanosleep */
 		hrtimer_init_sleeper(&t, current);
 		do {

@@ -2162,8 +2165,8 @@ static void spin(struct pktgen_dev *pkt_dev, ktime_t spin_until)
 			hrtimer_cancel(&t.timer);
 		} while (t.task && pkt_dev->running && !signal_pending(current));
 		__set_current_state(TASK_RUNNING);
+		end_time = ktime_now();
 	}
-	end_time = ktime_now();
 
 	pkt_dev->idle_acc += ktime_to_ns(ktime_sub(end_time, start_time));
 	pkt_dev->next_tx = ktime_add_ns(spin_until, pkt_dev->delay);