LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] Add "is_power_of_2" checking to log2.h.
@ 2007-01-30 11:06 Robert P. J. Day
  2007-01-30 12:25 ` Nick Piggin
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Robert P. J. Day @ 2007-01-30 11:06 UTC (permalink / raw)
  To: Linux kernel mailing list; +Cc: Andrew Morton, Paul Mackerras, dhowells, galak


  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>

---

  while people are coming up with more and more clever ways to do
rounding, we can at least add the check for power-of-2 now, so we can
start the cleanup.


 arch/powerpc/mm/pgtable_32.c  |    5 +----
 arch/ppc/mm/pgtable.c         |    5 +----
 arch/ppc/syslib/ppc85xx_rio.c |    2 --
 drivers/net/gianfar_ethtool.c |    2 --
 include/linux/log2.h          |   11 +++++++++++
 5 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 1891dbe..bd02272 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -294,11 +294,8 @@ void __init mapin_ram(void)
 	}
 }

-/* is x a power of 2? */
-#define is_power_of_2(x)	((x) != 0 && (((x) & ((x) - 1)) == 0))
-
 /* is x a power of 4? */
-#define is_power_of_4(x)	((x) != 0 && (((x) & (x-1)) == 0) && (ffs(x) & 1))
+#define is_power_of_4(x)	is_power_of_2(x) && (ffs(x) & 1))

 /*
  * Set up a mapping for a block of I/O.
diff --git a/arch/ppc/mm/pgtable.c b/arch/ppc/mm/pgtable.c
index 354a940..82b06a1 100644
--- a/arch/ppc/mm/pgtable.c
+++ b/arch/ppc/mm/pgtable.c
@@ -313,11 +313,8 @@ void __init mapin_ram(void)
 	}
 }

-/* is x a power of 2? */
-#define is_power_of_2(x)	((x) != 0 && (((x) & ((x) - 1)) == 0))
-
 /* is x a power of 4? */
-#define is_power_of_4(x)	((x) != 0 && (((x) & (x-1)) == 0) && (ffs(x) & 1))
+#define is_power_of_4(x)	is_power_of_2(x) && (ffs(x) & 1))

 /*
  * Set up a mapping for a block of I/O.
diff --git a/arch/ppc/syslib/ppc85xx_rio.c b/arch/ppc/syslib/ppc85xx_rio.c
index 05b0e94..2b09780 100644
--- a/arch/ppc/syslib/ppc85xx_rio.c
+++ b/arch/ppc/syslib/ppc85xx_rio.c
@@ -59,8 +59,6 @@
 #define DBELL_TID(x)		(*(u8 *)(x + DOORBELL_TID_OFFSET))
 #define DBELL_INF(x)		(*(u16 *)(x + DOORBELL_INFO_OFFSET))

-#define is_power_of_2(x)	(((x) & ((x) - 1)) == 0)
-
 struct rio_atmu_regs {
 	u32 rowtar;
 	u32 pad1;
diff --git a/drivers/net/gianfar_ethtool.c b/drivers/net/gianfar_ethtool.c
index 6d71bea..0d6943d 100644
--- a/drivers/net/gianfar_ethtool.c
+++ b/drivers/net/gianfar_ethtool.c
@@ -42,8 +42,6 @@

 #include "gianfar.h"

-#define is_power_of_2(x)        ((x) != 0 && (((x) & ((x) - 1)) == 0))
-
 extern void gfar_start(struct net_device *dev);
 extern int gfar_clean_rx_ring(struct net_device *dev, int rx_work_limit);

diff --git a/include/linux/log2.h b/include/linux/log2.h
index d02e1a5..99922be 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -44,6 +44,17 @@ int __ilog2_u64(u64 n)
 #endif

 /*
+ *  Determine whether some value is a power of two, where zero is
+ * *not* considered a power of two.
+ */
+
+static inline __attribute__((const))
+bool is_power_of_2(unsigned long n)
+{
+	return (n != 0 && ((n & (n - 1)) == 0));
+}
+
+/*
  * round up to nearest power of two
  */
 static inline __attribute__((const))
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://www.fsdev.dreamhosters.com/wiki/index.php?title=Main_Page
========================================================================

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 11:06 [PATCH] Add "is_power_of_2" checking to log2.h Robert P. J. Day
@ 2007-01-30 12:25 ` Nick Piggin
  2007-01-30 12:56   ` Robert P. J. Day
  2007-02-01 10:41 ` David Howells
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 21+ messages in thread
From: Nick Piggin @ 2007-01-30 12:25 UTC (permalink / raw)
  To: Robert P. J. Day
  Cc: Linux kernel mailing list, Andrew Morton, Paul Mackerras,
	dhowells, galak

