linux-kernel - Re: [PATCH v2 4/6] lib: Fix test_find_first_and_bit and test_find_next_and

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <86bb4c08-5706-4902-9ef5-a336123ba8e1@efficios.com>
Date: Fri, 30 Aug 2024 11:50:40 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Yury Norov <yury.norov@...il.com>
Cc: Rasmus Villemoes <linux@...musvillemoes.dk>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/6] lib: Fix test_find_first_and_bit and
 test_find_next_and_bit benchmark

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@...icios.com>
>> Cc: Yury Norov <yury.norov@...il.com>
>> Cc: Rasmus Villemoes <linux@...musvillemoes.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