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

[Wei Yang]

On Wed, Jul 10, 2024 at 09:17:00AM -0700, Yury Norov wrote:
>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.

I guess you didn't try to compile the test. After remove the bitmap_clear(),
we see this error.

cc -fsanitize=address -fsanitize=undefined  main.o xarray.o radix-tree.o idr.o linux.o test.o maple.o slab.o find_bit.o bitmap.o hweight.o vsprintf.o 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  -lpthread -lurcu -o main
/usr/bin/ld: xarray.o: in function `xas_squash_marks':
/home/richard/git/linux/tools/testing/radix-tree/../../../lib/xarray.c:139: undefined reference to `bitmap_clear'

>
>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

-- 
Wei Yang
Help you, Help me