RE: [PATCH] lib/x86: Optimise csum_partial of buffers that are not multiples of 8 bytes.

-----Original Message-----
From: Eric Dumazet <redacted>
Sent: Monday, December 13, 2021 10:45 AM
To: David Laight <redacted>
Cc: Noah Goldstein <redacted>; tglx@linutronix.de;
mingo@redhat.com; Borislav Petkov [off-list ref];
dave.hansen@linux.intel.com; X86 ML [off-list ref]; hpa@zytor.com;
peterz@infradead.org; Alexander Duyck [off-list ref]; open
list [off-list ref]; netdev [off-list ref]
Subject: Re: [PATCH] lib/x86: Optimise csum_partial of buffers that are not
multiples of 8 bytes.

On Mon, Dec 13, 2021 at 10:00 AM David Laight [off-list ref]
wrote:

quoted

Add in the trailing bytes first so that there is no need to worry
about the sum exceeding 64 bits.

Signed-off-by: David Laight <redacted>
---

This ought to be faster - because of all the removed 'adc $0'.
Guessing how fast x86 code will run is hard!
There are other ways of handing buffers that are shorter than 8 bytes,
but I'd rather hope they don't happen in any hot paths.

Note - I've not even compile tested it.
(But have tested an equivalent change before.)

 arch/x86/lib/csum-partial_64.c | 55
++++++++++++----------------------
 1 file changed, 19 insertions(+), 36 deletions(-)

diff --git a/arch/x86/lib/csum-partial_64.c

b/arch/x86/lib/csum-partial_64.c index abf819dd8525..fbcc073fc2b5
100644

--- a/arch/x86/lib/csum-partial_64.c
+++ b/arch/x86/lib/csum-partial_64.c

@@ -37,6 +37,24 @@ __wsum csum_partial(const void *buff, int len,

__wsum sum)

quoted

        u64 temp64 = (__force u64)sum;
        unsigned result;

+       if (len & 7) {
+               if (unlikely(len < 8)) {
+                       /* Avoid falling off the start of the buffer */
+                       if (len & 4) {
+                               temp64 += *(u32 *)buff;
+                               buff += 4;
+                       }
+                       if (len & 2) {
+                               temp64 += *(u16 *)buff;
+                               buff += 2;
+                       }
+                       if (len & 1)
+                               temp64 += *(u8 *)buff;
+                       goto reduce_to32;
+               }
+               temp64 += *(u64 *)(buff + len - 8) << (8 - (len & 7))
+ * 8;

This is reading far away (end of buffer).

Maybe instead read the first bytes and adjust @buff, to allow for better
hardware prefetching ?

That will cause the wsum to be aligned to the length instead of the buff wouldn't it? So we would need an extra rotation at the end to realign odd length sections wouldn't we?

Since our only concern here would be large buffers would it maybe make sense to just run the loop in the call in reverse if we make this change? That way we would still be accessing the same cache line once we start the loop. For smaller buffers I would imagine the overhead should be minimal since we likely would have the end of the buffer still in cache since it would be something like 40B over anyway.