linux-kernel - Re: [PATCH v11 2/2] lib/test_bitmap: add tests for bitmap

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: <27f5d15b-28da-469b-9625-1c840f33b60d@intel.com>
Date:   Thu, 9 Nov 2023 15:32:46 +0100
From:   Alexander Lobakin <aleksander.lobakin@...el.com>
To:     Alexander Potapenko <glider@...gle.com>
CC:     <catalin.marinas@....com>, <will@...nel.org>, <pcc@...gle.com>,
        <andreyknvl@...il.com>, <andriy.shevchenko@...ux.intel.com>,
        <linux@...musvillemoes.dk>, <yury.norov@...il.com>,
        <alexandru.elisei@....com>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <eugenis@...gle.com>,
        <syednwaris@...il.com>, <william.gray@...aro.org>
Subject: Re: [PATCH v11 2/2] lib/test_bitmap: add tests for
 bitmap_{read,write}()

From: Alexander Potapenko <glider@...gle.com>
Date: Thu, 9 Nov 2023 15:28:56 +0100

>>
>> Could we maybe rather extend __check_eq_uint to take ulongs? Doesn't
>> seem like they differ a lot.
> 
> We could redefine expect_eq_uint as:
> 
> #define expect_eq_uint(x, y)           expect_eq_ulong((unsigned
> int)(x), (unsigned int)(y))

Do we need explicit casts here tho?

> 
> and throw __expect_eq_uint away.
> 
> 
>>> +     }
>>> +     time = ktime_get() - time;
>>> +     pr_err("Time spent in %s:\t%llu\n", __func__, time);
>>
>> pr_err() is for printing errors and is shown in red by some log readers.
>> Maybe use pr_info() or pr_notice()? Definitely not an error or even warning.
> 
> Note that test_bitmap.c has 17 calls of pr_err() and 7 calls of
> pr_warn(), which aren't really consistent (e.g. they are used in
> certain __check helpers instead of pr_err()), and the existing
> performance tests are calling pr_err().

Correct, and that's what caught my attention: visual grepping for bitmap
messages makes no sense because some of them are red even thought all
tests pass correctly.

> I can change that in a separate patch, if you think it's worth the
> effort: the error messages should probably remain pr_err(), but the
> informational ones could be made pr_info().

Sounds good to be, would be nice to see!

[...]

>>> @@ -1237,6 +1411,9 @@ static void __init selftest(void)
>>>       test_bitmap_cut();
>>>       test_bitmap_print_buf();
>>>       test_bitmap_const_eval();
>>> +     test_bitmap_read_write();
>>> +     test_bitmap_read_perf();
>>> +     test_bitmap_write_perf();
>>>
>>>       test_find_nth_bit();
>>>       test_for_each_set_bit();
>>
>> Thanks,
>> Olek
> 
> 
> 
> --
> Alexander Potapenko
> Software Engineer
> 
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
> 
> Geschäftsführer: Paul Manicle, Liana Sebastian
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg

Thanks,
Olek