Re: [PATCH] ext4: improve str2hashbuf by processing 4-byte chunks

[Guan-Chun Wu <409411716@gms.tku.edu.tw>]

Hi David,

On Sun, Nov 16, 2025 at 07:35:13PM +0000, David Laight wrote:
> On Sun, 16 Nov 2025 21:01:05 +0800
> Guan-Chun Wu <409411716@gms.tku.edu.tw> wrote:
> 
> > The original byte-by-byte implementation with modulo checks is less
> > efficient. Refactor str2hashbuf_unsigned() and str2hashbuf_signed()
> > to process input in explicit 4-byte chunks instead of using a
> > modulus-based loop to emit words byte by byte.
> 
> There are much bigger gains to be made - the current code is horrid.
> Not least due to the costs of the indirect calls.
> It is better to use conditionals than indirect calls. 
>

Thanks for the feedback. I'll remove the redundant casts, and for the
unsigned version I'll switch to using get_unaligned_be32() to avoid
duplicating the implementation. If this approach looks reasonable, I
can send a v2 that replaces the indirect calls with conditionals.

Best regards,
Guan-Chun

> 
> > 
> > This change removes per-byte modulo checks and reduces loop iterations,
> > improving efficiency.
> > 
> > Performance test (x86_64, Intel Core i7-10700 @ 2.90GHz, average over 10000
> > runs, using kernel module for testing):
> > 
> >     len | orig_s | new_s | orig_u | new_u
> >     ----+--------+-------+--------+-------
> >       1 |   70   |   71  |   63   |   63
> >       8 |   68   |   64  |   64   |   62
> >      32 |   75   |   70  |   75   |   63
> >      64 |   96   |   71  |  100   |   68
> >     255 |  192   |  108  |  187   |   84
> > 
> > Signed-off-by: Guan-Chun Wu <409411716@gms.tku.edu.tw>
> > ---
> >  fs/ext4/hash.c | 48 ++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 32 insertions(+), 16 deletions(-)
> > 
> > diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c
> > index 33cd5b6b02d5..75105828e8b4 100644
> > --- a/fs/ext4/hash.c
> > +++ b/fs/ext4/hash.c
> > @@ -141,21 +141,29 @@ static void str2hashbuf_signed(const char *msg, int len, __u32 *buf, int num)
> >  	pad = (__u32)len | ((__u32)len << 8);
> >  	pad |= pad << 16;
> >  
> > -	val = pad;
> >  	if (len > num*4)
> >  		len = num * 4;
> > -	for (i = 0; i < len; i++) {
> > -		val = ((int) scp[i]) + (val << 8);
> > -		if ((i % 4) == 3) {
> > -			*buf++ = val;
> > -			val = pad;
> > -			num--;
> > -		}
> > +
> > +	while (len >= 4) {
> > +		val = ((int)scp[0] << 24) + ((int)scp[1] << 16) +
> > +				((int)scp[2] << 8) + (int)scp[3];
> 
> The (int) casts are unnecessary (throughout), 'char' is always promoted to
> 'signed int' before any arithmetic.
> 
> > +		*buf++ = val;
> > +		scp += 4;
> > +		len -= 4;
> > +		num--;
> >  	}
> > +
> > +	val = pad;
> > +
> > +	for (i = 0; i < len; i++)
> > +		val = (int)scp[i] + (val << 8);
> > +
> >  	if (--num >= 0)
> >  		*buf++ = val;
> > +
> >  	while (--num >= 0)
> >  		*buf++ = pad;
> > +
> >  }
> >  
> >  static void str2hashbuf_unsigned(const char *msg, int len, __u32 *buf, int num)
> > @@ -167,21 +175,29 @@ static void str2hashbuf_unsigned(const char *msg, int len, __u32 *buf, int num)
> >  	pad = (__u32)len | ((__u32)len << 8);
> >  	pad |= pad << 16;
> >  
> > -	val = pad;
> >  	if (len > num*4)
> >  		len = num * 4;
> > -	for (i = 0; i < len; i++) {
> > -		val = ((int) ucp[i]) + (val << 8);
> > -		if ((i % 4) == 3) {
> > -			*buf++ = val;
> > -			val = pad;
> > -			num--;
> > -		}
> > +
> > +	while (len >= 4) {
> > +		val = ((int)ucp[0] << 24) | ((int)ucp[1] << 16) |
> > +				((int)ucp[2] << 8) | (int)ucp[3];
> 
> Isn't that get_misaligned_be32() ?
> 
> 	David 
>
> > +		*buf++ = val;
> > +		ucp += 4;
> > +		len -= 4;
> > +		num--;
> >  	}
> > +
> > +	val = pad;
> > +
> > +	for (i = 0; i < len; i++)
> > +		val = (int)ucp[i] + (val << 8);
> > +
> >  	if (--num >= 0)
> >  		*buf++ = val;
> > +
> >  	while (--num >= 0)
> >  		*buf++ = pad;
> > +
> >  }
> >  
> >  /*
>