LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Robert P. J. Day" <rpjday@mindspring.com>
To: Richard Knutsson <ricknu-0@student.ltu.se>
Cc: Andreas Schwab <schwab@suse.de>,
	linux-mm@kvack.org, tony.luck@intel.com,
	linux-ia64@vger.kernel.org,
	Kernel Janitors List <kernel-janitors@lists.osdl.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [KJ] [PATCH] is_power_of_2 in ia64mm
Date: Fri, 16 Feb 2007 12:06:03 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.64.0702161202270.32716@CPE00045a9c397f-CM001225dbafb6> (raw)
In-Reply-To: <45D5DE6F.6030604@student.ltu.se>


i'm not clear on what the possible problem is here:

On Fri, 16 Feb 2007, Richard Knutsson wrote:

> Andreas Schwab wrote:
> > Richard Knutsson <ricknu-0@student.ltu.se> writes:
> >
> >> Andreas Schwab wrote:
> >>
> >>> Richard Knutsson <ricknu-0@student.ltu.se> writes:
> >>>
> >>>
> >>>> Vignesh Babu BM wrote:
> >>>>
> >>>>> @@ -175,7 +176,7 @@ static int __init hugetlb_setup_sz(char *str)
> >>>>>  		tr_pages = 0x15557000UL;
> >>>>>   	size = memparse(str, &str);
> >>>>> -	if (*str || (size & (size-1)) || !(tr_pages & size) ||
> >>>>> +	if (*str || !is_power_of_2(size) || !(tr_pages & size) ||
> >>>>>  		size <= PAGE_SIZE ||
> >>>>>  		size >= (1UL << PAGE_SHIFT << MAX_ORDER)) {
> >>>>>  		printk(KERN_WARNING "Invalid huge page size specified\n");
> >>>>>
> >>>> As we talked about before; is this really correct? !is_power_of_2(0) ==
> >>>> true while (0 & (0-1)) == 0.
> >>>>
> >>> size == 0 is also covered by the next two conditions, so the overall value
> >>> does not change.
> >>>
> >> Yes, but is it meant to state that 'size' is not a power of two?
> >>
> >
> > What else can it mean?

> What about !one_or_less_bit()? It has not been implemented (yet?)
> but been discussed.

but whether or not it's been implemented doesn't change whether or not
the code above can be simplified.  given what's being tested, and the
error message about whether a page size is valid, it seems fairly
clear that this is a power of two test.  what's the problem?

> It ended by concluding that is_power_of_2() should be fixed up first
> and then we can see about it.

there's nothing about is_power_of_2() that needs "fixing".  it's
correct as it's currently implemented.

> And as I stated before; !is_power_of_2(size) != (size & (size-1))!
> It may be a bug but then we have to be sure it is suppose to be the
> power of 2.

it may be that certain tests in the current code need to be examined
carefully to clarify what's being tested.  but the code above seems
straightforward.

rday

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

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

  parent reply	other threads:[~2007-02-16 17:10 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-16 12:03 [KJ][PATCH] " Vignesh Babu BM
2007-02-16 15:02 ` [KJ] [PATCH] " Richard Knutsson
2007-02-16 15:13   ` Andreas Schwab
2007-02-16 15:57     ` Richard Knutsson
2007-02-16 15:59       ` Andreas Schwab
2007-02-16 16:40         ` Richard Knutsson
2007-02-16 16:43           ` Andreas Schwab
2007-02-16 17:06           ` Robert P. J. Day [this message]
2007-02-16 17:36             ` Richard Knutsson
2007-02-16 16:58       ` Robert P. J. Day
2007-02-16 15:27   ` Boot time Bluetooth BUG: warning: (value > m) at hid-core.c:793 Fortier,Vincent [Montreal]
2007-02-18 21:23     ` Jiri Kosina
2007-02-19  0:25       ` Marcel Holtmann
2007-02-19  9:36         ` Jiri Kosina
2007-02-27 15:54         ` Jiri Kosina
2007-02-27 16:12           ` Fortier,Vincent [Montreal]
     [not found]             ` <Pine.LNX.4.64.0702271713220.10496@twin.jikos.cz>
2007-02-27 20:35               ` Fortier,Vincent [Montreal]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0702161202270.32716@CPE00045a9c397f-CM001225dbafb6 \
    --to=rpjday@mindspring.com \
    --cc=kernel-janitors@lists.osdl.org \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ricknu-0@student.ltu.se \
    --cc=schwab@suse.de \
    --cc=tony.luck@intel.com \
    --subject='Re: [KJ] [PATCH] is_power_of_2 in ia64mm' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).