linux-kernel - Re: [PATCH 2/4] find: add find_first_andnot

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: <aAmokt3PBX8LzbwJ@yury>
Date: Wed, 23 Apr 2025 22:57:22 -0400
From: Yury Norov <yury.norov@...il.com>
To: Reinette Chatre <reinette.chatre@...el.com>
Cc: Tony Luck <tony.luck@...el.com>, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>, x86@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] find: add find_first_andnot_bit()

On Wed, Apr 23, 2025 at 02:28:26PM -0700, Reinette Chatre wrote:
> Hi Yury,
> 
> On 4/7/25 8:38 AM, Yury Norov wrote:
> > From: Yury Norov [NVIDIA] <yury.norov@...il.com>
> > 
> > The function helps to implement cpumask_andnot() APIs.
> > 
> > Signed-off-by: Yury Norov [NVIDIA] <yury.norov@...il.com>
> > ---
> >  include/linux/find.h | 25 +++++++++++++++++++++++++
> >  lib/find_bit.c       | 11 +++++++++++
> >  2 files changed, 36 insertions(+)
> > 
> > diff --git a/include/linux/find.h b/include/linux/find.h
> > index 68685714bc18..d1578cfb667c 100644
> > --- a/include/linux/find.h
> > +++ b/include/linux/find.h
> > @@ -29,6 +29,8 @@ unsigned long __find_nth_and_andnot_bit(const unsigned long *addr1, const unsign
> >  					unsigned long n);
> >  extern unsigned long _find_first_and_bit(const unsigned long *addr1,
> >  					 const unsigned long *addr2, unsigned long size);
> > +unsigned long _find_first_andnot_bit(const unsigned long *addr1, const unsigned long *addr2,
> > +				 unsigned long size);
> >  unsigned long _find_first_and_and_bit(const unsigned long *addr1, const unsigned long *addr2,
> >  				      const unsigned long *addr3, unsigned long size);
> >  extern unsigned long _find_first_zero_bit(const unsigned long *addr, unsigned long size);
> > @@ -347,6 +349,29 @@ unsigned long find_first_and_bit(const unsigned long *addr1,
> >  }
> >  #endif
> >  
> > +/**
> > + * find_first_andnot_bit - find the first bit set in 1st memory region and unset in 2nd
> > + * @addr1: The first address to base the search on
> > + * @addr2: The second address to base the search on
> > + * @size: The bitmap size in bits
> > + *
> > + * Returns the bit number for the matched bit
> > + * If no bits are set, returns >= @size.
> 
> Should this be "If no bits are set, returns @size." to match similar document
> snippets as well as what the code does?
> 
> I am not familiar with the customs of this area but I did notice that
> this patch triggers some checkpatch.pl warnings about alignment not
> matching open parenthesis ... but looking at the existing content of this
> file this custom does not seem to apply here.

It was a really unclever decision to make find_bit() API returning
the size exactly, because this ends up with a few extra instructions
that we can avoid. For example, in FIND_FIRST_BIT() we have to return

        min(idx * BITS_PER_LONG + __ffs(MUNGE(val)), sz);

instead of just

        idx * BITS_PER_LONG + __ffs(MUNGE(val)

Unfortunately, the existing codebase relies on this overly strict
guarantee. For the new API, I encourage people to guarantee '>= size',
not '== size'. There's no difference from practical perspective. But
if one day somebody will decide to clean this up, there will be less 
burden to rework.