linux-kernel - Re: [PATCH v3 1/4] lib/find_bit: introduce FIND_FIRST

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: <xhsmhh71jb160.mognet@vschneid.remote.csb>
Date:   Wed, 07 Sep 2022 17:27:03 +0100
From:   Valentin Schneider <vschneid@...hat.com>
To:     Yury Norov <yury.norov@...il.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-kernel@...r.kernel.org
Cc:     Yury Norov <yury.norov@...il.com>,
        Alexey Klimov <klimov.linux@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Andy Whitcroft <apw@...onical.com>,
        Catalin Marinas <catalin.marinas@....com>,
        David Laight <David.Laight@...LAB.COM>,
        Dennis Zhou <dennis@...nel.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Kees Cook <keescook@...omium.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH v3 1/4] lib/find_bit: introduce FIND_FIRST_BIT() macro

On 27/08/22 10:58, Yury Norov wrote:
> Now that we have many flavors of find_first_bit(), and expect even more,
> it's better to have one macro that generates optimal code for all and makes
> maintaining of slightly different functions simpler.
>
> The logic common to all versions is moved to the new macro, and all the
> flavors are generated by providing an FETCH macro-parameter, like
> in this example:
>
>   #define FIND_FIRST_BIT(FETCH, MUNGE, size) ...
>
>   find_first_ornot_and_bit(addr1, addr2, addr3, size)
>   {
>         return FIND_NEXT_BIT(addr1[idx] | ~addr2[idx] & addr3[idx], /* nop */, size);
>   }
>
> The FETCH may be of any complexity, as soon as it only refers
> the bitmap(s) and an iterator idx.
>
> MUNGE is here to support _le code generation for BE builds. May be
> empty.
>
> Suggested-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Signed-off-by: Yury Norov <yury.norov@...il.com>

Just one small comment below about the /* nop */, regardless:

Reviewed-by: Valentin Schneider <vschneid@...hat.com>

>  unsigned long _find_first_bit(const unsigned long *addr, unsigned long size)
>  {
> -	unsigned long idx;
> -
> -	for (idx = 0; idx * BITS_PER_LONG < size; idx++) {
> -		if (addr[idx])
> -			return min(idx * BITS_PER_LONG + __ffs(addr[idx]), size);
> -	}
> -
> -	return size;
> +	return FIND_FIRST_BIT(addr[idx], /* nop */, size);

FWIW I thought passing an explicit identity-mapping macro would make things
a bit clearer, but not really since you have to hunt for where that macro
is defined (an inline "lambda x : x" would have been perfect :-)), so I
think what you've gone for is the lesser evil.

>  }
>  EXPORT_SYMBOL(_find_first_bit);