Robert P. J. Day wrote:
>   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>
> 
>  /*
> + *  Determine whether some value is a power of two, where zero is
> + * *not* considered a power of two.
> + */

Why the qualifier? Zero *is* not a power of 2, is it?

> +
> +static inline __attribute__((const))
> +bool is_power_of_2(unsigned long n)
> +{
> +	return (n != 0 && ((n & (n - 1)) == 0));
> +}
> +
> +/*
>   * round up to nearest power of two
>   */
>  static inline __attribute__((const))


-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 12:25 ` Nick Piggin
@ 2007-01-30 12:56   ` Robert P. J. Day
  2007-01-30 14:12     ` Jan Engelhardt
  2007-01-31  1:21     ` Nick Piggin
  0 siblings, 2 replies; 21+ messages in thread
From: Robert P. J. Day @ 2007-01-30 12:56 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Linux kernel mailing list, Andrew Morton, Paul Mackerras,
	dhowells, galak

On Tue, 30 Jan 2007, Nick Piggin wrote:

> Robert P. J. Day wrote:
> >   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>
> >
> >  /*
> > + *  Determine whether some value is a power of two, where zero is
> > + * *not* considered a power of two.
> > + */
>
> Why the qualifier? Zero *is* not a power of 2, is it?

no, but it bears repeating since some developers might think it *is*.
if you peruse the current kernel code, you'll find some tests of the
simpler form:

	((n & (n - 1)) == 0))

which is clearly testing for "power of twoness" but which will return
true for a value of zero.  that's wrong, and it's why it's emphasized
in the comment.

rday

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://www.fsdev.dreamhosters.com/wiki/index.php?title=Main_Page
========================================================================

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 12:56   ` Robert P. J. Day
@ 2007-01-30 14:12     ` Jan Engelhardt
  2007-01-30 14:45       ` Robert P. J. Day
  2007-01-31 10:13       ` Vegard Nossum
  2007-01-31  1:21     ` Nick Piggin
  1 sibling, 2 replies; 21+ messages in thread
From: Jan Engelhardt @ 2007-01-30 14:12 UTC (permalink / raw)
  To: Robert P. J. Day
  Cc: Nick Piggin, Linux kernel mailing list, Andrew Morton,
	Paul Mackerras, dhowells, galak


>> Why the qualifier? Zero *is* not a power of 2, is it?

No, it is not:

	In[1]:= Solve[2^n == 0, n]

	Out[1]= {}

So says Mathematica5.



Jan
-- 
ft: http://freshmeat.net/p/chaostables/

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 14:12     ` Jan Engelhardt
@ 2007-01-30 14:45       ` Robert P. J. Day
  2007-01-30 15:50         ` Jan Engelhardt
  2007-06-11 23:11         ` H. Peter Anvin
  2007-01-31 10:13       ` Vegard Nossum
  1 sibling, 2 replies; 21+ messages in thread
From: Robert P. J. Day @ 2007-01-30 14:45 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Nick Piggin, Linux kernel mailing list, Andrew Morton,
	Paul Mackerras, dhowells, galak

On Tue, 30 Jan 2007, Jan Engelhardt wrote:

>
> >> Why the qualifier? Zero *is* not a power of 2, is it?
>
> No, it is not:
>
> 	In[1]:= Solve[2^n == 0, n]
>
> 	Out[1]= {}
>
> So says Mathematica5.

oooookay, that's kind of like taking a sandblaster to a soup cracker.

