87 lines
		
	
	
		
			2.9 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
			
		
		
	
	
			87 lines
		
	
	
		
			2.9 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
From 03333931c17d9c62ba4063d4e4fec1578c0729a7 Mon Sep 17 00:00:00 2001
 | 
						|
From: Eric Dumazet <eric.dumazet@gmail.com>
 | 
						|
Date: Sat, 12 May 2012 21:23:23 +0000
 | 
						|
Subject: [PATCH] codel: use u16 field instead of 31bits for rec_inv_sqrt
 | 
						|
 | 
						|
commit 6ff272c9ad65eda219cd975b9da2dbc31cc812ee upstream.
 | 
						|
 | 
						|
David pointed out gcc might generate poor code with 31bit fields.
 | 
						|
 | 
						|
Using u16 is more than enough and permits a better code output.
 | 
						|
 | 
						|
Also make the code intent more readable using constants, fixed point arithmetic
 | 
						|
not being trivial for everybody.
 | 
						|
 | 
						|
Suggested-by: David Miller <davem@davemloft.net>
 | 
						|
Signed-off-by: Eric Dumazet <edumazet@google.com>
 | 
						|
Signed-off-by: David S. Miller <davem@davemloft.net>
 | 
						|
---
 | 
						|
 include/net/codel.h |   25 +++++++++++++++----------
 | 
						|
 1 file changed, 15 insertions(+), 10 deletions(-)
 | 
						|
 | 
						|
--- a/include/net/codel.h
 | 
						|
+++ b/include/net/codel.h
 | 
						|
@@ -133,13 +133,17 @@ struct codel_params {
 | 
						|
 struct codel_vars {
 | 
						|
 	u32		count;
 | 
						|
 	u32		lastcount;
 | 
						|
-	bool		dropping:1;
 | 
						|
-	u32		rec_inv_sqrt:31;
 | 
						|
+	bool		dropping;
 | 
						|
+	u16		rec_inv_sqrt;
 | 
						|
 	codel_time_t	first_above_time;
 | 
						|
 	codel_time_t	drop_next;
 | 
						|
 	codel_time_t	ldelay;
 | 
						|
 };
 | 
						|
 
 | 
						|
+#define REC_INV_SQRT_BITS (8 * sizeof(u16)) /* or sizeof_in_bits(rec_inv_sqrt) */
 | 
						|
+/* needed shift to get a Q0.32 number from rec_inv_sqrt */
 | 
						|
+#define REC_INV_SQRT_SHIFT (32 - REC_INV_SQRT_BITS)
 | 
						|
+
 | 
						|
 /**
 | 
						|
  * struct codel_stats - contains codel shared variables and stats
 | 
						|
  * @maxpacket:	largest packet we've seen so far
 | 
						|
@@ -173,17 +177,18 @@ static void codel_stats_init(struct code
 | 
						|
  * http://en.wikipedia.org/wiki/Methods_of_computing_square_roots#Iterative_methods_for_reciprocal_square_roots
 | 
						|
  * new_invsqrt = (invsqrt / 2) * (3 - count * invsqrt^2)
 | 
						|
  *
 | 
						|
- * Here, invsqrt is a fixed point number (< 1.0), 31bit mantissa)
 | 
						|
+ * Here, invsqrt is a fixed point number (< 1.0), 32bit mantissa, aka Q0.32
 | 
						|
  */
 | 
						|
 static void codel_Newton_step(struct codel_vars *vars)
 | 
						|
 {
 | 
						|
-	u32 invsqrt = vars->rec_inv_sqrt;
 | 
						|
-	u32 invsqrt2 = ((u64)invsqrt * invsqrt) >> 31;
 | 
						|
-	u64 val = (3LL << 31) - ((u64)vars->count * invsqrt2);
 | 
						|
+	u32 invsqrt = ((u32)vars->rec_inv_sqrt) << REC_INV_SQRT_SHIFT;
 | 
						|
+	u32 invsqrt2 = ((u64)invsqrt * invsqrt) >> 32;
 | 
						|
+	u64 val = (3LL << 32) - ((u64)vars->count * invsqrt2);
 | 
						|
 
 | 
						|
-	val = (val * invsqrt) >> 32;
 | 
						|
+	val >>= 2; /* avoid overflow in following multiply */
 | 
						|
+	val = (val * invsqrt) >> (32 - 2 + 1);
 | 
						|
 
 | 
						|
-	vars->rec_inv_sqrt = val;
 | 
						|
+	vars->rec_inv_sqrt = val >> REC_INV_SQRT_SHIFT;
 | 
						|
 }
 | 
						|
 
 | 
						|
 /*
 | 
						|
@@ -195,7 +200,7 @@ static codel_time_t codel_control_law(co
 | 
						|
 				      codel_time_t interval,
 | 
						|
 				      u32 rec_inv_sqrt)
 | 
						|
 {
 | 
						|
-	return t + reciprocal_divide(interval, rec_inv_sqrt << 1);
 | 
						|
+	return t + reciprocal_divide(interval, rec_inv_sqrt << REC_INV_SQRT_SHIFT);
 | 
						|
 }
 | 
						|
 
 | 
						|
 
 | 
						|
@@ -326,7 +331,7 @@ static struct sk_buff *codel_dequeue(str
 | 
						|
 			codel_Newton_step(vars);
 | 
						|
 		} else {
 | 
						|
 			vars->count = 1;
 | 
						|
-			vars->rec_inv_sqrt = 0x7fffffff;
 | 
						|
+			vars->rec_inv_sqrt = ~0U >> REC_INV_SQRT_SHIFT;
 | 
						|
 		}
 | 
						|
 		vars->lastcount = vars->count;
 | 
						|
 		vars->drop_next = codel_control_law(now, params->interval,
 |