more potential janitor work: simplifying test for power of 2

more potential janitor work: simplifying test for power of 2

[Robert P. J. Day]

  As another example of a cleanup script I wrote lo those many years
ago, this one checked if someone was manually testing if a value was a
power of two (do not make fun of my coding):

#!/bin/sh

DIR=${1-*}

echo "PATTERN:     x & (x - 1):\n"
grep -Ern "([^\(\)]+) ?\& ?\(\1 ?- ?1\)" ${DIR}
echo "PATTERN:     x & ((x) - 1):\n"
grep -Ern "([^\(\)]+) ?\& ?\(\(\1\) ?- ?1\)" ${DIR}
echo "PATTERN:     (x) & (x - 1):\n"
grep -Ern "\(([^\(\)]+)\) ?\& ?\(\1 ?- ?1\)" ${DIR}
echo "PATTERN:     (x) & ((x) - 1):\n"
grep -Ern "\(([^\(\)]+)\) ?\& ?\(\(\1\) ?- ?1\)" ${DIR}

  As you can see, the typical hacky check was to test the value of
some variant of "n & (n - 1)" to see if n was in fact a power of 2.
Note that, when run from the top of the kernel source tree, you can
focus the search on any subdirectory.  There is a ton of this sort of
thing scattered throughout the kernel source; for example:

$ test_for_power_of_2.sh drivers/net/ethernet
PATTERN:     x & (x - 1):

drivers/net/ethernet/huawei/hinic/hinic_hw_eqs.c:756:	if (eq->num_elem_in_pg & (eq->num_elem_in_pg - 1)) {
drivers/net/ethernet/huawei/hinic/hinic_hw_wq.c:456:	if (num_q_pages & (num_q_pages - 1)) {
drivers/net/ethernet/huawei/hinic/hinic_hw_wq.c:523:	if (q_depth & (q_depth - 1)) {
drivers/net/ethernet/huawei/hinic/hinic_hw_wq.c:620:	if (q_depth & (q_depth - 1)) {
drivers/net/ethernet/huawei/hinic/hinic_hw_api_cmd.c:872:	if (attr->num_cells & (attr->num_cells - 1)) {
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/definer.c:691:	if (flags & (flags - 1))
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/definer.c:697:	if (flags & (flags - 1))
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/definer.c:702:	if (flags & (flags - 1))
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/definer.c:709:	if (flags & (flags - 1))
drivers/net/ethernet/mellanox/mlx5/core/steering/sws/dr_rule.c:680:		byte_mask = byte_mask & (byte_mask - 1);
drivers/net/ethernet/mellanox/mlx5/core/lib/dm.c:163:	if (!length || (length & (length - 1)) ||
drivers/net/ethernet/sfc/siena/siena_sriov.c:505:	return ((buf_count & (buf_count - 1)) || buf_count > max_buf_count);
drivers/net/ethernet/sfc/siena/siena_sriov.c:956:	BUG_ON(vf->evq0_count & (vf->evq0_count - 1));
drivers/net/ethernet/broadcom/bnxt/bnxt.c:4736:	while (pages & (pages - 1))
drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c:1230:#define POWER_OF_2(x)	((0 != x) && (0 == (x & (x-1))))
drivers/net/ethernet/synopsys/dwc-xlgmac-common.c:110:	if (pdata->tx_desc_count & (pdata->tx_desc_count - 1)) {
drivers/net/ethernet/synopsys/dwc-xlgmac-common.c:118:	if (pdata->rx_desc_count & (pdata->rx_desc_count - 1)) {
drivers/net/ethernet/freescale/fman/fman_keygen.c:700:	if (hash_size == 0 || (hash_size & (hash_size - 1)) != 0) {
drivers/net/ethernet/chelsio/cxgb4/sge.c:5094:	    (fl_large_pg & (fl_large_pg-1)) != 0) {
drivers/net/ethernet/chelsio/cxgb4vf/sge.c:2649:	    (fl_large_pg & (fl_large_pg - 1)) != 0) {
drivers/net/ethernet/chelsio/cxgb/cxgb2.c:657:		if (advertising & (advertising - 1))
drivers/net/ethernet/emulex/benet/be.h:136:	BUG_ON(limit & (limit - 1));
drivers/net/ethernet/intel/igc/igc_leds.c:156:	if (flags & (flags - 1))
drivers/net/ethernet/intel/igc/igc_ethtool.c:1286:	    (rule->filter.match_flags & (rule->filter.match_flags - 1)))
drivers/net/ethernet/intel/fm10k/fm10k_tlv.c:203:	if (!msg || !len || len > 8 || (len & (len - 1)))
drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c:1917:	if ((table->mem_table.depth & (table->mem_table.depth - 1)) != 0) {
PATTERN:     x & ((x) - 1):

PATTERN:     (x) & (x - 1):

PATTERN:     (x) & ((x) - 1):

  This can be simplified using the kernel header file
include/linux/log2.h, which defines the test using an exclusive-OR
operation:

/**
 * is_power_of_2() - check if a value is a power of two
 * @n: the value to check
 *
 * Determine whether some value is a power of two, where zero is
 * *not* considered a power of two.
 * Return: true if @n is a power of 2, otherwise false.
 */
static __always_inline __attribute__((const))
bool is_power_of_2(unsigned long n)
{
	return n - 1 < (n ^ (n - 1));
}

  Lots of simplification there if one wants some janitor work; I don't
see a coccinelle test for this.

rday

Re: more potential janitor work: simplifying test for power of 2

[Linus Probert]

On Tue Mar 31, 2026 at 1:49 PM CEST, Robert P. J. Day wrote:
>

[...]

>   Lots of simplification there if one wants some janitor work; I don't
> see a coccinelle test for this.

I'll definitely take a look.

I've only ever used coccinelle, never written any scritps for it. But
for these kinds of things, is it valuable to introduce such tests?

Or are they involved in any form of automated testing? I assume we don't
want to wile up an angry mob of maintainers.

Br,
Linus

Re: more potential janitor work: simplifying test for power of 2

[Linus Probert]

I took a closer look at this. Many of the occurences here are actually
used to check if a binary flag has more then one bit set. This is quite
a common pattern.

Eg. 'if (a & (a - 1))' would pass if a = 0b100, not if a = 0b110. Since:
	0b100 - 0b001 = 0b011 -> (0b100 & 0b011) => false
	0b110 - 0b001 = 0b101 -> (0b100 & 0b101) => true

The power of two check would 'normally' use the negation:
	'if (!(a & (a - 1)))'

Just a heads up so anyone else reading this doesn't go blindly swapping
out this pattern for 'is_power_of_2()'.

You need to apply some thinking. No 'sed' work.

Br,
Linus

Re: more potential janitor work: simplifying test for power of 2

[Julia Lawall]

On Thu, 2 Apr 2026, Linus Probert wrote:

> I took a closer look at this. Many of the occurences here are actually
> used to check if a binary flag has more then one bit set. This is quite
> a common pattern.
>
> Eg. 'if (a & (a - 1))' would pass if a = 0b100, not if a = 0b110. Since:
> 	0b100 - 0b001 = 0b011 -> (0b100 & 0b011) => false
> 	0b110 - 0b001 = 0b101 -> (0b100 & 0b101) => true

If it's common, maybe there should be a function for it that properly
reflects the intended behavior.

julia

>
> The power of two check would 'normally' use the negation:
> 	'if (!(a & (a - 1)))'
>
> Just a heads up so anyone else reading this doesn't go blindly swapping
> out this pattern for 'is_power_of_2()'.
>
> You need to apply some thinking. No 'sed' work.
>
> Br,
> Linus
>
>

Re: more potential janitor work: simplifying test for power of 2

[Linus Probert]

On Thu Apr 2, 2026 at 12:07 PM CEST, Julia Lawall wrote:
>
>
> On Thu, 2 Apr 2026, Linus Probert wrote:
>
>> I took a closer look at this. Many of the occurences here are actually
>> used to check if a binary flag has more then one bit set. This is quite
>> a common pattern.
>>
>> Eg. 'if (a & (a - 1))' would pass if a = 0b100, not if a = 0b110. Since:
>> 	0b100 - 0b001 = 0b011 -> (0b100 & 0b011) => false
>> 	0b110 - 0b001 = 0b101 -> (0b100 & 0b101) => true
>
> If it's common, maybe there should be a function for it that properly
> reflects the intended behavior.
>

Not my decision but I would guess that the general consensus is that we
don't want to obscure bit manipulation in helper functions.

These patterns are often considered base knowledge and since it doesn't take
up any space a "helper" function only obscures what's happening. In
particular from a review perspective.

So unless there exists a function for this pattern already I don't think
it's something that merits replacing.

That's my 2c on that subject.

That said, there are certainly places where swapping in the
is_power_of_2() function is applicable. It uses a fancy bit trick which
wasn't as obvious to me. Generally the right side of the expression
below is the one I would use and no helper function existed.

	(n - 1 < (n ^ (n - 1))) == (n && !(n & (n - 1)))

So if you are looking into doing some janitor work in this area you
should keep in mind that, without that zero guard 'n &&' swapping the
function might be altering the logic.

So, a heads up is all.

Br,
Linus

Re: more potential janitor work: simplifying test for power of 2

[Robert P. J. Day]

On Thu, 2 Apr 2026, Linus Probert wrote:

> On Thu Apr 2, 2026 at 12:07 PM CEST, Julia Lawall wrote:
> >
> >
> > On Thu, 2 Apr 2026, Linus Probert wrote:
> >
> >> I took a closer look at this. Many of the occurences here are actually
> >> used to check if a binary flag has more then one bit set. This is quite
> >> a common pattern.
> >>
> >> Eg. 'if (a & (a - 1))' would pass if a = 0b100, not if a = 0b110. Since:
> >> 	0b100 - 0b001 = 0b011 -> (0b100 & 0b011) => false
> >> 	0b110 - 0b001 = 0b101 -> (0b100 & 0b101) => true
> >
> > If it's common, maybe there should be a function for it that properly
> > reflects the intended behavior.
> >
>
> Not my decision but I would guess that the general consensus is that we
> don't want to obscure bit manipulation in helper functions.
>
> These patterns are often considered base knowledge and since it doesn't take
> up any space a "helper" function only obscures what's happening. In
> particular from a review perspective.
>
> So unless there exists a function for this pattern already I don't think
> it's something that merits replacing.
>
> That's my 2c on that subject.
>
> That said, there are certainly places where swapping in the
> is_power_of_2() function is applicable. It uses a fancy bit trick which
> wasn't as obvious to me. Generally the right side of the expression
> below is the one I would use and no helper function existed.
>
> 	(n - 1 < (n ^ (n - 1))) == (n && !(n & (n - 1)))
>
> So if you are looking into doing some janitor work in this area you
> should keep in mind that, without that zero guard 'n &&' swapping the
> function might be altering the logic.
>
> So, a heads up is all.

  Not to toot my own horn but if you go back through history, I was
the one who kicked off the introduction of the first "is power of 2"
helper function many, many years ago:


  commit 63c2f782e8f6aafbc11b14b2cb291b3dc9fc217d
  Author: Robert P. J. Day <rpjday@mindspring.com>
  Date:   Tue Jan 30 06:06:00 2007 -0500

    [POWERPC] Add "is_power_of_2" checking to log2.h.

    Add the inline function "is_power_of_2()" to log2.h, where the value
    zero is *not* considered to be a power of two.

    Signed-off-by: Robert P. J. Day <rpjday@mindspring.com>
    Acked-by: Kumar Gala <galak@kernel.crashing.org>
    Signed-off-by: Paul Mackerras <paulus@samba.org>


  And, no, a lot of that content was not immediately obvious as a
candidate for simplification; when I wrote my regex searching scripts,
I deliberately made them overly general *knowing* I would get false
positives, then I manually checked whether they should be simplified.
It never occurred to me that some of those checks were actually asking
whether more than one bit flag was set, which semantically is asking a
different question.

  Anyway, people were asking about janitorial work so I threw out a
couple of ideas. Whether they're worth pursing ... not my call.

rday

Re: more potential janitor work: simplifying test for power of 2

[Linus Probert]

On Thu Apr 2, 2026 at 2:40 PM CEST, Robert P. J. Day wrote:
> On Thu, 2 Apr 2026, Linus Probert wrote:
>
>> On Thu Apr 2, 2026 at 12:07 PM CEST, Julia Lawall wrote:
>> >
>> >
>> > On Thu, 2 Apr 2026, Linus Probert wrote:
>> >

[...]

>
>   Not to toot my own horn but if you go back through history, I was
> the one who kicked off the introduction of the first "is power of 2"
> helper function many, many years ago:

Totally worth a toot, it's a neat piece of bit operation.

[...]

>   And, no, a lot of that content was not immediately obvious as a
> candidate for simplification; when I wrote my regex searching scripts,
> I deliberately made them overly general *knowing* I would get false
> positives, then I manually checked whether they should be simplified.
> It never occurred to me that some of those checks were actually asking
> whether more than one bit flag was set, which semantically is asking a
> different question.

Throwing a wide net makes sense. I first noted this alternate 'question'
pattern after some scrolling. Didn't consider that case at first.

Just wanted to share that finding since others might have picked up on
this opportunity to contribute and perhaps not noticed the "semantic
difference".

>   Anyway, people were asking about janitorial work so I threw out a
> couple of ideas. Whether they're worth pursing ... not my call.

I was asking and appreciate the suggestions. If you have more stuff
knocking about I'm certain it will be appreciated by some of us
neophytes.

I'm learning that the hard part isn't immediately the code change. The
whole email patch cycle and high standards on commit message subject and
content takes some getting used to.

I doubt the kernel project is a large contributor to
https://latenightcommits.com/ :)

Br,
Linus

Re: more potential janitor work: simplifying test for power of 2

[Robert P. J. Day]

On Thu, 2 Apr 2026, Linus Probert wrote:

... snip ...

> I was asking and appreciate the suggestions. If you have more stuff
> knocking about I'm certain it will be appreciated by some of us
> neophytes.

  IMHO, i'll repeat that simplifying code that manually determines the
length of an array could be simplified based on
include/linux/array_size.h:

/**
 * ARRAY_SIZE - get the number of elements in array @arr
 * @arr: array to be sized
 */
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))

/**
 * ARRAY_END - get a pointer to one past the last element in array @arr
 * @arr: array
 */
#define ARRAY_END(arr)  (&(arr)[ARRAY_SIZE(arr)])


  there's *tons* of that all over the place.

rday