mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Demian Shulhan <demyansh@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Saeed Bishara <saeed.bishara.os@gmail.com>,
	Eric Biggers <ebiggers@kernel.org>,
	Ard Biesheuvel <ardb@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Marco Elver <elver@google.com>, David Gow <davidgow@google.com>,
	Brendan Higgins <brendan.higgins@linux.dev>,
	linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
	kunit-dev@googlegroups.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] arm64: csum: Add fused copy and Internet checksum
Date: Sat, 14 Nov 2026 14:26:31 +0000	[thread overview]
Message-ID: <20261114142631.7352b895@pumpkin> (raw)
In-Reply-To: <20261114040943.219870-2-demyansh@gmail.com>

On Sat, 14 Nov 2026 03:57:00 +0000
Demian Shulhan <demyansh@gmail.com> wrote:

> arm64 currently falls back to the generic csum_partial_copy_nocheck() in
> lib/checksum.c, which performs memcpy() followed by a second pass in
> csum_partial(). This double pass is hit on software GSO paths
> (skb_segment() without SG or TX checksum offload, e.g. TCP over
> WireGuard) and UDP/ICMP replies.
> 
> Implement a single-pass csum_partial_copy_nocheck() in
> arch/arm64/lib/csum.c using four independent __uint128_t accumulators
> over a 32-byte stride, followed by 16/8/4/2/1-byte tail handling. Each
> word is read once into a GPR before storing and summing, emitting paired
> ldp/stp and adds/cinc carry propagation with no stack frame.

Is that right?
I thought ldp/stp required aligned addresses.
The code below is using get_unaligned().
I think the timings below are for ldp/stp?

> 
> Throughput on ARM Neoverse-V2 (Google Axion, 3.0 GHz, GCC 15.2 -O2,
> warm L1 cache):
> 
>    len   memcpy + do_csum   csum_partial_copy_nocheck   speedup
>   ----   ----------------   -------------------------   -------
>     64         10245 MB/s                  15902 MB/s    +55.2%
>    256         18851 MB/s                  26026 MB/s    +38.1%
>    512         22465 MB/s                  28421 MB/s    +26.5%
>   1024         24330 MB/s                  29900 MB/s    +22.9%
>   1368         24192 MB/s                  30221 MB/s    +24.9%
>   1448         24188 MB/s                  30279 MB/s    +25.2%
>   1500         24165 MB/s                  30096 MB/s    +24.5%
>   4096         25299 MB/s                  29946 MB/s    +18.4%

