Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230628020657.957880-1-goldstein.w.n@gmail.com> <20230628091241.GAZJv5ie0xVGvnMKIM@fat_crate.local>
In-Reply-To: <20230628091241.GAZJv5ie0xVGvnMKIM@fat_crate.local>
From:   Noah Goldstein <goldstein.w.n@gmail.com>
Date:   Wed, 28 Jun 2023 10:32:12 -0500
Message-ID: <CAFUsyfKYQk87fxCDj9FpM9xzLwZVRrfUZgyUCF5G+HuC0N2pXg@mail.gmail.com>
Subject: Re: x86/csum: Remove unnecessary odd handling
To:     Borislav Petkov <bp@alien8.de>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>, x86@kernel.org,
        edumazet@google.com, tglx@linutronix.de, mingo@redhat.com,
        dave.hansen@linux.intel.com, hpa@zytor.com,
        lkml <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Jun 28, 2023 at 4:12=E2=80=AFAM Borislav Petkov <bp@alien8.de> wrot=
e:
>
> + Linus who's been poking at this yesterday.
>
Linus, if you're planning a patch and want to just integrate the codes
here I'm happy drop this patch

> + lkml. Please always CC lkml when sending patches.

Ah, will do so in the future.
>
> On Tue, Jun 27, 2023 at 09:06:57PM -0500, Noah Goldstein wrote:
> > The special case for odd aligned buffers is unnecessary and mostly
> > just adds overhead. Aligned buffers is the expectations, and even for
> > unaligned buffer, the only case that was helped is if the buffer was
> > 1-byte from word aligned which is ~1/7 of the cases. Overall it seems
> > highly unlikely to be worth to extra branch.
> >
> > It was left in the previous perf improvement patch because I was
> > erroneously comparing the exact output of `csum_partial(...)`, but
> > really we only need `csum_fold(csum_partial(...))` to match so its
> > safe to remove.
> >
> > All csum kunit tests pass.
> >
> > Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
> > ---
> >  arch/x86/lib/csum-partial_64.c | 37 ++--------------------------------
> >  1 file changed, 2 insertions(+), 35 deletions(-)
> >
> > diff --git a/arch/x86/lib/csum-partial_64.c b/arch/x86/lib/csum-partial=
_64.c
> > index cea25ca8b8cf..d06112e98893 100644
> > --- a/arch/x86/lib/csum-partial_64.c
> > +++ b/arch/x86/lib/csum-partial_64.c
> > @@ -11,28 +11,6 @@
> >  #include <asm/checksum.h>
> >  #include <asm/word-at-a-time.h>
> >
> > -static inline unsigned short from32to16(unsigned a)
> > -{
> > -     unsigned short b =3D a >> 16;
> > -     asm("addw %w2,%w0\n\t"
> > -         "adcw $0,%w0\n"
> > -         : "=3Dr" (b)
> > -         : "0" (b), "r" (a));
> > -     return b;
> > -}
> > -
> > -static inline __wsum csum_tail(u64 temp64, int odd)
> > -{
> > -     unsigned int result;
> > -
> > -     result =3D add32_with_carry(temp64 >> 32, temp64 & 0xffffffff);
> > -     if (unlikely(odd)) {
> > -             result =3D from32to16(result);
> > -             result =3D ((result >> 8) & 0xff) | ((result & 0xff) << 8=
);
> > -     }
> > -     return (__force __wsum)result;
> > -}
> > -
> >  /*
> >   * Do a checksum on an arbitrary memory area.
> >   * Returns a 32bit checksum.
> > @@ -47,17 +25,6 @@ static inline __wsum csum_tail(u64 temp64, int odd)
> >  __wsum csum_partial(const void *buff, int len, __wsum sum)
> >  {
> >       u64 temp64 =3D (__force u64)sum;
> > -     unsigned odd;
> > -
> > -     odd =3D 1 & (unsigned long) buff;
> > -     if (unlikely(odd)) {
> > -             if (unlikely(len =3D=3D 0))
> > -                     return sum;
> > -             temp64 =3D ror32((__force u32)sum, 8);
> > -             temp64 +=3D (*(unsigned char *)buff << 8);
> > -             len--;
> > -             buff++;
> > -     }
> >
> >       /*
> >        * len =3D=3D 40 is the hot case due to IPv6 headers, but annotat=
ing it likely()
> > @@ -73,7 +40,7 @@ __wsum csum_partial(const void *buff, int len, __wsum=
 sum)
> >                   "adcq $0,%[res]"
> >                   : [res] "+r"(temp64)
> >                   : [src] "r"(buff), "m"(*(const char(*)[40])buff));
> > -             return csum_tail(temp64, odd);
> > +             return add32_with_carry(temp64 >> 32, temp64 & 0xffffffff=
);
> >       }
> >       if (unlikely(len >=3D 64)) {
> >               /*
> > @@ -143,7 +110,7 @@ __wsum csum_partial(const void *buff, int len, __ws=
um sum)
> >                   : [res] "+r"(temp64)
> >                   : [trail] "r"(trail));
> >       }
> > -     return csum_tail(temp64, odd);
> > +     return add32_with_carry(temp64 >> 32, temp64 & 0xffffffff);
> >  }
> >  EXPORT_SYMBOL(csum_partial);
> >
> > --
> > 2.34.1
> >
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette