Patchwork lib/checksum.c: optimize do_csum a bit

login
register
mail settings
Submitter Ian Abbott
Date July 7, 2011, 11:18 a.m.
Message ID <1310037529-30854-1-git-send-email-abbotti@mev.co.uk>
Download mbox | patch
Permalink /patch/103642/
State Accepted
Delegated to: David Miller
Headers show

Comments

Ian Abbott - July 7, 2011, 11:18 a.m.
Reduce the number of variables modified by the loop in do_csum() by 1,
which seems like a good idea.  On Nios II (a RISC CPU with 3-operand
instruction set) it reduces the loop from 7 to 6 instructions, including
the conditional branch.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 lib/checksum.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)
David Miller - July 7, 2011, 11:29 a.m.
From: Ian Abbott <abbotti@mev.co.uk>
Date: Thu, 7 Jul 2011 12:18:49 +0100

> Reduce the number of variables modified by the loop in do_csum() by 1,
> which seems like a good idea.  On Nios II (a RISC CPU with 3-operand
> instruction set) it reduces the loop from 7 to 6 instructions, including
> the conditional branch.
> 
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>

I think you'll overshoot past the end of the buffer when there are
trailing bytes to handle.

The whole reason we need the count variable is to handle those
kinds of cases.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ian Abbott - July 7, 2011, 11:32 a.m.
On 07/07/11 12:29, David Miller wrote:
> From: Ian Abbott <abbotti@mev.co.uk>
> Date: Thu, 7 Jul 2011 12:18:49 +0100
> 
>> Reduce the number of variables modified by the loop in do_csum() by 1,
>> which seems like a good idea.  On Nios II (a RISC CPU with 3-operand
>> instruction set) it reduces the loop from 7 to 6 instructions, including
>> the conditional branch.
>>
>> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> 
> I think you'll overshoot past the end of the buffer when there are
> trailing bytes to handle.
> 
> The whole reason we need the count variable is to handle those
> kinds of cases.

I don't think it does.  That's what the & ~3 was for.
David Miller - July 7, 2011, 11:36 a.m.
From: Ian Abbott <abbotti@mev.co.uk>
Date: Thu, 7 Jul 2011 12:32:45 +0100

> On 07/07/11 12:29, David Miller wrote:
>> From: Ian Abbott <abbotti@mev.co.uk>
>> Date: Thu, 7 Jul 2011 12:18:49 +0100
>> 
>>> Reduce the number of variables modified by the loop in do_csum() by 1,
>>> which seems like a good idea.  On Nios II (a RISC CPU with 3-operand
>>> instruction set) it reduces the loop from 7 to 6 instructions, including
>>> the conditional branch.
>>>
>>> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
>> 
>> I think you'll overshoot past the end of the buffer when there are
>> trailing bytes to handle.
>> 
>> The whole reason we need the count variable is to handle those
>> kinds of cases.
> 
> I don't think it does.  That's what the & ~3 was for.

Aha, yes that indeed makes it work.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - July 7, 2011, 11:52 a.m.
From: David Miller <davem@davemloft.net>
Date: Thu, 07 Jul 2011 04:36:25 -0700 (PDT)

> From: Ian Abbott <abbotti@mev.co.uk>
> Date: Thu, 7 Jul 2011 12:32:45 +0100
> 
>> On 07/07/11 12:29, David Miller wrote:
>>> From: Ian Abbott <abbotti@mev.co.uk>
>>> Date: Thu, 7 Jul 2011 12:18:49 +0100
>>> 
>>>> Reduce the number of variables modified by the loop in do_csum() by 1,
>>>> which seems like a good idea.  On Nios II (a RISC CPU with 3-operand
>>>> instruction set) it reduces the loop from 7 to 6 instructions, including
>>>> the conditional branch.
>>>>
>>>> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
>>> 
>>> I think you'll overshoot past the end of the buffer when there are
>>> trailing bytes to handle.
>>> 
>>> The whole reason we need the count variable is to handle those
>>> kinds of cases.
>> 
>> I don't think it does.  That's what the & ~3 was for.
> 
> Aha, yes that indeed makes it work.

I've applied this to net-next-2.6, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/lib/checksum.c b/lib/checksum.c
index 0975087..8df2f91 100644
--- a/lib/checksum.c
+++ b/lib/checksum.c
@@ -49,7 +49,7 @@  static inline unsigned short from32to16(unsigned int x)
 
 static unsigned int do_csum(const unsigned char *buff, int len)
 {
-	int odd, count;
+	int odd;
 	unsigned int result = 0;
 
 	if (len <= 0)
@@ -64,25 +64,22 @@  static unsigned int do_csum(const unsigned char *buff, int len)
 		len--;
 		buff++;
 	}
-	count = len >> 1;		/* nr of 16-bit words.. */
-	if (count) {
+	if (len >= 2) {
 		if (2 & (unsigned long) buff) {
 			result += *(unsigned short *) buff;
-			count--;
 			len -= 2;
 			buff += 2;
 		}
-		count >>= 1;		/* nr of 32-bit words.. */
-		if (count) {
+		if (len >= 4) {
+			const unsigned char *end = buff + ((unsigned)len & ~3);
 			unsigned int carry = 0;
 			do {
 				unsigned int w = *(unsigned int *) buff;
-				count--;
 				buff += 4;
 				result += carry;
 				result += w;
 				carry = (w > result);
-			} while (count);
+			} while (buff < end);
 			result += carry;
 			result = (result & 0xffff) + (result >> 16);
 		}