seriously, though, there is the potential of breaking something with
this change since you can see how there is some inconsistency in how
it's done *now* just for powerpc which, in some places, defines its
own versions of this:

./arch/ppc/mm/pgtable.c:
	#define is_power_of_2(x) ((x) != 0 && (((x) & ((x) - 1)) == 0))
./arch/ppc/syslib/ppc85xx_rio.c:
	#define is_power_of_2(x) (((x) & ((x) - 1)) == 0)
./arch/powerpc/mm/pgtable_32.c:
	#define is_power_of_2(x) ((x) != 0 && (((x) & ((x) - 1)) == 0))

note how the first and third macros *won't* consider zero a power of
two, while the second one *will*.  hence the need for a single
standard for all of this, just to play it safe.

rday

p.s.  in case you missed it, that was a not-so-subtle plea to please
just apply this patch so we can move on.

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://www.fsdev.dreamhosters.com/wiki/index.php?title=Main_Page
========================================================================

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 14:45       ` Robert P. J. Day
@ 2007-01-30 15:50         ` Jan Engelhardt
  2007-01-30 16:00           ` Robert P. J. Day
  2007-06-11 23:11         ` H. Peter Anvin
  1 sibling, 1 reply; 21+ messages in thread
From: Jan Engelhardt @ 2007-01-30 15:50 UTC (permalink / raw)
  To: Robert P. J. Day
  Cc: Nick Piggin, Linux kernel mailing list, Andrew Morton,
	Paul Mackerras, dhowells, galak


On Jan 30 2007 09:45, Robert P. J. Day wrote:
>On Tue, 30 Jan 2007, Jan Engelhardt wrote:
>>
>> >> Why the qualifier? Zero *is* not a power of 2, is it?
>>
>> No, it is not:
>>
>> 	In[1]:= Solve[2^n == 0, n]
>>
>> 	Out[1]= {}
>>
>> So says Mathematica5.
>
>oooookay, that's kind of like taking a sandblaster to a soup cracker.

Hehe. Well, there is a non-representable solution:

	In[2]:= 2^-Infinity
	Out[2]= 0


>seriously, though, there is the potential of breaking something with
>this change since you can see how there is some inconsistency in how
>it's done *now* just for powerpc which, in some places, defines its
>own versions of this:
>
>./arch/ppc/mm/pgtable.c:
>	#define is_power_of_2(x) ((x) != 0 && (((x) & ((x) - 1)) == 0))
>./arch/ppc/syslib/ppc85xx_rio.c:
>	#define is_power_of_2(x) (((x) & ((x) - 1)) == 0)
>./arch/powerpc/mm/pgtable_32.c:
>	#define is_power_of_2(x) ((x) != 0 && (((x) & ((x) - 1)) == 0))
>
>note how the first and third macros *won't* consider zero a power of
>two, while the second one *will*.  hence the need for a single
>standard for all of this, just to play it safe.

Hmpf. Perhaps a second macro "is_intdivisible_by_power_of_2" or so could
catch the "am I zero or 2^n" question.


Jan
-- 
ft: http://freshmeat.net/p/chaostables/

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 15:50         ` Jan Engelhardt
@ 2007-01-30 16:00           ` Robert P. J. Day
  0 siblings, 0 replies; 21+ messages in thread
From: Robert P. J. Day @ 2007-01-30 16:00 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Nick Piggin, Linux kernel mailing list, Andrew Morton,
	Paul Mackerras, dhowells, galak

On Tue, 30 Jan 2007, Jan Engelhardt wrote:

>
> On Jan 30 2007 09:45, Robert P. J. Day wrote:

> >seriously, though, there is the potential of breaking something with
> >this change since you can see how there is some inconsistency in how
> >it's done *now* just for powerpc which, in some places, defines its
> >own versions of this:
> >
> >./arch/ppc/mm/pgtable.c:
> >	#define is_power_of_2(x) ((x) != 0 && (((x) & ((x) - 1)) == 0))
> >./arch/ppc/syslib/ppc85xx_rio.c:
> >	#define is_power_of_2(x) (((x) & ((x) - 1)) == 0)
> >./arch/powerpc/mm/pgtable_32.c:
> >	#define is_power_of_2(x) ((x) != 0 && (((x) & ((x) - 1)) == 0))
> >
> >note how the first and third macros *won't* consider zero a power of
> >two, while the second one *will*.  hence the need for a single
> >standard for all of this, just to play it safe.
>
> Hmpf. Perhaps a second macro "is_intdivisible_by_power_of_2" or so could
> catch the "am I zero or 2^n" question.

no.  if that's really what the programmer wants, they can code it that
way explicitly.  let's not make this overly obscure.

rday

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 12:56   ` Robert P. J. Day
  2007-01-30 14:12     ` Jan Engelhardt
@ 2007-01-31  1:21     ` Nick Piggin
  2007-01-31  7:20       ` Robert P. J. Day
  1 sibling, 1 reply; 21+ messages in thread
From: Nick Piggin @ 2007-01-31  1:21 UTC (permalink / raw)
  To: Robert P. J. Day
  Cc: Linux kernel mailing list, Andrew Morton, Paul Mackerras,
	dhowells, galak

Robert P. J. Day wrote:
> On Tue, 30 Jan 2007, Nick Piggin wrote:
> 
> 
>>Robert P. J. Day wrote:
>>
>>>  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>
>>>
>>> /*
>>>+ *  Determine whether some value is a power of two, where zero is
>>>+ * *not* considered a power of two.
>>>+ */
>>
>>Why the qualifier? Zero *is* not a power of 2, is it?
> 
> 
> no, but it bears repeating since some developers might think it *is*.
> if you peruse the current kernel code, you'll find some tests of the
> simpler form:
> 
> 	((n & (n - 1)) == 0))
> 
> which is clearly testing for "power of twoness" but which will return
> true for a value of zero.  that's wrong, and it's why it's emphasized
> in the comment.

I would have thought you'd comment the broken ones, but that's just me.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-31  1:21     ` Nick Piggin
@ 2007-01-31  7:20       ` Robert P. J. Day
  0 siblings, 0 replies; 21+ messages in thread
From: Robert P. J. Day @ 2007-01-31  7:20 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Linux kernel mailing list, Andrew Morton, Paul Mackerras,
	dhowells, galak

On Wed, 31 Jan 2007, Nick Piggin wrote:

> Robert P. J. Day wrote:
> > On Tue, 30 Jan 2007, Nick Piggin wrote:
> >
> >
> > > Robert P. J. Day wrote:
> > >
> > > >  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>
> > > >
> > > > /*
> > > > + *  Determine whether some value is a power of two, where zero is
> > > > + * *not* considered a power of two.
> > > > + */
> > >
> > > Why the qualifier? Zero *is* not a power of 2, is it?
> >
> >
> > no, but it bears repeating since some developers might think it *is*.
> > if you peruse the current kernel code, you'll find some tests of the
> > simpler form:
> >
> > 	((n & (n - 1)) == 0))
> >
> > which is clearly testing for "power of twoness" but which will
> > return true for a value of zero.  that's wrong, and it's why it's
> > emphasized in the comment.
>
> I would have thought you'd comment the broken ones, but that's just
> me.

  good point, so let's just sum up here.  (man, it's hard to believe
that something this simple could drag on so long.  i feel like i'm
discussing free device driver development or something. :-)

  the new is_power_of_2() macro is defined as:

  (n != 0 && ((n & (n - 1)) == 0))

which (correctly, IMHO) does *not* identify zero as a power of two.
if someone truly wants *that* test, they can write it themselves:

  if (x == 0 || is_power_of_2(x))

  this means that, if someone wants to start rewriting those tests in
the source tree, every time they run across an apparent "power of two"
test of the simpler form:

  (n & (n - 1))

they have to ask themselves, "ok, did this coder mean to include zero
or not?"  in some cases, it's probably not going to be obvious.
(maybe the maintainers could do a quick check themselves and make the
substitution 'cuz, once the kernel janitors get ahold of this, you
never know what hilarity will ensue. :-)

  as far as the patch itself i submitted is concerned, the *only*
