* [PATCH] radix tree test suite: put definition of bitmap_clear() into lib/bitmap.c
@ 2024-07-10 9:55 Wei Yang
2024-07-10 16:17 ` Yury Norov
0 siblings, 1 reply; 4+ messages in thread
From: Wei Yang @ 2024-07-10 9:55 UTC (permalink / raw)
To: yury.norov, linux, willy; +Cc: linux-kernel, Wei Yang
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@gmail.com>
CC: Matthew Wilcox <willy@infradead.org>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] radix tree test suite: put definition of bitmap_clear() into lib/bitmap.c
2024-07-10 9:55 [PATCH] radix tree test suite: put definition of bitmap_clear() into lib/bitmap.c Wei Yang
@ 2024-07-10 16:17 ` Yury Norov
2024-07-10 16:31 ` Wei Yang
0 siblings, 1 reply; 4+ messages in thread
From: Yury Norov @ 2024-07-10 16:17 UTC (permalink / raw)
To: Wei Yang; +Cc: linux, willy, linux-kernel
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@gmail.com>
> CC: Matthew Wilcox <willy@infradead.org>
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] radix tree test suite: put definition of bitmap_clear() into lib/bitmap.c
2024-07-10 16:17 ` Yury Norov
@ 2024-07-10 16:31 ` Wei Yang
2024-07-10 16:44 ` Yury Norov
0 siblings, 1 reply; 4+ messages in thread
From: Wei Yang @ 2024-07-10 16:31 UTC (permalink / raw)
To: Yury Norov; +Cc: Wei Yang, linux, willy, linux-kernel
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@gmail.com>
>> CC: Matthew Wilcox <willy@infradead.org>
>
>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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] radix tree test suite: put definition of bitmap_clear() into lib/bitmap.c
2024-07-10 16:31 ` Wei Yang
@ 2024-07-10 16:44 ` Yury Norov
0 siblings, 0 replies; 4+ messages in thread
From: Yury Norov @ 2024-07-10 16:44 UTC (permalink / raw)
To: Wei Yang; +Cc: linux, willy, linux-kernel
On Wed, Jul 10, 2024 at 04:31:34PM +0000, Wei Yang wrote:
> 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@gmail.com>
> >> CC: Matthew Wilcox <willy@infradead.org>
> >
> >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.
No, I didn't.
> 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'
Ok then. If xarray userspace test pulls code directly from kernel
part, we still need bitmap_clear.
Since the function was introduced, we wrote an inline version of
it. We usually try to keep tools implementation synced with the
mother kernel, particularly for extra testing coverage.
Can you please send a v2 that syncs tools implementation with the
kernel one, and can you also mention how it is used in the code,
so those lazy folks like me will not get fooled with grep.
Thanks,
Yury
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-07-10 16:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-10 9:55 [PATCH] radix tree test suite: put definition of bitmap_clear() into lib/bitmap.c Wei Yang
2024-07-10 16:17 ` Yury Norov
2024-07-10 16:31 ` Wei Yang
2024-07-10 16:44 ` Yury Norov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox