[PATCH] use "is_power_of_2()" macro for simplicity.

[PATCH] use "is_power_of_2()" macro for simplicity.

[Robert P. J. Day]

Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>

---

  this source file already includes log2.h for other power-of-2
operations, so might as well finish off the job.


diff --git a/kernel/kfifo.c b/kernel/kfifo.c
index bc41ad0..04a1748 100644
--- a/kernel/kfifo.c
+++ b/kernel/kfifo.c
@@ -74,7 +74,7 @@ struct kfifo *kfifo_alloc(unsigned int size, gfp_t gfp_mask, spinlock_t *lock)
 	 * round up to the next power of 2, since our 'let the indices
 	 * wrap' tachnique works only in this case.
 	 */
-	if (size & (size - 1)) {
+	if (!is_power_of_2(size)) {
 		BUG_ON(size > 0x80000000);
 		size = roundup_pow_of_two(size);
 	}
-- 
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
====================================

Re: [PATCH] use "is_power_of_2()" macro for simplicity.

[Matthew Wilcox]

On Wed, Nov 07, 2007 at 03:03:39AM -0500, Robert P. J. Day wrote:
>  	 */
> -	if (size & (size - 1)) {
> +	if (!is_power_of_2(size)) {
>  		BUG_ON(size > 0x80000000);
>  		size = roundup_pow_of_two(size);
>  	}

Why not just delete the if?

	BUG_ON(size > 0x80000000);
	size = roundup_pow_of_two(size);

It's not a fast-path, and we'll consume a little less .text

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] use "is_power_of_2()" macro for simplicity.

[Robert P. J. Day]

On Wed, 7 Nov 2007, Matthew Wilcox wrote:

> On Wed, Nov 07, 2007 at 03:03:39AM -0500, Robert P. J. Day wrote:
> >  	 */
> > -	if (size & (size - 1)) {
> > +	if (!is_power_of_2(size)) {
> >  		BUG_ON(size > 0x80000000);
> >  		size = roundup_pow_of_two(size);
> >  	}
>
> Why not just delete the if?
>
> 	BUG_ON(size > 0x80000000);
> 	size = roundup_pow_of_two(size);
>
> It's not a fast-path, and we'll consume a little less .text

sure, that's doable.  i'm only reluctant to make simplifications like
that since sometimes it's visually useful to have a test there, to
perhaps make it clear that something should *normally* be a power of
two, but you want to handle the case when, occasionally, it isn't.

by taking out the test, someone reading the code might get the
impression that that value will *always* be rounded up (that is,
actually changed).  like i said, it's more a visual thing than
anything else but, if no one cares, i'll take out the test.  as you
noticed, it's entirely equivalent.

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

http://crashcourse.ca
====================================