Introduce a macro that can efficiently extract the least significant
non-zero bit from a value.

Interestingly, this bit-twiddling trick is open-coded in some places, but
it also appears to be little known, leading to various inefficient
implementations in other places. Let's make it part of the standard bitops
arsenal.

Define the macro in a separate header file included from <linux/bitops.h>,
to allow using it in very low-level header files that may not want to
include all of <linux/bitops.h>.

Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
 MAINTAINERS             |  1 +
 include/linux/bitops.h  |  1 +
 include/linux/ffs_val.h | 21 +++++++++++++++++++++
 3 files changed, 23 insertions(+)
 create mode 100644 include/linux/ffs_val.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a0dd762f5648b..8f15c76a67ea2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4466,6 +4466,7 @@ F:	arch/*/lib/bitops.c
 F:	include/asm-generic/bitops
 F:	include/asm-generic/bitops.h
 F:	include/linux/bitops.h
+F:	include/linux/ffs_val.h
 F:	lib/hweight.c
 F:	lib/test_bitops.c
 F:	tools/*/bitops*
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index ea7898cc59039..209f0c3e07b9e 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -4,6 +4,7 @@
 
 #include <asm/types.h>
 #include <linux/bits.h>
+#include <linux/ffs_val.h>
 #include <linux/typecheck.h>
 
 #include <uapi/linux/kernel.h>
diff --git a/include/linux/ffs_val.h b/include/linux/ffs_val.h
new file mode 100644
index 0000000000000..193ec86d2b53b
--- /dev/null
+++ b/include/linux/ffs_val.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_LINUX_FFS_VAL_H_
+#define _ASM_LINUX_FFS_VAL_H_
+
+/**
+ * ffs_val - find the value of the first set bit
+ * @x: the value to search
+ *
+ * Unlike ffs(), which returns a bit position, ffs_val() returns the bit
+ * value itself.
+ *
+ * Returns:
+ * least significant non-zero bit, 0 if all bits are zero
+ */
+#define ffs_val(x)			\
+({					\
+	const typeof(x) val__ = (x);	\
+	val__ & -val__;			\
+})
+
+#endif /* _ASM_LINUX_FFS_VAL_H_ */
-- 
2.52.0

Hi Petr

Am 09.01.26 um 17:37 schrieb Petr Tesarik:
> Introduce a macro that can efficiently extract the least significant
> non-zero bit from a value.
>
> Interestingly, this bit-twiddling trick is open-coded in some places, but
> it also appears to be little known, leading to various inefficient
> implementations in other places. Let's make it part of the standard bitops
> arsenal.
>
> Define the macro in a separate header file included from <linux/bitops.h>,
> to allow using it in very low-level header files that may not want to
> include all of <linux/bitops.h>.
>
> Signed-off-by: Petr Tesarik <ptesarik@suse.com>
> ---
>   MAINTAINERS             |  1 +
>   include/linux/bitops.h  |  1 +
>   include/linux/ffs_val.h | 21 +++++++++++++++++++++
>   3 files changed, 23 insertions(+)
>   create mode 100644 include/linux/ffs_val.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a0dd762f5648b..8f15c76a67ea2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4466,6 +4466,7 @@ F:	arch/*/lib/bitops.c
>   F:	include/asm-generic/bitops
>   F:	include/asm-generic/bitops.h
>   F:	include/linux/bitops.h
> +F:	include/linux/ffs_val.h
>   F:	lib/hweight.c
>   F:	lib/test_bitops.c
>   F:	tools/*/bitops*
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index ea7898cc59039..209f0c3e07b9e 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -4,6 +4,7 @@
>   
>   #include <asm/types.h>
>   #include <linux/bits.h>
> +#include <linux/ffs_val.h>
>   #include <linux/typecheck.h>
>   
>   #include <uapi/linux/kernel.h>
> diff --git a/include/linux/ffs_val.h b/include/linux/ffs_val.h
> new file mode 100644
> index 0000000000000..193ec86d2b53b
> --- /dev/null
> +++ b/include/linux/ffs_val.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_LINUX_FFS_VAL_H_
> +#define _ASM_LINUX_FFS_VAL_H_
> +
> +/**
> + * ffs_val - find the value of the first set bit
> + * @x: the value to search
> + *
> + * Unlike ffs(), which returns a bit position, ffs_val() returns the bit
> + * value itself.

This sentence was confusing me at first, because the individual bit's 
value is always '1'. Maybe say something more descriptive, such as 
'ffs_val returns the value resulting from that bit's position.'

> + *
> + * Returns:
> + * least significant non-zero bit, 0 if all bits are zero

Same here.

> + */
> +#define ffs_val(x)			\
> +({					\
> +	const typeof(x) val__ = (x);	\
> +	val__ & -val__;			\

Is this construct supposed to work with signed integers and/or negative 
numbers? I assume that two's complement can be expected nowadays, but 
for LONG_MIN it returns zero AFAICT. The documentation should mention 
these cases.

Best regards
Thomas

> +})
> +
> +#endif /* _ASM_LINUX_FFS_VAL_H_ */

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

On Mon, 12 Jan 2026 09:15:41 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Hi Petr
> 
> Am 09.01.26 um 17:37 schrieb Petr Tesarik:
> > Introduce a macro that can efficiently extract the least significant
> > non-zero bit from a value.
> >
> > Interestingly, this bit-twiddling trick is open-coded in some places, but
> > it also appears to be little known, leading to various inefficient
> > implementations in other places. Let's make it part of the standard bitops
> > arsenal.
> >
> > Define the macro in a separate header file included from <linux/bitops.h>,
> > to allow using it in very low-level header files that may not want to
> > include all of <linux/bitops.h>.
> >
> > Signed-off-by: Petr Tesarik <ptesarik@suse.com>
> > ---
> >   MAINTAINERS             |  1 +
> >   include/linux/bitops.h  |  1 +
> >   include/linux/ffs_val.h | 21 +++++++++++++++++++++
> >   3 files changed, 23 insertions(+)
> >   create mode 100644 include/linux/ffs_val.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a0dd762f5648b..8f15c76a67ea2 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -4466,6 +4466,7 @@ F:	arch/*/lib/bitops.c
> >   F:	include/asm-generic/bitops
> >   F:	include/asm-generic/bitops.h
> >   F:	include/linux/bitops.h
> > +F:	include/linux/ffs_val.h
> >   F:	lib/hweight.c
> >   F:	lib/test_bitops.c
> >   F:	tools/*/bitops*
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > index ea7898cc59039..209f0c3e07b9e 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -4,6 +4,7 @@
> >   
> >   #include <asm/types.h>
> >   #include <linux/bits.h>
> > +#include <linux/ffs_val.h>
> >   #include <linux/typecheck.h>
> >   
> >   #include <uapi/linux/kernel.h>
> > diff --git a/include/linux/ffs_val.h b/include/linux/ffs_val.h
> > new file mode 100644
> > index 0000000000000..193ec86d2b53b
> > --- /dev/null
> > +++ b/include/linux/ffs_val.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _ASM_LINUX_FFS_VAL_H_
> > +#define _ASM_LINUX_FFS_VAL_H_
> > +
> > +/**
> > + * ffs_val - find the value of the first set bit
> > + * @x: the value to search
> > + *
> > + * Unlike ffs(), which returns a bit position, ffs_val() returns the bit
> > + * value itself.  
> 
> This sentence was confusing me at first, because the individual bit's 
> value is always '1'. Maybe say something more descriptive, such as 
> 'ffs_val returns the value resulting from that bit's position.'

An example would clarify things massively.
Perhaps ffs_val(0x40100) is 0x100.

> > + *
> > + * Returns:
> > + * least significant non-zero bit, 0 if all bits are zero  
> 
> Same here.
> 
> > + */
> > +#define ffs_val(x)			\
> > +({					\
> > +	const typeof(x) val__ = (x);	\
> > +	val__ & -val__;			\  
> 
> Is this construct supposed to work with signed integers and/or negative 
> numbers? I assume that two's complement can be expected nowadays, but 
> for LONG_MIN it returns zero AFAICT. The documentation should mention 
> these cases.

Writing as 'x & (~x + 1)' makes it pretty obvious that it is always valid.
For two's compliment '-x == ~x + 1' and the compiler will do the transformation.
That expression would also be valid for one's compliment and sign overpunch
systems - but the result for negative values might be unexpected!

OTOH I'm pretty sure most modern C code assumes two's compliment binary
numbers (and NULL being the all-zero bit pattern).
You'd have grief trying to compile C for an ICL System-25 - decimal/BCD
numbers with sign overpunch and addresses binary_page * 10000 + BCD_offset.
(Still in use in the late 1980s.)

	David


> 
> Best regards
> Thomas
> 
> > +})
> > +
> > +#endif /* _ASM_LINUX_FFS_VAL_H_ */  
>

On Mon, 12 Jan 2026, David Laight wrote:

> OTOH I'm pretty sure most modern C code assumes two's compliment binary
> numbers (and NULL being the all-zero bit pattern).

 FWIW ISO C23 mandates the two's complement integer sign representation.  
Other sign representations are no longer allowed.

  Maciej

On Mon, 12 Jan 2026 09:15:41 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Hi Petr
> 
> Am 09.01.26 um 17:37 schrieb Petr Tesarik:
> > Introduce a macro that can efficiently extract the least significant
> > non-zero bit from a value.
> >
> > Interestingly, this bit-twiddling trick is open-coded in some places, but
> > it also appears to be little known, leading to various inefficient
> > implementations in other places. Let's make it part of the standard bitops
> > arsenal.
> >
> > Define the macro in a separate header file included from <linux/bitops.h>,
> > to allow using it in very low-level header files that may not want to
> > include all of <linux/bitops.h>.
> >
> > Signed-off-by: Petr Tesarik <ptesarik@suse.com>
> > ---
> >   MAINTAINERS             |  1 +
> >   include/linux/bitops.h  |  1 +
> >   include/linux/ffs_val.h | 21 +++++++++++++++++++++
> >   3 files changed, 23 insertions(+)
> >   create mode 100644 include/linux/ffs_val.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a0dd762f5648b..8f15c76a67ea2 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -4466,6 +4466,7 @@ F:	arch/*/lib/bitops.c
> >   F:	include/asm-generic/bitops
> >   F:	include/asm-generic/bitops.h
> >   F:	include/linux/bitops.h
> > +F:	include/linux/ffs_val.h
> >   F:	lib/hweight.c
> >   F:	lib/test_bitops.c
> >   F:	tools/*/bitops*
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > index ea7898cc59039..209f0c3e07b9e 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -4,6 +4,7 @@
> >   
> >   #include <asm/types.h>
> >   #include <linux/bits.h>
> > +#include <linux/ffs_val.h>
> >   #include <linux/typecheck.h>
> >   
> >   #include <uapi/linux/kernel.h>
> > diff --git a/include/linux/ffs_val.h b/include/linux/ffs_val.h
> > new file mode 100644
> > index 0000000000000..193ec86d2b53b
> > --- /dev/null
> > +++ b/include/linux/ffs_val.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _ASM_LINUX_FFS_VAL_H_
> > +#define _ASM_LINUX_FFS_VAL_H_
> > +
> > +/**
> > + * ffs_val - find the value of the first set bit
> > + * @x: the value to search
> > + *
> > + * Unlike ffs(), which returns a bit position, ffs_val() returns the bit
> > + * value itself.  
> 
> This sentence was confusing me at first, because the individual bit's 
> value is always '1'. Maybe say something more descriptive, such as 
> 'ffs_val returns the value resulting from that bit's position.'

I think I found the best wording only when composing the cover letter:
isolate the least significant set bit. I should have gone back and
adjusted the patches, too.

> > + *
> > + * Returns:
> > + * least significant non-zero bit, 0 if all bits are zero  
> 
> Same here.
> 
> > + */
> > +#define ffs_val(x)			\
> > +({					\
> > +	const typeof(x) val__ = (x);	\
> > +	val__ & -val__;			\  
> 
> Is this construct supposed to work with signed integers and/or negative 
> numbers? I assume that two's complement can be expected nowadays, but 
> for LONG_MIN it returns zero AFAICT. The documentation should mention 
> these cases.

It works fine for all numbers: -LONG_MIN is LONG_MIN, and that is
incidentally the number with only the least significant bit set.

But this is all moot. After reading some other replies, I'm not even
sure, this helper adds any value. I'm going to start over by converting
occurrences of x & -x with the most suitable existing helper, and see
if there's still some value then.

Last but not least, if there's no good self-explanatory name for this
operation, then I'm told that open-coding "x & -x" is in fact easier to
read. Then again, this is the opinion of people who coded back in the
1980s, and my general feeling is that bit operations are generally less
well understood by people born after 2000...

Glad to spark a bikeshedding flamewar, anyway. /s

Petr T

On Fri,  9 Jan 2026 17:37:56 +0100
Petr Tesarik <ptesarik@suse.com> wrote:

> Introduce a macro that can efficiently extract the least significant
> non-zero bit from a value.
> 
> Interestingly, this bit-twiddling trick is open-coded in some places, but
> it also appears to be little known, leading to various inefficient
> implementations in other places. Let's make it part of the standard bitops
> arsenal.

I'm not sure whether ffs_val(x) is actually more readable than an
open-coded (x & -x).
If you don't know what either means you have to look it up or work
it out.
The latter just requires a bit of thought, the former searching through
the source tree for the correct header and then believing the comment
or, again, working out what it does.

That said, I'm not objecting to adding it, but the churn of changing
existing code is probably not worth the effort.

I'd also define it as x & (~x + 1) - which makes it a lot more obvious
why it is correct, the compiler will convert it to a signed negate.

Also, as I pointed out earlier, many modern cpu have an instruction
for ffs(). While x & -x is usualy better than 1u << __ffs(x); the same
is not true for y * (x & -x) and y << __ffs(x).
In particular on Zen4/5 bsf (used for __ffs) has a latency of 1 but the
multiply has a latency of 3.
Intel mainstream x86 cpu all have latency 3 for both imul and bsf.

There should be #define definitions of is_power_of_2_or_zero() !(x + (x-1))
and is_power_of_2() (!x && is_power_of_2_or_zero(x)) in the same header.
But there is only an inline is_power_of_2(unsigned long) in log.h.

	David

On Fri, Jan 09, 2026 at 05:37:56PM +0100, Petr Tesarik wrote:
> Introduce a macro that can efficiently extract the least significant
> non-zero bit from a value.
> 
> Interestingly, this bit-twiddling trick is open-coded in some places, but
> it also appears to be little known, leading to various inefficient
> implementations in other places. Let's make it part of the standard bitops
> arsenal.
> 
> Define the macro in a separate header file included from <linux/bitops.h>,
> to allow using it in very low-level header files that may not want to
> include all of <linux/bitops.h>.

Nice catch. Thanks!

> 
> Signed-off-by: Petr Tesarik <ptesarik@suse.com>
> ---
>  MAINTAINERS             |  1 +
>  include/linux/bitops.h  |  1 +
>  include/linux/ffs_val.h | 21 +++++++++++++++++++++
>  3 files changed, 23 insertions(+)
>  create mode 100644 include/linux/ffs_val.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a0dd762f5648b..8f15c76a67ea2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4466,6 +4466,7 @@ F:	arch/*/lib/bitops.c
>  F:	include/asm-generic/bitops
>  F:	include/asm-generic/bitops.h
>  F:	include/linux/bitops.h
> +F:	include/linux/ffs_val.h

No need for a separate header. Just put int straight in bitops.h.

>  F:	lib/hweight.c
>  F:	lib/test_bitops.c
>  F:	tools/*/bitops*
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index ea7898cc59039..209f0c3e07b9e 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -4,6 +4,7 @@
>  
>  #include <asm/types.h>
>  #include <linux/bits.h>
> +#include <linux/ffs_val.h>
>  #include <linux/typecheck.h>
>  
>  #include <uapi/linux/kernel.h>
> diff --git a/include/linux/ffs_val.h b/include/linux/ffs_val.h
> new file mode 100644
> index 0000000000000..193ec86d2b53b
> --- /dev/null
> +++ b/include/linux/ffs_val.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_LINUX_FFS_VAL_H_
> +#define _ASM_LINUX_FFS_VAL_H_
> +
> +/**
> + * ffs_val - find the value of the first set bit

By definition, the value of 1st set bit is 1, just like any other set
bit. :)

> + * @x: the value to search
> + *
> + * Unlike ffs(), which returns a bit position, ffs_val() returns the bit
> + * value itself.
> + *
> + * Returns:
> + * least significant non-zero bit, 0 if all bits are zero
> + */
> +#define ffs_val(x)			\
> +({					\
> +	const typeof(x) val__ = (x);	\

const auto? Also, are you sure it works OK with unsigned types? No
warnings? Maybe add a test?

> +	val__ & -val__;			\
> +})

This macro returns in fact a mask containing LSB only, so I'd suggest
to choose a name like lsb_mask().

This is also a replacement of BIT(ffs()), GENMASK(ffs(), 0) constructions.
Can you check the kernel, and convert those patterns too? I found at least
one in drivers/clk/nxp/clk-lpc32xx.c:lpc32xx_clk_div_quirk().

Thanks,
Yury

> +
> +#endif /* _ASM_LINUX_FFS_VAL_H_ */
> -- 
> 2.52.0

On Fri, 9 Jan 2026 12:16:06 -0500
Yury Norov <ynorov@nvidia.com> wrote:

> On Fri, Jan 09, 2026 at 05:37:56PM +0100, Petr Tesarik wrote:
> > Introduce a macro that can efficiently extract the least significant
> > non-zero bit from a value.
> > 
> > Interestingly, this bit-twiddling trick is open-coded in some places, but
> > it also appears to be little known, leading to various inefficient
> > implementations in other places. Let's make it part of the standard bitops
> > arsenal.
> > 
> > Define the macro in a separate header file included from <linux/bitops.h>,
> > to allow using it in very low-level header files that may not want to
> > include all of <linux/bitops.h>.  
> 
> Nice catch. Thanks!

It's been on my TODO list for months, but my first attempt failed
because of a coccinelle bug, and then I never got to it again.

> > Signed-off-by: Petr Tesarik <ptesarik@suse.com>
> > ---
> >  MAINTAINERS             |  1 +
> >  include/linux/bitops.h  |  1 +
> >  include/linux/ffs_val.h | 21 +++++++++++++++++++++
> >  3 files changed, 23 insertions(+)
> >  create mode 100644 include/linux/ffs_val.h
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a0dd762f5648b..8f15c76a67ea2 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -4466,6 +4466,7 @@ F:	arch/*/lib/bitops.c
> >  F:	include/asm-generic/bitops
> >  F:	include/asm-generic/bitops.h
> >  F:	include/linux/bitops.h
> > +F:	include/linux/ffs_val.h  
> 
> No need for a separate header. Just put int straight in bitops.h.

Well, <linux/bitops.h> is a bit heavy, so I was afraid of spoiling
build times if I include it from <asm-generic/div64.h>, but if you say
it's fine, yes, why not, let's put it into bitops.h somewhere before
#include <asm/bitops.h>.

> >  F:	lib/hweight.c
> >  F:	lib/test_bitops.c
> >  F:	tools/*/bitops*
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > index ea7898cc59039..209f0c3e07b9e 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -4,6 +4,7 @@
> >  
> >  #include <asm/types.h>
> >  #include <linux/bits.h>
> > +#include <linux/ffs_val.h>
> >  #include <linux/typecheck.h>
> >  
> >  #include <uapi/linux/kernel.h>
> > diff --git a/include/linux/ffs_val.h b/include/linux/ffs_val.h
> > new file mode 100644
> > index 0000000000000..193ec86d2b53b
> > --- /dev/null
> > +++ b/include/linux/ffs_val.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _ASM_LINUX_FFS_VAL_H_
> > +#define _ASM_LINUX_FFS_VAL_H_
> > +
> > +/**
> > + * ffs_val - find the value of the first set bit  
> 
> By definition, the value of 1st set bit is 1, just like any other set
> bit. :)

I'm struggling with suitable wording. The trouble is that "find first
set bit" is generally understood as find the bit _position_. Maybe I
should say _isolate_ the bit, or something like that.

> > + * @x: the value to search
> > + *
> > + * Unlike ffs(), which returns a bit position, ffs_val() returns the bit
> > + * value itself.
> > + *
> > + * Returns:
> > + * least significant non-zero bit, 0 if all bits are zero
> > + */
> > +#define ffs_val(x)			\
> > +({					\
> > +	const typeof(x) val__ = (x);	\  
> 
> const auto? Also, are you sure it works OK with unsigned types? No
> warnings? Maybe add a test?

The "const auto" is good idea. Regarding unsigned types, indeed, the
result of applying unary minus to any non-zero value is out of bounds
of an unsigned type. However, the C standard has this much to say:
"C’s unsigned integer types are ‘‘modulo’’ in the LIA−1 sense in that
overflows or out-of-bounds results silently wrap."

Besides, this patch series does not change anything, it merely puts the
arithmetic inside a macro.

> > +	val__ & -val__;			\
> > +})  
> 
> This macro returns in fact a mask containing LSB only, so I'd suggest
> to choose a name like lsb_mask().

Mask is a terrible word, because it doesn't say if the masked bit is
set or clear. Even if I limit myself to the Linux kernel, it's used for
both in different contexts.

What about isolate_lsb()?

The only issue is that LSB may also refer to least-significant BYTE. :-(

> This is also a replacement of BIT(ffs()), GENMASK(ffs(), 0) constructions.
> Can you check the kernel, and convert those patterns too? I found at least
> one in drivers/clk/nxp/clk-lpc32xx.c:lpc32xx_clk_div_quirk().

Yes, there's a lot of places under drivers/ that can benefit from this
macro. I didn't want to spam everyone with this RFC, as we iron out the
details. I'm even unsure about the correct process to get such a change
into the kernel.

Thanks for the review and suggestions!

Petr T

On Fri, Jan 9, 2026, at 18:46, Petr Tesarik wrote:
> On Fri, 9 Jan 2026 12:16:06 -0500 Yury Norov <ynorov@nvidia.com> wrote:
>> > diff --git a/MAINTAINERS b/MAINTAINERS
>> > index a0dd762f5648b..8f15c76a67ea2 100644
>> > --- a/MAINTAINERS
>> > +++ b/MAINTAINERS
>> > @@ -4466,6 +4466,7 @@ F:	arch/*/lib/bitops.c
>> >  F:	include/asm-generic/bitops
>> >  F:	include/asm-generic/bitops.h
>> >  F:	include/linux/bitops.h
>> > +F:	include/linux/ffs_val.h  
>> 
>> No need for a separate header. Just put int straight in bitops.h.
>
> Well, <linux/bitops.h> is a bit heavy, so I was afraid of spoiling
> build times if I include it from <asm-generic/div64.h>, but if you say
> it's fine, yes, why not, let's put it into bitops.h somewhere before
> #include <asm/bitops.h>.

A single macro definition makes zero difference for build times.

What makes linux/bitops.h expensive is the 419kb of preprocessed
source code that result from the generated atomics in asm/bitops.h
that get included everywhere.

    Arnd

> > No need for a separate header. Just put int straight in bitops.h.
> 
> Well, <linux/bitops.h> is a bit heavy, so I was afraid of spoiling
> build times if I include it from <asm-generic/div64.h>, but if you say
> it's fine, yes, why not, let's put it into bitops.h somewhere before
> #include <asm/bitops.h>.

Unless you have strong performance numbers, let's keep it in the
bitops.h
 
> > > +/**
> > > + * ffs_val - find the value of the first set bit  
> > 
> > By definition, the value of 1st set bit is 1, just like any other set
> > bit. :)
> 
> I'm struggling with suitable wording. The trouble is that "find first
> set bit" is generally understood as find the bit _position_. Maybe I
> should say _isolate_ the bit, or something like that.
> 
> > > + * @x: the value to search
> > > + *
> > > + * Unlike ffs(), which returns a bit position, ffs_val() returns the bit
> > > + * value itself.
> > > + *
> > > + * Returns:
> > > + * least significant non-zero bit, 0 if all bits are zero
> > > + */
> > > +#define ffs_val(x)			\
> > > +({					\
> > > +	const typeof(x) val__ = (x);	\  
> > 
> > const auto? Also, are you sure it works OK with unsigned types? No
> > warnings? Maybe add a test?
> 
> The "const auto" is good idea. Regarding unsigned types, indeed, the
> result of applying unary minus to any non-zero value is out of bounds
> of an unsigned type. However, the C standard has this much to say:
> "C’s unsigned integer types are ‘‘modulo’’ in the LIA−1 sense in that
> overflows or out-of-bounds results silently wrap."
> 
> Besides, this patch series does not change anything, it merely puts the
> arithmetic inside a macro.
> 
> > > +	val__ & -val__;			\
> > > +})  
> > 
> > This macro returns in fact a mask containing LSB only, so I'd suggest
> > to choose a name like lsb_mask().
> 
> Mask is a terrible word, because it doesn't say if the masked bit is
> set or clear. Even if I limit myself to the Linux kernel, it's used for
> both in different contexts.
> 
> What about isolate_lsb()?

In bitops world, something_mask() has a very clear meaning. Consider
GENMASK(), BITMAP_FIRST_BYTE_MASK(), __BF_FIELD_CHECK_MASK(), and so
on.

lsb_mask(), or LSB_MASK() if you prefer, is just right.

> The only issue is that LSB may also refer to least-significant BYTE. :-(

It will never mean byte because it hosts in bitops.h, not byteops.h
 
> > This is also a replacement of BIT(ffs()), GENMASK(ffs(), 0) constructions.
> > Can you check the kernel, and convert those patterns too? I found at least
> > one in drivers/clk/nxp/clk-lpc32xx.c:lpc32xx_clk_div_quirk().
> 
> Yes, there's a lot of places under drivers/ that can benefit from this
> macro. I didn't want to spam everyone with this RFC, as we iron out the
> details. I'm even unsure about the correct process to get such a change
> into the kernel.

If you don't want to fix every driver, it's OK. But please keep core
kernel clean.

> 
> Thanks for the review and suggestions!
> 
> Petr T

On Fri, 9 Jan 2026 13:26:30 -0500
Yury Norov <ynorov@nvidia.com> wrote:

> > > No need for a separate header. Just put int straight in bitops.h.  
> > 
> > Well, <linux/bitops.h> is a bit heavy, so I was afraid of spoiling
> > build times if I include it from <asm-generic/div64.h>, but if you say
> > it's fine, yes, why not, let's put it into bitops.h somewhere before
> > #include <asm/bitops.h>.  
> 
> Unless you have strong performance numbers, let's keep it in the
> bitops.h

OK.

> > > > +/**
> > > > + * ffs_val - find the value of the first set bit    
> > > 
> > > By definition, the value of 1st set bit is 1, just like any other set
> > > bit. :)  
> > 
> > I'm struggling with suitable wording. The trouble is that "find first
> > set bit" is generally understood as find the bit _position_. Maybe I
> > should say _isolate_ the bit, or something like that.
> >   
> > > > + * @x: the value to search
> > > > + *
> > > > + * Unlike ffs(), which returns a bit position, ffs_val() returns the bit
> > > > + * value itself.
> > > > + *
> > > > + * Returns:
> > > > + * least significant non-zero bit, 0 if all bits are zero
> > > > + */
> > > > +#define ffs_val(x)			\
> > > > +({					\
> > > > +	const typeof(x) val__ = (x);	\    
> > > 
> > > const auto? Also, are you sure it works OK with unsigned types? No
> > > warnings? Maybe add a test?  
> > 
> > The "const auto" is good idea. Regarding unsigned types, indeed, the
> > result of applying unary minus to any non-zero value is out of bounds
> > of an unsigned type. However, the C standard has this much to say:
> > "C’s unsigned integer types are ‘‘modulo’’ in the LIA−1 sense in that
> > overflows or out-of-bounds results silently wrap."
> > 
> > Besides, this patch series does not change anything, it merely puts the
> > arithmetic inside a macro.
> >   
> > > > +	val__ & -val__;			\
> > > > +})    
> > > 
> > > This macro returns in fact a mask containing LSB only, so I'd suggest
> > > to choose a name like lsb_mask().  
> > 
> > Mask is a terrible word, because it doesn't say if the masked bit is
> > set or clear. Even if I limit myself to the Linux kernel, it's used for
> > both in different contexts.
> > 
> > What about isolate_lsb()?  
> 
> In bitops world, something_mask() has a very clear meaning. Consider
> GENMASK(), BITMAP_FIRST_BYTE_MASK(), __BF_FIELD_CHECK_MASK(), and so
> on.
> 
> lsb_mask(), or LSB_MASK() if you prefer, is just right.

I assume you mean BITMAP_FIRST_WORD_MASK(), not btrfs-specific
BITMAP_FIRST_BYTE_MASK(), defined in fs/btrfs/extent_io.h.

But then it nicely illustrate exactly what I mean:

  BIT_MASK(4) = 0x0000000000000010
  GENMASK(4, 0) = 0x000000000000001f
  BITMAP_FIRST_WORD_MASK(4) == 0xfffffffffffffff0

> > The only issue is that LSB may also refer to least-significant BYTE. :-(  
> 
> It will never mean byte because it hosts in bitops.h, not byteops.h

I'm afraid that identifiers from various sources end up next to each
other and they do not carry tags where each of them came from...

> > > This is also a replacement of BIT(ffs()), GENMASK(ffs(), 0) constructions.
> > > Can you check the kernel, and convert those patterns too? I found at least
> > > one in drivers/clk/nxp/clk-lpc32xx.c:lpc32xx_clk_div_quirk().  
> > 
> > Yes, there's a lot of places under drivers/ that can benefit from this
> > macro. I didn't want to spam everyone with this RFC, as we iron out the
> > details. I'm even unsure about the correct process to get such a change
> > into the kernel.  
> 
> If you don't want to fix every driver, it's OK. But please keep core
> kernel clean.

Yes, getting acks from all maintainers will take a lot of time even
then.

Again, thank you for all the tips!

Petr T

On Fri, Jan 09, 2026 at 08:27:11PM +0100, Petr Tesarik wrote:
> On Fri, 9 Jan 2026 13:26:30 -0500
> Yury Norov <ynorov@nvidia.com> wrote:

...

> > In bitops world, something_mask() has a very clear meaning. Consider
> > GENMASK(), BITMAP_FIRST_BYTE_MASK(), __BF_FIELD_CHECK_MASK(), and so
> > on.
> > 
> > lsb_mask(), or LSB_MASK() if you prefer, is just right.
> 
> I assume you mean BITMAP_FIRST_WORD_MASK(), not btrfs-specific
> BITMAP_FIRST_BYTE_MASK(), defined in fs/btrfs/extent_io.h.
> 
> But then it nicely illustrate exactly what I mean:

They all have a consistent meaning:

>   BIT_MASK(4) = 0x0000000000000010

A mask constructed from bit #4.

>   GENMASK(4, 0) = 0x000000000000001f

A mask constructed from bits #0..4.

>   BITMAP_FIRST_WORD_MASK(4) == 0xfffffffffffffff0

A mask constructed from all bits starting #4.

Don't see any misalignment here.
 
> > > The only issue is that LSB may also refer to least-significant BYTE. :-(  
> > 
> > It will never mean byte because it hosts in bitops.h, not byteops.h
> 
> I'm afraid that identifiers from various sources end up next to each
> other and they do not carry tags where each of them came from...

You can name it lsb_bitmask() if you want, either way works. But to me
shorter is better.