linux-kernel - Re: [PATCH] crc32: Optimize inner loop.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20111109161539.b2cdf381.akpm@linux-foundation.org>
Date:	Wed, 9 Nov 2011 16:15:39 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Joakim Tjernlund <Joakim.Tjernlund@...nsmode.se>
Cc:	<linux-kernel@...r.kernel.org>,
	Bob Pearson <rpearson@...temfabricworks.com>,
	Frank Zago <fzago@...temfabricworks.com>
Subject: Re: [PATCH] crc32: Optimize inner loop.

On Thu, 3 Nov 2011 10:28:25 +0100
Joakim Tjernlund <Joakim.Tjernlund@...nsmode.se> wrote:

> taking a pointer reference to each row in the crc table matrix,
> one can reduce the inner loop with a few insn's
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@...nsmode.se>

As we have other developers who have previously shown an interest in
the crc32 code it is a good idea to cc them on future patches to get a
bit more review and perhaps testing input.

This patch seems fairly harmless.  Some runtime testing results would
have been appropriate.

Also, something like "reduce a few insns" is extremely easy to
substantiate, so why not do it?  With my compiler, the patch reduces
the crc32.o text by 22 bytes.

>  As the slice by 8 CRC32 optimizations seems stalled, I figured
>  I send in this generic one.

I meant to ask about that.  The discussion was fairly long and complex
and I'm now unsure whether the patch I have is a still up-to-date,
whether it is a good one, whether it has additional testing results,
etc.

So probably the best thing to do now is for me to drop it.  Please
resend when convenient.  Be sure to cc all previous reviewers and also
update the changelog to cover any important issues which were
brought up in the review.  This is to show what the issues were, how
they were addressed, etc.

Thanks.

> index 4855995..b06d1e7 100644
> --- a/lib/crc32.c
> +++ b/lib/crc32.c
> @@ -51,20 +51,21 @@ static inline u32
>  crc32_body(u32 crc, unsigned char const *buf, size_t len, const u32 (*tab)[256])
>  {
>  # ifdef __LITTLE_ENDIAN
> -#  define DO_CRC(x) crc = tab[0][(crc ^ (x)) & 255] ^ (crc >> 8)
> -#  define DO_CRC4 crc = tab[3][(crc) & 255] ^ \
> -		tab[2][(crc >> 8) & 255] ^ \
> -		tab[1][(crc >> 16) & 255] ^ \
> -		tab[0][(crc >> 24) & 255]
> +#  define DO_CRC(x) crc = t0[(crc ^ (x)) & 255] ^ (crc >> 8)
> +#  define DO_CRC4 crc = t3[(crc) & 255] ^ \
> +		t2[(crc >> 8) & 255] ^ \
> +		t1[(crc >> 16) & 255] ^ \
> +		t0[(crc >> 24) & 255]
>  # else
> -#  define DO_CRC(x) crc = tab[0][((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
> -#  define DO_CRC4 crc = tab[0][(crc) & 255] ^ \
> -		tab[1][(crc >> 8) & 255] ^ \
> -		tab[2][(crc >> 16) & 255] ^ \
> -		tab[3][(crc >> 24) & 255]
> +#  define DO_CRC(x) crc = t0[((crc >> 24) ^ (x)) & 255] ^ (crc << 8)
> +#  define DO_CRC4 crc = t0[(crc) & 255] ^ \
> +		t1[(crc >> 8) & 255] ^  \
> +		t2[(crc >> 16) & 255] ^	\
> +		t3[(crc >> 24) & 255]
>  # endif
>  	const u32 *b;
>  	size_t    rem_len;
> +	const u32 *t0=tab[0], *t1=tab[1], *t2=tab[2], *t3=tab[3];
>  
>  	/* Align it */
>  	if (unlikely((long)buf & 3 && len)) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/