1. Patchew
  2. linux
  3. [PATCH v2 0/6] lib: Extend bitmap find binary operations
  4. View patch

[PATCH v2 4/6] lib: Fix test_find_first_and_bit and test_find_next_and_bit benchmark

Mathieu Desnoyers posted 6 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH v2 4/6] lib: Fix test_find_first_and_bit and test_find_next_and_bit benchmark
Posted by Mathieu Desnoyers 1 year, 5 months ago 
Modify test_find_first_bit so it modifies a local copy of bitmap rather
than modifying the input bitmap, which removes the requirement of
placing it last in the tests.

Calls to test_find_first_and_bit and test_find_next_and_bit are placed
after test_find_first_bit, which makes them use a bitmap entirely filled
rather than the expected bitmap (random-filled or sparse).

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/find_bit_benchmark.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/find_bit_benchmark.c b/lib/find_bit_benchmark.c
index d3fb09e6eff1..aee2ebb6b3cd 100644
--- a/lib/find_bit_benchmark.c
+++ b/lib/find_bit_benchmark.c
@@ -30,18 +30,20 @@ static DECLARE_BITMAP(bitmap, BITMAP_LEN) __initdata;
 static DECLARE_BITMAP(bitmap2, BITMAP_LEN) __initdata;
 
 /*
- * This is Schlemiel the Painter's algorithm. It should be called after
- * all other tests for the same bitmap because it sets all bits of bitmap to 1.
+ * This is Schlemiel the Painter's algorithm.
  */
 static int __init test_find_first_bit(void *bitmap, unsigned long len)
 {
+	static DECLARE_BITMAP(cp, BITMAP_LEN) __initdata;
 	unsigned long i, cnt;
 	ktime_t time;
 
+	bitmap_copy(cp, bitmap, BITMAP_LEN);
+
 	time = ktime_get();
 	for (cnt = i = 0; i < len; cnt++) {
-		i = find_first_bit(bitmap, len);
-		__clear_bit(i, bitmap);
+		i = find_first_bit(cp, len);
+		__clear_bit(i, cp);
 	}
 	time = ktime_get() - time;
 	pr_err("find_first_bit:     %18llu ns, %6ld iterations\n", time, cnt);
-- 
2.39.2
Re: [PATCH v2 4/6] lib: Fix test_find_first_and_bit and test_find_next_and_bit benchmark
Posted by Yury Norov 1 year, 5 months ago 
On Thu, Aug 29, 2024 at 09:59:24AM -0400, Mathieu Desnoyers wrote:
> Modify test_find_first_bit so it modifies a local copy of bitmap rather
> than modifying the input bitmap, which removes the requirement of
> placing it last in the tests.
> 
> Calls to test_find_first_and_bit and test_find_next_and_bit are placed
> after test_find_first_bit, which makes them use a bitmap entirely filled
> rather than the expected bitmap (random-filled or sparse).
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  lib/find_bit_benchmark.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/find_bit_benchmark.c b/lib/find_bit_benchmark.c
> index d3fb09e6eff1..aee2ebb6b3cd 100644
> --- a/lib/find_bit_benchmark.c
> +++ b/lib/find_bit_benchmark.c
> @@ -30,18 +30,20 @@ static DECLARE_BITMAP(bitmap, BITMAP_LEN) __initdata;
>  static DECLARE_BITMAP(bitmap2, BITMAP_LEN) __initdata;
>  
>  /*
> - * This is Schlemiel the Painter's algorithm. It should be called after
> - * all other tests for the same bitmap because it sets all bits of bitmap to 1.
> + * This is Schlemiel the Painter's algorithm.
>   */

Good to drop it, moreover, the comment is incorrect - we set all bits
to 0, not 1.

>  static int __init test_find_first_bit(void *bitmap, unsigned long len)
>  {
> +	static DECLARE_BITMAP(cp, BITMAP_LEN) __initdata;

This days we can allocate automatic variables, which is better than
statics:

   unsigned long *cp __free(bitmap) = bitmap_alloc(len, GFP_KERNEL);

If no objections, I can fix it inplace. The rest of the series looks
good. I'll add it in bitmap-for-next for testing shortly.

Thanks,
Yury

>  	unsigned long i, cnt;
>  	ktime_t time;
>  
> +	bitmap_copy(cp, bitmap, BITMAP_LEN);
> +
>  	time = ktime_get();
>  	for (cnt = i = 0; i < len; cnt++) {
> -		i = find_first_bit(bitmap, len);
> -		__clear_bit(i, bitmap);
> +		i = find_first_bit(cp, len);
> +		__clear_bit(i, cp);
>  	}
>  	time = ktime_get() - time;
>  	pr_err("find_first_bit:     %18llu ns, %6ld iterations\n", time, cnt);
> -- 
> 2.39.2
Re: [PATCH v2 4/6] lib: Fix test_find_first_and_bit and test_find_next_and_bit benchmark
Posted by Mathieu Desnoyers 1 year, 5 months ago 
On 2024-08-30 17:46, Yury Norov wrote:
> On Thu, Aug 29, 2024 at 09:59:24AM -0400, Mathieu Desnoyers wrote:
>> Modify test_find_first_bit so it modifies a local copy of bitmap rather
>> than modifying the input bitmap, which removes the requirement of
>> placing it last in the tests.
>>
>> Calls to test_find_first_and_bit and test_find_next_and_bit are placed
>> after test_find_first_bit, which makes them use a bitmap entirely filled
>> rather than the expected bitmap (random-filled or sparse).
>>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Yury Norov <yury.norov@gmail.com>
>> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> ---
>>   lib/find_bit_benchmark.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/find_bit_benchmark.c b/lib/find_bit_benchmark.c
>> index d3fb09e6eff1..aee2ebb6b3cd 100644
>> --- a/lib/find_bit_benchmark.c
>> +++ b/lib/find_bit_benchmark.c
>> @@ -30,18 +30,20 @@ static DECLARE_BITMAP(bitmap, BITMAP_LEN) __initdata;
>>   static DECLARE_BITMAP(bitmap2, BITMAP_LEN) __initdata;
>>   
>>   /*
>> - * This is Schlemiel the Painter's algorithm. It should be called after
>> - * all other tests for the same bitmap because it sets all bits of bitmap to 1.
>> + * This is Schlemiel the Painter's algorithm.
>>    */
> 
> Good to drop it, moreover, the comment is incorrect - we set all bits
> to 0, not 1.
> 
>>   static int __init test_find_first_bit(void *bitmap, unsigned long len)
>>   {
>> +	static DECLARE_BITMAP(cp, BITMAP_LEN) __initdata;
> 
> This days we can allocate automatic variables, which is better than
> statics:
> 
>     unsigned long *cp __free(bitmap) = bitmap_alloc(len, GFP_KERNEL);
> 
> If no objections, I can fix it inplace. The rest of the series looks
> good. I'll add it in bitmap-for-next for testing shortly.

I'm ok with the in-place fix, thanks!

Mathieu

> 
> Thanks,
> Yury
> 
>>   	unsigned long i, cnt;
>>   	ktime_t time;
>>   
>> +	bitmap_copy(cp, bitmap, BITMAP_LEN);
>> +
>>   	time = ktime_get();
>>   	for (cnt = i = 0; i < len; cnt++) {
>> -		i = find_first_bit(bitmap, len);
>> -		__clear_bit(i, bitmap);
>> +		i = find_first_bit(cp, len);
>> +		__clear_bit(i, cp);
>>   	}
>>   	time = ktime_get() - time;
>>   	pr_err("find_first_bit:     %18llu ns, %6ld iterations\n", time, cnt);
>> -- 
>> 2.39.2

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.