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

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

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