place that changed the existing semantics was here:

=================================================
--- a/arch/ppc/syslib/ppc85xx_rio.c
+++ b/arch/ppc/syslib/ppc85xx_rio.c
@@ -59,8 +59,6 @@
 #define DBELL_TID(x)           (*(u8 *)(x + DOORBELL_TID_OFFSET))
 #define DBELL_INF(x)           (*(u16 *)(x + DOORBELL_INFO_OFFSET))

-#define is_power_of_2(x)       (((x) & ((x) - 1)) == 0)
-
 struct rio_atmu_regs {
        u32 rowtar;
        u32 pad1;
=================================================

so if the powerpc people are ok with that, then the patch itself
should be fine, and it's only the upcoming substitutions in the source
tree that will have to be checked carefully, one by one.

rday

--

========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://www.fsdev.dreamhosters.com/wiki/index.php?title=Main_Page
========================================================================

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 14:12     ` Jan Engelhardt
  2007-01-30 14:45       ` Robert P. J. Day
@ 2007-01-31 10:13       ` Vegard Nossum
  2007-01-31 10:47         ` Robert P. J. Day
  1 sibling, 1 reply; 21+ messages in thread
From: Vegard Nossum @ 2007-01-31 10:13 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Robert P. J. Day, Nick Piggin, Linux kernel mailing list,
	Andrew Morton, Paul Mackerras, dhowells, galak

On Tue, January 30, 2007 3:12 pm, Jan Engelhardt wrote:
>
>>> Why the qualifier? Zero *is* not a power of 2, is it?
>
> No, it is not:
>
> 	In[1]:= Solve[2^n == 0, n]
>
> 	Out[1]= {}
>
> So says Mathematica5.

As a side note, I would just like to point out that Mathematica does not
deal with modular arithmetic by default (which programmers very much do).
In fact, in modular arithmetic, zero IS a power of two.

2^n = 0 (mod 2^n)

To see if it holds for bytes, substitute n = 8, and you get 2^8 = 0 (mod
256). In other words: Zero is the eighth power of two modulo 256.

Modular arithmetic is, however, very often a source of errors in
programming (unchecked-for overflows and underflows), and it is
questionable whether the programmer would really want 0 reported as a
power of two.


Vegard


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-31 10:13       ` Vegard Nossum
@ 2007-01-31 10:47         ` Robert P. J. Day
  0 siblings, 0 replies; 21+ messages in thread
From: Robert P. J. Day @ 2007-01-31 10:47 UTC (permalink / raw)
  To: Vegard Nossum
  Cc: Jan Engelhardt, Nick Piggin, Linux kernel mailing list,
	Andrew Morton, Paul Mackerras, dhowells, galak

On Wed, 31 Jan 2007, Vegard Nossum wrote:

> On Tue, January 30, 2007 3:12 pm, Jan Engelhardt wrote:
> >
> >>> Why the qualifier? Zero *is* not a power of 2, is it?
> >
> > No, it is not:
> >
> > 	In[1]:= Solve[2^n == 0, n]
> >
> > 	Out[1]= {}
> >
> > So says Mathematica5.
>
> As a side note, I would just like to point out that Mathematica does
> not deal with modular arithmetic by default (which programmers very
> much do). In fact, in modular arithmetic, zero IS a power of two.
>
> 2^n = 0 (mod 2^n)
>
> To see if it holds for bytes, substitute n = 8, and you get 2^8 = 0
> (mod 256). In other words: Zero is the eighth power of two modulo
> 256.

that's a bit esoteric but, yes, you make a good point.

> Modular arithmetic is, however, very often a source of errors in
> programming (unchecked-for overflows and underflows), and it is
> questionable whether the programmer would really want 0 reported as
> a power of two.

precisely.   given the definition of "is_power_of_2()" that's been
published, some people will (quite correctly) point out that using
that macro in place of the test "(n & (n - 1))" isn't quite the same
thing.  and they would be correct.

but one wonders how many programmers have been using that very test
"((n & (n - 1))" without realizing that it would accept zero, and that
it has been accepting zero all this time, perhaps leading to weird and
inexplicable errors.

rewriting that test as "is_power_of_2()" may, in fact, cause some
things to suddenly break, but perhaps those are things that should
have been *forced* to break anyway, to identify where the condition
check was incorrect all this time, and someone's just been lucky, or
something like that.

***NOTE***:  and that's why i suggested that the individual
maintainers might want to make this substitution themselves, to make
sure everything continues to work.  because if you leave it as a
project for the kernel janitors, they might not realize what you had
in mind.  so it's in your best interest to start cleaning this up on
your own.

just my $0.02 (Cdn.)

rday

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://www.fsdev.dreamhosters.com/wiki/index.php?title=Main_Page
========================================================================

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h. 
  2007-01-30 11:06 [PATCH] Add "is_power_of_2" checking to log2.h Robert P. J. Day
  2007-01-30 12:25 ` Nick Piggin
@ 2007-02-01 10:41 ` David Howells
  2007-02-01 10:43 ` David Howells
  2007-02-20 15:29 ` Mariusz Kozlowski
  3 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2007-02-01 10:41 UTC (permalink / raw)
  To: Nick Piggin
  Cc: Robert P. J. Day, Linux kernel mailing list, Andrew Morton,
	Paul Mackerras, dhowells, galak

Nick Piggin <nickpiggin@yahoo.com.au> wrote:

> > + *  Determine whether some value is a power of two, where zero is
> > + * *not* considered a power of two.
> > + */
> 
> Why the qualifier? Zero *is* not a power of 2, is it?

The qualifier is worth leaving in the comment, just so that people who want to
use the function can be certain what it'll do for them.

David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h. 
  2007-01-30 11:06 [PATCH] Add "is_power_of_2" checking to log2.h Robert P. J. Day
  2007-01-30 12:25 ` Nick Piggin
  2007-02-01 10:41 ` David Howells
@ 2007-02-01 10:43 ` David Howells
  2007-02-01 10:49   ` Robert P. J. Day
  2007-02-01 12:39   ` Tim Schmielau
  2007-02-20 15:29 ` Mariusz Kozlowski
  3 siblings, 2 replies; 21+ messages in thread
From: David Howells @ 2007-02-01 10:43 UTC (permalink / raw)
  To: Robert P. J. Day
  Cc: Linux kernel mailing list, Andrew Morton, Paul Mackerras,
	dhowells, galak


Robert P. J. Day <rpjday@mindspring.com> wrote:

> +#define is_power_of_4(x)	is_power_of_2(x) && (ffs(x) & 1))

If this is such a commonly implemented op, it should probably be implemented
globally too.

I also wonder if there's some better way of implementing it than this, but I
can't think of one offhand.

David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-02-01 10:43 ` David Howells
@ 2007-02-01 10:49   ` Robert P. J. Day
  2007-02-01 12:39   ` Tim Schmielau
  1 sibling, 0 replies; 21+ messages in thread
From: Robert P. J. Day @ 2007-02-01 10:49 UTC (permalink / raw)
  To: David Howells
  Cc: Linux kernel mailing list, Andrew Morton, Paul Mackerras, galak

On Thu, 1 Feb 2007, David Howells wrote:

>
> Robert P. J. Day <rpjday@mindspring.com> wrote:
>
> > +#define is_power_of_4(x)	is_power_of_2(x) && (ffs(x) & 1))
>
> If this is such a commonly implemented op, it should probably be implemented
> globally too.

there are only two checks of the form "is_power_of_4" in the entire
tree, and they're both in the power pc stuff, so i doubt it's worth
trying to formalize that in any way.

rday

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://www.fsdev.dreamhosters.com/wiki/index.php?title=Main_Page
========================================================================

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h. 
  2007-02-01 10:43 ` David Howells
  2007-02-01 10:49   ` Robert P. J. Day
@ 2007-02-01 12:39   ` Tim Schmielau
  2007-02-01 20:17     ` Valdis.Kletnieks
  1 sibling, 1 reply; 21+ messages in thread
From: Tim Schmielau @ 2007-02-01 12:39 UTC (permalink / raw)
  To: David Howells
  Cc: Robert P. J. Day, Linux kernel mailing list, Andrew Morton,
	Paul Mackerras, galak

On Thu, 1 Feb 2007, David Howells wrote:
> Robert P. J. Day <rpjday@mindspring.com> wrote:
> 
> > +#define is_power_of_4(x)	is_power_of_2(x) && (ffs(x) & 1))
> 
> If this is such a commonly implemented op, it should probably be implemented
> globally too.
> 
> I also wonder if there's some better way of implementing it than this, but I
> can't think of one offhand.

#define is_power_of_2_or_zero(x) ((x & (x-1))==0)
#define is_power_of_4(x) (is_power_of_2_or_zero(x) \
                          && (x & ((typeof(x))0x55555555)))

?

Tim

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-02-01 12:39   ` Tim Schmielau
@ 2007-02-01 20:17     ` Valdis.Kletnieks
  2007-02-01 21:55       ` Tim Schmielau
  0 siblings, 1 reply; 21+ messages in thread
From: Valdis.Kletnieks @ 2007-02-01 20:17 UTC (permalink / raw)
  To: Tim Schmielau
  Cc: David Howells, Robert P. J. Day, Linux kernel mailing list,
	Andrew Morton, Paul Mackerras, galak

[-- Attachment #1: Type: text/plain, Size: 239 bytes --]

On Thu, 01 Feb 2007 13:39:15 +0100, Tim Schmielau said:
> #define is_power_of_4(x) (is_power_of_2_or_zero(x) \
>                           && (x & ((typeof(x))0x55555555)))

Those 5's are going to need more magic if x is a 64-bit typeof?


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-02-01 20:17     ` Valdis.Kletnieks
@ 2007-02-01 21:55       ` Tim Schmielau
  0 siblings, 0 replies; 21+ messages in thread
From: Tim Schmielau @ 2007-02-01 21:55 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: David Howells, Robert P. J. Day, Linux kernel mailing list,
	Andrew Morton, Paul Mackerras, galak

On Thu, 1 Feb 2007, Valdis.Kletnieks@vt.edu wrote:

> On Thu, 01 Feb 2007 13:39:15 +0100, Tim Schmielau said:
> > #define is_power_of_4(x) (is_power_of_2_or_zero(x) \
> >                           && (x & ((typeof(x))0x55555555)))
> 
> Those 5's are going to need more magic if x is a 64-bit typeof?

