mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Demian Shulhan <demyansh@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Eric Biggers <ebiggers@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Marco Elver <elver@google.com>, Ard Biesheuvel <ardb@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	David Gow <davidgow@google.com>,
	Brendan Higgins <brendan.higgins@linux.dev>,
	Nathan Chancellor <nathan@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kunit-dev@googlegroups.com,
	netdev@vger.kernel.org, llvm@lists.linux.dev
Subject: Re: [PATCH 0/2] arm64: csum: Add fused copy and Internet checksum
Date: Tue, 29 Sep 2026 18:33:06 -0700	[thread overview]
Message-ID: <20260929183306.24a26c74@kernel.org> (raw)
In-Reply-To: <20260927131838.6774-1-demyansh@gmail.com>

On Sun, 27 Sep 2026 15:17:56 +0200 Demian Shulhan wrote:
> arm64 currently uses the generic csum_partial_copy_nocheck(), which
> performs memcpy() followed by a second pass for csum_partial(). This
> double pass exerts unnecessary pressure on the L1 cache.
> 
> Replace it with a single-pass implementation. The new implementation
> provides a general-purpose register path for short buffers and atomic
> contexts, and a kernel-mode NEON path for lengths >= 1024 bytes.
> 
> Measured in-kernel on an Ampere Altra (Neoverse-N1):
> - Scalar path: 1.2x-1.6x faster for lengths < 1024 bytes.
> - NEON path: 1.2x faster at 1024 bytes, scaling up to 1.6x-1.8x at
>   4096 bytes.
> On Apple M-series cores, gains are 1.3-1.7x below 1024 bytes and
> 1.6-2.4x above. No length or alignment regresses on either
> microarchitecture.
> 
> Patch 1 implements the fused routines and the dispatcher.
> Patch 2 adds KUnit test coverage for the new API and internal paths.
> 
> Tested: in-kernel benchmark module on Neoverse-N1 with both
> implementations cross-checked (0 mismatches); KUnit suite under QEMU
> (with/without KASAN, with PREEMPT_RT), exhaustive and random userspace
> testing of both routines against a naive reference with PROT_NONE guard
> pages, gcc 13 and clang 18 W=1 builds, checkpatch --strict.
> 
NIPA CI flagged a regression from this patch: the new "checksum" KUnit
suite fails on the x86-64 test kernel (ARCH=x86_64, qemu). Specifically:

  test_csum_copy_small_all_alignments  (len=0, src_off=0, dst_off=0)
  test_csum_copy_patterns              (len=1, src_off=0, dst_off=0)
  test_csum_copy_zero_len

All three failures involve a zero-length (or very short) copy. Digging
into it, x86-64's csum_partial_copy_generic() (arch/x86/lib/csum-copy_64.S)
seeds its accumulator with -1 (0xffffffff) rather than 0:

    movl  $-1, %eax
    ...
    cmpl  $8, %ecx
    jb    .Lshort

For len == 0 (and other very short lengths that never execute an
add/adc against the seed) it returns that -1 unmodified, which folds to
0. The naive reference used by the new tests, and csum_partial()'s own
convention for an empty input, instead treat the "empty checksum" as raw
sum 0, which folds to 0xffff. So the new tests' expectations don't match
the pre-existing x86-64 assembly implementation for these edge cases.

It looks like the tests were validated against the new arm64
implementation and against memcpy()+csum_partial() under QEMU on arm64,
but not run against x86-64's existing csum_partial_copy_nocheck()
implementation, which is what our CI kunit runner builds by default.

Could you take a look at either:
  - adjusting the zero/short-length expectations in the new test to
    match the existing (documented?) x86-64 behavior, or
  - treating this as a real x86-64 bug and fixing
    csum_partial_copy_generic()'s handling of very short lengths,

whichever is judged correct? Happy to share the full kunit log if useful.

      parent reply	other threads:[~2026-09-30  1:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-27 13:17 Demian Shulhan
2026-09-27 13:17 ` [PATCH 1/2] " Demian Shulhan
2026-09-27 13:17 ` [PATCH 2/2] lib/tests: checksum: Add KUnit tests for csum_partial_copy_nocheck() Demian Shulhan
2026-09-27 17:44 ` [PATCH 0/2] arm64: csum: Add fused copy and Internet checksum David Laight
2026-09-30  1:33 ` Jakub Kicinski [this message]

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=20260929183306.24a26c74@kernel.org \
    --to=kuba@kernel.org \
    --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=kunit-dev@googlegroups.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mark.rutland@arm.com \
    --cc=nathan@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=robin.murphy@arm.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®