Re: [PATCH 2/4] find: add find_first_andnot_bit()

Yury Norov
2025-04-24T02:57:25Z

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@gmail.com>
> >
> > The function helps to implement cpumask_andnot() APIs.
> >
> > Signed-off-by: Yury Norov [NVIDIA] <yury.norov@gmail.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.