public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fix maxcpus=N parsing
@ 2007-08-27 15:02 Hugh Dickins
  2007-08-27 17:20 ` Linus Torvalds
  2007-08-28 17:58 ` Rusty Russell
  0 siblings, 2 replies; 4+ messages in thread
From: Hugh Dickins @ 2007-08-27 15:02 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andrew Morton, Len Brown, Andi Kleen, Rusty Russell, linux-kernel

Fix 61ec7567db103d537329b0db9a887db570431ff4: maxcpus=N is now having no
effect on x86_64, and freezing bootup on i386 (because of inconsistency
with the separate maxcpus parsing down in arch/i386, I guess).  That's
because early_param parsing is a little different from __setup parsing,
and needs the "=" omitted: then it seems to work as the original commit
intended (no mention of IO-APIC in /proc/interrupts when maxcpus=0).

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
Sorry, I noticed this back in -mm, but got diverted by deeper mysteries.
Cc'ed Rusty: I presume there's a good reason why early_param parsing is
confusingly different, but he may know better and want to change it.

It's odd that i386 treats maxcpus=N differently from other architectures:
on i386 it limits cpu_possible_map, on others it just limits what boots
(then powersaved is liable to bring up the others on x86_64 - hmmm).

--- 2.6.23-rc3-git10/init/main.c	2007-08-26 18:10:20.000000000 +0100
+++ linux/init/main.c	2007-08-26 18:59:16.000000000 +0100
@@ -168,7 +168,7 @@ static int __init maxcpus(char *str)
 	return 0;
 }
 
-early_param("maxcpus=", maxcpus);
+early_param("maxcpus", maxcpus);
 #else
 #define max_cpus NR_CPUS
 #endif

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

* Re: [PATCH] fix maxcpus=N parsing
  2007-08-27 15:02 [PATCH] fix maxcpus=N parsing Hugh Dickins
@ 2007-08-27 17:20 ` Linus Torvalds
  2007-08-27 20:53   ` Hugh Dickins
  2007-08-28 17:58 ` Rusty Russell
  1 sibling, 1 reply; 4+ messages in thread
From: Linus Torvalds @ 2007-08-27 17:20 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Andrew Morton, Len Brown, Andi Kleen, Rusty Russell, linux-kernel



On Mon, 27 Aug 2007, Hugh Dickins wrote:
>
> Fix 61ec7567db103d537329b0db9a887db570431ff4: maxcpus=N is now having no
> ...

On a totally unrelated issue:

While looking at the history in gitk and gitweb etc shows these commit 
ID's as nice hyperlinks, I really think it's nicer for everybody if the 
commit is also described with its "headline", so that non-git users (or 
even git users, when just using "git log" or similar) at least can grep or 
google for it.

So when writing descriptions, I would prefer it if people wrote them 
something like

   Commit 61ec7567db103d537329b0db9a887db570431ff4 ('ACPI: boot correctly 
   with "nosmp" or "maxcpus=0"') broke maxcpus parsing on x86[-64] ...

so that it gets both the exact commit naming and the nice hyperlinks where 
appropriate *and* it ends up being more useful even without the links.

Yes, it's redundant information, but since we have the useful one-line 
descriptions, it's generally a good idea. And I really do think that it 
tends to make the explanations read better too.

For example, in this example, I think it really made it more clear what 
kind of change had caused the breakage. Maybe that's not always true, but 
I suspect it's true most of the time.

I'll fix it up manually (I often do), but I thought I'd just mention this 
stylistic issue.

		Linus

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

* Re: [PATCH] fix maxcpus=N parsing
  2007-08-27 17:20 ` Linus Torvalds
@ 2007-08-27 20:53   ` Hugh Dickins
  0 siblings, 0 replies; 4+ messages in thread
From: Hugh Dickins @ 2007-08-27 20:53 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Andrew Morton, Len Brown, Andi Kleen, Rusty Russell, linux-kernel

On Mon, 27 Aug 2007, Linus Torvalds wrote:
> On Mon, 27 Aug 2007, Hugh Dickins wrote:
> >
> > Fix 61ec7567db103d537329b0db9a887db570431ff4: maxcpus=N is now having no
> > ...
> 
> On a totally unrelated issue:
> 
> While looking at the history in gitk and gitweb etc shows these commit 
> ID's as nice hyperlinks, I really think it's nicer for everybody if the 
> commit is also described with its "headline", so that non-git users (or 
> even git users, when just using "git log" or similar) at least can grep or 
> google for it.
> 
> So when writing descriptions, I would prefer it if people wrote them 
> something like
> 
>    Commit 61ec7567db103d537329b0db9a887db570431ff4 ('ACPI: boot correctly 
>    with "nosmp" or "maxcpus=0"') broke maxcpus parsing on x86[-64] ...
> 
> so that it gets both the exact commit naming and the nice hyperlinks where 
> appropriate *and* it ends up being more useful even without the links.

Agreed and noted.

> 
> Yes, it's redundant information, but since we have the useful one-line 
> descriptions, it's generally a good idea. And I really do think that it 
> tends to make the explanations read better too.
> 
> For example, in this example, I think it really made it more clear what 
> kind of change had caused the breakage. Maybe that's not always true, but 
> I suspect it's true most of the time.
> 
> I'll fix it up manually (I often do), but I thought I'd just mention this 
> stylistic issue.

Thanks.

Hugh

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

* Re: [PATCH] fix maxcpus=N parsing
  2007-08-27 15:02 [PATCH] fix maxcpus=N parsing Hugh Dickins
  2007-08-27 17:20 ` Linus Torvalds
@ 2007-08-28 17:58 ` Rusty Russell
  1 sibling, 0 replies; 4+ messages in thread
From: Rusty Russell @ 2007-08-28 17:58 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Linus Torvalds, Andrew Morton, Len Brown, Andi Kleen,
	linux-kernel

On Mon, 2007-08-27 at 16:02 +0100, Hugh Dickins wrote:
> Fix 61ec7567db103d537329b0db9a887db570431ff4: maxcpus=N is now having no
> effect on x86_64, and freezing bootup on i386 (because of inconsistency
> with the separate maxcpus parsing down in arch/i386, I guess).  That's
> because early_param parsing is a little different from __setup parsing,
> and needs the "=" omitted: then it seems to work as the original commit
> intended (no mention of IO-APIC in /proc/interrupts when maxcpus=0).
> 
> Signed-off-by: Hugh Dickins <hugh@veritas.com>
> ---
> Sorry, I noticed this back in -mm, but got diverted by deeper mysteries.
> Cc'ed Rusty: I presume there's a good reason why early_param parsing is
> confusingly different, but he may know better and want to change it.

Yeah, early_param is modelled on module_param which does more than the
naive substring match of __setup.  There's a warning in the header IIRC.

The original intention wass that everything would move to
module_param-style parameters.  However __setup is still useful for
trivial core stuff.

> It's odd that i386 treats maxcpus=N differently from other architectures:
> on i386 it limits cpu_possible_map, on others it just limits what boots
> (then powersaved is liable to bring up the others on x86_64 - hmmm).

Indeed, it'd be nice to see this made uniform.

But this patch is fine.

Cheers,
Rusty.


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

end of thread, other threads:[~2007-08-28 17:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-27 15:02 [PATCH] fix maxcpus=N parsing Hugh Dickins
2007-08-27 17:20 ` Linus Torvalds
2007-08-27 20:53   ` Hugh Dickins
2007-08-28 17:58 ` Rusty Russell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox