gfs2: avoid inefficient use of crc32_le_shift()

[PATCH] gfs2: avoid inefficient use of crc32_le_shift()

Posted by Eric Biggers 7 months, 1 week ago

From: Eric Biggers <ebiggers@google.com>

__get_log_header() was using crc32_le_shift() to update a CRC with four
zero bytes.  However, this is about 5x slower than just CRC'ing four
zero bytes in the normal way.  Just do that instead.

(We could instead make crc32_le_shift() faster on short lengths.  But
all its callers do just fine without it, so I'd like to just remove it.)

Signed-off-by: Eric Biggers <ebiggers@google.com>
---

Please consider this patch for the gfs2 tree for 6.16.  If it doesn't
get picked up, I'll take it through the crc tree.

 fs/gfs2/recovery.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index f4fe7039f725b..9080f1b5f434d 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -116,19 +116,20 @@ void gfs2_revoke_clean(struct gfs2_jdesc *jd)
 }
 
 int __get_log_header(struct gfs2_sbd *sdp, const struct gfs2_log_header *lh,
 		     unsigned int blkno, struct gfs2_log_header_host *head)
 {
+	const u32 zero = 0;
 	u32 hash, crc;
 
 	if (lh->lh_header.mh_magic != cpu_to_be32(GFS2_MAGIC) ||
 	    lh->lh_header.mh_type != cpu_to_be32(GFS2_METATYPE_LH) ||
 	    (blkno && be32_to_cpu(lh->lh_blkno) != blkno))
 		return 1;
 
 	hash = crc32(~0, lh, LH_V1_SIZE - 4);
-	hash = ~crc32_le_shift(hash, 4); /* assume lh_hash is zero */
+	hash = ~crc32(hash, &zero, 4); /* assume lh_hash is zero */
 
 	if (be32_to_cpu(lh->lh_hash) != hash)
 		return 1;
 
 	crc = crc32c(~0, (void *)lh + LH_V1_SIZE + 4,

base-commit: 3ce9925823c7d6bb0e6eb951bf2db0e9e182582d
-- 
2.49.0

Re: [PATCH] gfs2: avoid inefficient use of crc32_le_shift()

Posted by Andreas Gruenbacher 7 months, 1 week ago

On Sun, May 11, 2025 at 8:40 PM Eric Biggers <ebiggers@kernel.org> wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> __get_log_header() was using crc32_le_shift() to update a CRC with four
> zero bytes.  However, this is about 5x slower than just CRC'ing four
> zero bytes in the normal way.  Just do that instead.
>
> (We could instead make crc32_le_shift() faster on short lengths.  But
> all its callers do just fine without it, so I'd like to just remove it.)
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>
> Please consider this patch for the gfs2 tree for 6.16.  If it doesn't
> get picked up, I'll take it through the crc tree.
>
>  fs/gfs2/recovery.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
> index f4fe7039f725b..9080f1b5f434d 100644
> --- a/fs/gfs2/recovery.c
> +++ b/fs/gfs2/recovery.c
> @@ -116,19 +116,20 @@ void gfs2_revoke_clean(struct gfs2_jdesc *jd)
>  }
>
>  int __get_log_header(struct gfs2_sbd *sdp, const struct gfs2_log_header *lh,
>                      unsigned int blkno, struct gfs2_log_header_host *head)
>  {
> +       const u32 zero = 0;
>         u32 hash, crc;
>
>         if (lh->lh_header.mh_magic != cpu_to_be32(GFS2_MAGIC) ||
>             lh->lh_header.mh_type != cpu_to_be32(GFS2_METATYPE_LH) ||
>             (blkno && be32_to_cpu(lh->lh_blkno) != blkno))
>                 return 1;
>
>         hash = crc32(~0, lh, LH_V1_SIZE - 4);
> -       hash = ~crc32_le_shift(hash, 4); /* assume lh_hash is zero */
> +       hash = ~crc32(hash, &zero, 4); /* assume lh_hash is zero */
>
>         if (be32_to_cpu(lh->lh_hash) != hash)
>                 return 1;
>
>         crc = crc32c(~0, (void *)lh + LH_V1_SIZE + 4,
>
> base-commit: 3ce9925823c7d6bb0e6eb951bf2db0e9e182582d
> --
> 2.49.0
>

Applied to for-next.

Thanks,
Andreas