[Cluster-devel] [GFS2] Faster gfs2_bitfit algorithm

[Steven Whitehouse]

Hi,

On Fri, 2008-03-07 at 23:12 -0600, Bob Peterson wrote:
> Hi,
> 
> This version of the gfs2_bitfit algorithm is up to four
> times faster than its predecessor.
> 
> Regards,
> 
> Bob Peterson
> 
> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
> --
>  fs/gfs2/rgrp.c |   79 +++++++++++++++++++++++++++++++++----------------------
>  1 files changed, 47 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 4291375..4dee88b 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -33,6 +33,16 @@
>  #define BFITNOENT ((u32)~0)
>  #define NO_BLOCK ((u64)~0)
>  
> +#if BITS_PER_LONG == 32
> +#define LBITMASK   (unsigned long)(0x55555555)
> +#define LBITSKIP55 (unsigned long)(0x55555555)
> +#define LBITSKIP00 (unsigned long)(0x00000000)
> +#else
> +#define LBITMASK   (unsigned long)(0x5555555555555555)
> +#define LBITSKIP55 (unsigned long)(0x5555555555555555)
> +#define LBITSKIP00 (unsigned long)(0x0000000000000000)
> +#endif
> +
You can just use the UL suffix rather than a cast. Also if you'd used
sizeof(unsigned long) == 4 then that would have worked for your
userspace tests as well.

>  /*
>   * These routines are used by the resource group routines (rgrp.c)
>   * to keep track of block allocation.  Each block is represented by two
> @@ -138,43 +148,48 @@ static inline unsigned char gfs2_testbit(struct gfs2_rgrpd *rgd,
>  static u32 gfs2_bitfit(const u8 *buffer, unsigned int buflen, u32 goal,
>  		       u8 old_state)
>  {
> -	const u8 *byte;
> -	u32 blk = goal;
> -	unsigned int bit, bitlong;
> -	const unsigned long *plong;
> -#if BITS_PER_LONG == 32
> -	const unsigned long plong55 = 0x55555555;
> -#else
> -	const unsigned long plong55 = 0x5555555555555555;
> -#endif
> -
> -	byte = buffer + (goal / GFS2_NBBY);
> -	plong = (const unsigned long *)(buffer + (goal / GFS2_NBBY));
> -	bit = (goal % GFS2_NBBY) * GFS2_BIT_SIZE;
> -	bitlong = bit;
> -
> -	while (byte < buffer + buflen) {
> -
> -		if (bitlong == 0 && old_state == 0 && *plong == plong55) {
> -			plong++;
> -			byte += sizeof(unsigned long);
> -			blk += sizeof(unsigned long) * GFS2_NBBY;
> -			continue;
> +	const u8 *byte, *start, *end;
> +	int bit, startbit;
> +	u32 g1, g2, misaligned;
> +	unsigned long *plong, lskipval;
> +	unsigned long lskipvals[4] = {LBITSKIP55, LBITSKIP00,
> +				      LBITSKIP55, LBITSKIP00};
> +
It looks like you don't actually need an array here, but just a
conditional based upon one of the bits of "old_state" ? Also I think
lskipval can be marked const.

> +	lskipval = lskipvals[old_state];
> +	g1 = (goal / GFS2_NBBY);
> +	start = buffer + g1;
> +	byte = start;
> +        end = buffer + buflen;
> +	g2 = ALIGN(g1, sizeof(unsigned long));
> +	plong = (unsigned long *)(buffer + g2);
> +	startbit = bit = (goal % GFS2_NBBY) * GFS2_BIT_SIZE;
> +	misaligned = g2 - g1;
> +	while (byte < end) {
> +
> +		if (bit == 0 && !misaligned) {
> +			if (((*plong) & LBITMASK) == lskipval) {
> +				plong++;
> +				byte += sizeof(unsigned long);
> +				continue;
> +			}
> +		}
Now this is the bit I'm most worried about. We ought to be able to get
rid of the test for misaligned simply by testing for that once at the
start of the loop and jumping either to the "unsigned long at a time" or
"byte at a time" parts of the loop. Also it should be possible to get
rid of the increment of the byte counter by updating it based on the
plong pointer when the loop has terminated for some reason.

As per the note in the bz, it ought to be possible to only have two
tests in this loop: have we hit the end of the buffer, and have we found
what we are looking for.

Did you try out using prefetch(), and/or unrolling the loop by testing
two (or more) unsigned longs at once? See <linux/prefetch.h>

Having said that, the results that you are seeing are a significant
improvement on what was there before and I think should make a
worthwhile difference to the speed of our block allocation,

Steve.