Yeah, I can't count. Make that

#define is_power_of_2_or_zero(x) ((x & (x-1))==0)
#define is_power_of_4(x) (is_power_of_2_or_zero(x) \
                          && (x & ((typeof(x))0x5555555555555555ull)))

of course.

Thanks,
Tim

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 11:06 [PATCH] Add "is_power_of_2" checking to log2.h Robert P. J. Day
                   ` (2 preceding siblings ...)
  2007-02-01 10:43 ` David Howells
@ 2007-02-20 15:29 ` Mariusz Kozlowski
  2007-02-20 15:56   ` Kumar Gala
  3 siblings, 1 reply; 21+ messages in thread
From: Mariusz Kozlowski @ 2007-02-20 15:29 UTC (permalink / raw)
  To: Robert P. J. Day
  Cc: Linux kernel mailing list, Andrew Morton, Paul Mackerras,
	dhowells, galak

Hello, 

Please fix the parentheses thing in both arch/powerpc/mm/pgtable_32.c and 
arch/ppc/mm/pgtable.c.

> +#define is_power_of_4(x)	is_power_of_2(x) && (ffs(x) & 1))

Regards,

	Mariusz Kozlowski

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-02-20 15:29 ` Mariusz Kozlowski
@ 2007-02-20 15:56   ` Kumar Gala
  2007-02-20 16:14     ` Mariusz Kozlowski
  0 siblings, 1 reply; 21+ messages in thread
From: Kumar Gala @ 2007-02-20 15:56 UTC (permalink / raw)
  To: Mariusz Kozlowski
  Cc: Robert P. J. Day, Linux kernel mailing list, Andrew Morton,
	Paul Mackerras, dhowells


On Feb 20, 2007, at 9:29 AM, Mariusz Kozlowski wrote:

> Hello,
>
> Please fix the parentheses thing in both arch/powerpc/mm/ 
> pgtable_32.c and
> arch/ppc/mm/pgtable.c.
>
>> +#define is_power_of_4(x)	is_power_of_2(x) && (ffs(x) & 1))

These should be fixed and pushed upstream by now.

- k

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-02-20 15:56   ` Kumar Gala
@ 2007-02-20 16:14     ` Mariusz Kozlowski
  0 siblings, 0 replies; 21+ messages in thread
From: Mariusz Kozlowski @ 2007-02-20 16:14 UTC (permalink / raw)
  To: Kumar Gala
  Cc: Robert P. J. Day, Linux kernel mailing list, Andrew Morton,
	Paul Mackerras, dhowells

Hello,

> > Please fix the parentheses thing in both arch/powerpc/mm/ 
> > pgtable_32.c and
> > arch/ppc/mm/pgtable.c.
> >
> >> +#define is_power_of_4(x)	is_power_of_2(x) && (ffs(x) & 1))
> 
> These should be fixed and pushed upstream by now.

Great. Thanks.

Regards,

	Mariusz Kozlowski

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] Add "is_power_of_2" checking to log2.h.
  2007-01-30 14:45       ` Robert P. J. Day
  2007-01-30 15:50         ` Jan Engelhardt
@ 2007-06-11 23:11         ` H. Peter Anvin
  1 sibling, 0 replies; 21+ messages in thread