That is topping out at 10 bytes/clock.
Each loop iteration copies 32 bytes - so must take 3 clocks.
Which means it must be doing 16 byte read/write in a single clock.
So rather begs the question of why it is limited by the loop control
and checksum instructions rather than the memory accesses.
The checksum code needs 4 ALU instructions (for 16 bytes), if the CPU
can execute more than that then there is half a chance of getting the
loop down to 2 clocks.
Just moving one (or both) the pointer updates into the middle may help.
Otherwise you try using negative offsets from the end of the buffer
and 'reg + reg' addressing (will need more pointers since I don't think
arm has 'reg + reg + offset' addressing.

> 
> Suggested-by: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: Demian Shulhan <demyansh@gmail.com>
> ---
>  arch/arm64/include/asm/checksum.h |   3 +
>  arch/arm64/lib/csum.c             | 100 ++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/checksum.h b/arch/arm64/include/asm/checksum.h
> index dc52b733675d..9f766db01152 100644
> --- a/arch/arm64/include/asm/checksum.h
> +++ b/arch/arm64/include/asm/checksum.h
> @@ -44,6 +44,9 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
>  extern unsigned int do_csum(const unsigned char *buff, int len);
>  #define do_csum do_csum
>  
> +#define _HAVE_ARCH_CSUM_AND_COPY
> +__wsum csum_partial_copy_nocheck(const void *src, void *dst, int len);
> +
>  #include <asm-generic/checksum.h>
>  
>  #endif	/* __ASM_CHECKSUM_H */
> diff --git a/arch/arm64/lib/csum.c b/arch/arm64/lib/csum.c
> index 75db718e5b6a..a9617cde28c1 100644
> --- a/arch/arm64/lib/csum.c
> +++ b/arch/arm64/lib/csum.c
> @@ -2,8 +2,10 @@
>  // Copyright (C) 2019-2020 Arm Ltd.
>  
>  #include <linux/compiler.h>
> +#include <linux/export.h>
>  #include <linux/kasan-checks.h>
>  #include <linux/kernel.h>
> +#include <linux/unaligned.h>
>  
>  #include <net/checksum.h>
>  
> @@ -116,6 +118,104 @@ unsigned int __no_sanitize_address do_csum(const unsigned char *buff, int len)
>  	return sum >> 16;
>  }
>  
> +static __always_inline __wsum csum_fold128(__uint128_t sum)
> +{
> +	u64 s;
> +
> +	sum += (sum >> 64) | (sum << 64);
> +	s = sum >> 64;
> +	s += (s >> 32) | (s << 32);
> +	return (__force __wsum)(u32)(s >> 32);
> +}
> +
> +/*
> + * Single-pass copy and ones' complement sum. Each word is read once into a
> + * GPR before storing and summing, so the returned sum always matches @dst
> + * even if @src is concurrently modified.
> + */
> +__wsum csum_partial_copy_nocheck(const void *src, void *dst, int len)
> +{
> +	const u8 *s = src;
> +	u8 *d = dst;
> +	__uint128_t sum0 = 0, sum1 = 0, sum2 = 0, sum3 = 0;
> +	unsigned int n;
> +
> +	if (unlikely(len <= 0))
> +		return 0;

Hmmm...
Does zero actually happen often enough for that to be a gain?
Even with the unlikely() it is likely to slow down the normal path.
And that assumes that the branch isn't mispredicted and that gcc
doesn't ignore the unlikely(). 

> +
> +	n = (unsigned int)len >> 5;
> +	if (n) {
> +		do {
> +			u64 v0, v1;
> +
> +			v0 = get_unaligned((const u64 *)s);
> +			v1 = get_unaligned((const u64 *)(s + 8));
> +			put_unaligned(v0, (u64 *)d);
> +			put_unaligned(v1, (u64 *)(d + 8));
> +			sum0 += v0;
> +			sum1 += v1;
> +
> +			v0 = get_unaligned((const u64 *)(s + 16));
> +			v1 = get_unaligned((const u64 *)(s + 24));
> +			put_unaligned(v0, (u64 *)(d + 16));
> +			put_unaligned(v1, (u64 *)(d + 24));
> +			sum2 += v0;
> +			sum3 += v1;

Is using sum0/1 here actually slower?
My 'gut feel' is that it won't be - and saves the adds below.

It is also worth checking that gcc doesn't zero extend v0 into a
register and then do a full 128bit add.
I'm sure I looked at the code using only 64bit variables for:
			sum0 += v0;
			carry0 += sum0 < v0;
both gcc and clang used 'add 0 with carry' for the second line.

> +
> +			s += 32;
> +			d += 32;
> +		} while (--n);

It is a shame to have to update 3 variables here.
They may be taking the 3rd clock.

David


> +
> +		sum0 += sum2;
> +		sum1 += sum3;
> +	}
> +
> +	if (len & 16) {
> +		u64 v0 = get_unaligned((const u64 *)s);
> +		u64 v1 = get_unaligned((const u64 *)(s + 8));
> +
> +		put_unaligned(v0, (u64 *)d);
> +		put_unaligned(v1, (u64 *)(d + 8));
> +		sum0 += v0;
> +		sum1 += v1;
> +		s += 16;
> +		d += 16;
> +	}
> +	if (len & 8) {
> +		u64 v = get_unaligned((const u64 *)s);
> +
> +		put_unaligned(v, (u64 *)d);
> +		sum0 += v;
> +		s += 8;
> +		d += 8;
> +	}
> +	if (len & 4) {
> +		u32 v = get_unaligned((const u32 *)s);
> +
> +		put_unaligned(v, (u32 *)d);
> +		sum1 += v;
> +		s += 4;
> +		d += 4;
> +	}
> +	if (len & 2) {
> +		u16 v = get_unaligned((const u16 *)s);
> +
> +		put_unaligned(v, (u16 *)d);
> +		sum0 += v;
> +		s += 2;
> +		d += 2;
> +	}
> +	if (len & 1) {
> +		u8 v = *s;
> +
> +		*d = v;
> +		sum1 += v;
> +	}
> +
> +	return csum_fold128(sum0 + sum1);
> +}
> +EXPORT_SYMBOL(csum_partial_copy_nocheck);
> +
>  __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
>  			const struct in6_addr *daddr,
>  			__u32 len, __u8 proto, __wsum csum)


  reply	other threads:[~2026-11-14 14:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-14  3:56 [PATCH v2 0/2] " Demian Shulhan
2026-11-14  3:57 ` [PATCH v2 1/2] " Demian Shulhan
2026-11-14 14:26   ` David Laight [this message]
2026-11-14  3:57 ` [PATCH v2 2/2] lib/tests: checksum: Add KUnit tests for csum_partial_copy_nocheck() Demian Shulhan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261114142631.7352b895@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=ardb@kernel.org \
    --cc=brendan.higgins@linux.dev \
    --cc=catalin.marinas@arm.com \
    --cc=davidgow@google.com \
    --cc=demyansh@gmail.com \
    --cc=ebiggers@kernel.org \
    --cc=elver@google.com \
    --cc=kuba@kernel.org \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=nathan@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=saeed.bishara.os@gmail.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®