Re: [PATCH] radix tree test suite: put definition of bitmap_clear() into lib/bitmap.c

[Yury Norov]

On Wed, Jul 10, 2024 at 09:55:54AM +0000, Wei Yang wrote:
> Instead of keeping a bitmap.c in radix-tree own directory, we can use
> the common implementation in lib/ directory.
> 
> Just move the definition and link some related libs, no functional
> change.
> 
> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
> CC: Matthew Wilcox <willy@xxxxxxxxxxxxx>

Hi Wei,

bitmap_clear() is not used in radix tests, and in fact not used
anywhere in tools at all. Unless you have plans on it, it's
better to drop bitmap_clear() from tools, instead of bloating
the library.

Thanks,
Yury

> ---
>  tools/lib/bitmap.c                | 20 ++++++++++++++++++++
>  tools/testing/radix-tree/Makefile |  4 ++--
>  tools/testing/radix-tree/bitmap.c | 23 -----------------------
>  3 files changed, 22 insertions(+), 25 deletions(-)
>  delete mode 100644 tools/testing/radix-tree/bitmap.c
> 
> diff --git a/tools/lib/bitmap.c b/tools/lib/bitmap.c
> index c3e4871967bc..a96dbf001244 100644
> --- a/tools/lib/bitmap.c
> +++ b/tools/lib/bitmap.c
> @@ -100,3 +100,23 @@ bool __bitmap_intersects(const unsigned long *bitmap1,
>  			return true;
>  	return false;
>  }
> +
> +void bitmap_clear(unsigned long *map, unsigned int start, int len)
> +{
> +	unsigned long *p = map + BIT_WORD(start);
> +	const unsigned int size = start + len;
> +	int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
> +	unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
> +
> +	while (len - bits_to_clear >= 0) {
> +		*p &= ~mask_to_clear;
> +		len -= bits_to_clear;
> +		bits_to_clear = BITS_PER_LONG;
> +		mask_to_clear = ~0UL;
> +		p++;
> +	}
> +	if (len) {
> +		mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
> +		*p &= ~mask_to_clear;
> +	}
> +}
> diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile
> index 7527f738b4a1..505cc24a8b9c 100644
> --- a/tools/testing/radix-tree/Makefile
> +++ b/tools/testing/radix-tree/Makefile
> @@ -5,8 +5,8 @@ CFLAGS += -I. -I../../include -I../../../lib -g -Og -Wall \
>  LDFLAGS += -fsanitize=address -fsanitize=undefined
>  LDLIBS+= -lpthread -lurcu
>  TARGETS = main idr-test multiorder xarray maple
> -CORE_OFILES := xarray.o radix-tree.o idr.o linux.o test.o find_bit.o bitmap.o \
> -			 slab.o maple.o
> +LIBS := find_bit.o bitmap.o hweight.o vsprintf.o
> +CORE_OFILES := xarray.o radix-tree.o idr.o linux.o test.o slab.o maple.o $(LIBS)
>  OFILES = main.o $(CORE_OFILES) regression1.o regression2.o regression3.o \
>  	 regression4.o tag_check.o multiorder.o idr-test.o iteration_check.o \
>  	 iteration_check_2.o benchmark.o
> diff --git a/tools/testing/radix-tree/bitmap.c b/tools/testing/radix-tree/bitmap.c
> deleted file mode 100644
> index 66ec4a24a203..000000000000
> --- a/tools/testing/radix-tree/bitmap.c
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/* lib/bitmap.c pulls in at least two other files. */
> -
> -#include <linux/bitmap.h>
> -
> -void bitmap_clear(unsigned long *map, unsigned int start, int len)
> -{
> -	unsigned long *p = map + BIT_WORD(start);
> -	const unsigned int size = start + len;
> -	int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
> -	unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
> -
> -	while (len - bits_to_clear >= 0) {
> -		*p &= ~mask_to_clear;
> -		len -= bits_to_clear;
> -		bits_to_clear = BITS_PER_LONG;
> -		mask_to_clear = ~0UL;
> -		p++;
> -	}
> -	if (len) {
> -		mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
> -		*p &= ~mask_to_clear;
> -	}
> -}
> -- 
> 2.34.1