Re: [PATCH] Revert "sh: Handle calling csum_partial with misaligned data"

From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Guenter Roeck <linux@roeck-us.net>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	Rich Felker <dalias@libc.org>,
	 linux-sh@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Revert "sh: Handle calling csum_partial with misaligned data"
Date: Thu, 02 May 2024 09:23:35 +0200	[thread overview]
Message-ID: <87b7b7a99b8addc82ac7ce229801b744c9ef838e.camel@physik.fu-berlin.de> (raw)
In-Reply-To: <CAMuHMdVcE79i0RPQHJBJjpXY6U-M-AQ2gh+C25u_777PZgPxXQ@mail.gmail.com>

Hi Geert,

On Thu, 2024-05-02 at 09:21 +0200, Geert Uytterhoeven wrote:
> On landisk:
> 
>  KTAP version 1
>  1..1
>      KTAP version 1
>      # Subtest: checksum
>      # module: checksum_kunit
>      1..5
> -    # test_csum_fixed_random_inputs: ASSERTION FAILED at
> lib/checksum_kunit.c:500
> -    Expected ( u64)result == ( u64)expec, but
> -        ( u64)result == 53378 (0xd082)
> -        ( u64)expec == 33488 (0x82d0)
> -    not ok 1 test_csum_fixed_random_inputs
> -    # test_csum_all_carry_inputs: ASSERTION FAILED at lib/checksum_kunit.c:525
> -    Expected ( u64)result == ( u64)expec, but
> -        ( u64)result == 65281 (0xff01)
> -        ( u64)expec == 65280 (0xff00)
> -    not ok 2 test_csum_all_carry_inputs
> -    # test_csum_no_carry_inputs: ASSERTION FAILED at lib/checksum_kunit.c:573
> -    Expected ( u64)result == ( u64)expec, but
> -        ( u64)result == 65535 (0xffff)
> -        ( u64)expec == 65534 (0xfffe)
> -    not ok 3 test_csum_no_carry_inputs
> +    # test_csum_fixed_random_inputs: Test should be marked slow
> (runtime: 9.814991070s)
> +    ok 1 test_csum_fixed_random_inputs
> +    # test_csum_all_carry_inputs: Test should be marked slow
> (runtime: 19.621274580s)
> +    ok 2 test_csum_all_carry_inputs
> +    # test_csum_no_carry_inputs: Test should be marked slow (runtime:
> 19.614096540s)
> +    ok 3 test_csum_no_carry_inputs
>      ok 4 test_ip_fast_csum
>      ok 5 test_csum_ipv6_magic
> -# checksum: pass:2 fail:3 skip:0 total:5
> -# Totals: pass:2 fail:3 skip:0 total:5
> -not ok 1 checksum
> +# checksum: pass:5 fail:0 skip:0 total:5
> +# Totals: pass:5 fail:0 skip:0 total:5
> +ok 1 checksum
> 
> As we aim for correctness over performance:
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> However, given the big impact on performance, it would be great if
> someone could find out what's wrong with the optimized version.

Thanks for testing this. I will pick this up then since it actually fixes a bug.

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913