From: H. Peter Anvin @ 2007-06-11 23:11 UTC (permalink / raw)
  To: Robert P. J. Day
  Cc: Jan Engelhardt, Nick Piggin, Linux kernel mailing list,
	Andrew Morton, Paul Mackerras, dhowells, galak

Robert P. J. Day wrote:
> 
> seriously, though, there is the potential of breaking something with
> this change since you can see how there is some inconsistency in how
> it's done *now* just for powerpc which, in some places, defines its
> own versions of this:
> 
> ./arch/ppc/mm/pgtable.c:
> 	#define is_power_of_2(x) ((x) != 0 && (((x) & ((x) - 1)) == 0))
> ./arch/ppc/syslib/ppc85xx_rio.c:
> 	#define is_power_of_2(x) (((x) & ((x) - 1)) == 0)
> ./arch/powerpc/mm/pgtable_32.c:
> 	#define is_power_of_2(x) ((x) != 0 && (((x) & ((x) - 1)) == 0))
> 
> note how the first and third macros *won't* consider zero a power of
> two, while the second one *will*.  hence the need for a single
> standard for all of this, just to play it safe.
> 

I suspect the reason the test for zero was omitted is because the author
didn't want the extra cost (the test for zero needs an extra branch on a
lot of architectures.)

	-hpa

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2007-06-11 23:15 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-01-30 11:06 [PATCH] Add "is_power_of_2" checking to log2.h Robert P. J. Day
2007-01-30 12:25 ` Nick Piggin
2007-01-30 12:56   ` Robert P. J. Day
2007-01-30 14:12     ` Jan Engelhardt
2007-01-30 14:45       ` Robert P. J. Day
2007-01-30 15:50         ` Jan Engelhardt
2007-01-30 16:00           ` Robert P. J. Day
2007-06-11 23:11         ` H. Peter Anvin
2007-01-31 10:13       ` Vegard Nossum
2007-01-31 10:47         ` Robert P. J. Day
2007-01-31  1:21     ` Nick Piggin
2007-01-31  7:20       ` Robert P. J. Day
2007-02-01 10:41 ` David Howells
2007-02-01 10:43 ` David Howells
2007-02-01 10:49   ` Robert P. J. Day
2007-02-01 12:39   ` Tim Schmielau
2007-02-01 20:17     ` Valdis.Kletnieks
2007-02-01 21:55       ` Tim Schmielau
2007-02-20 15:29 ` Mariusz Kozlowski
2007-02-20 15:56   ` Kumar Gala
2007-02-20 16:14     ` Mariusz Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).