linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
@ 2016-10-13  8:55 Michael Ellerman
  2016-10-13  8:55 ` [PATCH 2/2] kernel/smp: Tell the user we're bringing up secondary CPUs Michael Ellerman
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Michael Ellerman @ 2016-10-13  8:55 UTC (permalink / raw)
  To: akpm
  Cc: tglx, mingo, hpa, x86, peterz, jgross, mgorman, richard,
	len.brown, bp, boris.ostrovsky, tim.c.chen, ak, jolsa,
	linux-kernel

Currently after bringing up secondary CPUs all arches print "Brought up
%d CPUs". On x86 they also print the number of nodes that were brought
online.

It would be nice to also print the number of nodes on other arches.
Although we could override smp_announce() on the other ~10 NUMA aware
arches, it seems simpler to just always print the number of nodes. On
non-NUMA arches there is just always 1 node.

Having done that, smp_announce() is no longer weak, and seems small
enough to just pull directly into smp_init().

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/x86/kernel/smpboot.c |  8 --------
 kernel/smp.c              | 11 +++++------
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 42a93621f5b0..7eb8dfa56d34 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -821,14 +821,6 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
 	return (send_status | accept_status);
 }
 
-void smp_announce(void)
-{
-	int num_nodes = num_online_nodes();
-
-	printk(KERN_INFO "x86: Booted up %d node%s, %d CPUs\n",
-	       num_nodes, (num_nodes > 1 ? "s" : ""), num_online_cpus());
-}
-
 /* reduce the number of lines printed when booting a large cpu count system */
 static void announce_cpu(int cpu, int apicid)
 {
diff --git a/kernel/smp.c b/kernel/smp.c
index bba3b201668d..6f5696d260c8 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -543,15 +543,11 @@ void __init setup_nr_cpu_ids(void)
 	nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask),NR_CPUS) + 1;
 }
 
-void __weak smp_announce(void)
-{
-	printk(KERN_INFO "Brought up %d CPUs\n", num_online_cpus());
-}
-
 /* Called by boot processor to activate the rest. */
 void __init smp_init(void)
 {
 	unsigned int cpu;
+	int num_nodes;
 
 	idle_threads_init();
 	cpuhp_threads_init();
@@ -564,8 +560,11 @@ void __init smp_init(void)
 			cpu_up(cpu);
 	}
 
+	num_nodes = num_online_nodes();
+	pr_info("smp: Brought up %d node%s, %d CPUs\n",
+		num_nodes, (num_nodes > 1 ? "s" : ""), num_online_cpus());
+
 	/* Any cleanup work */
-	smp_announce();
 	smp_cpus_done(setup_max_cpus);
 }
 
-- 
2.7.4

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

* [PATCH 2/2] kernel/smp: Tell the user we're bringing up secondary CPUs
  2016-10-13  8:55 [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches Michael Ellerman
@ 2016-10-13  8:55 ` Michael Ellerman
  2016-10-14 11:54 ` [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches Borislav Petkov
  2016-10-16  7:04 ` Ingo Molnar
  2 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2016-10-13  8:55 UTC (permalink / raw)
  To: akpm
  Cc: tglx, mingo, hpa, x86, peterz, jgross, mgorman, richard,
	len.brown, bp, boris.ostrovsky, tim.c.chen, ak, jolsa,
	linux-kernel

Currently we don't print anything before starting to bring up secondary
CPUs. This can be confusing if it takes a long time to bring up the
secondaries, or if the kernel crashes while doing so and produces no
further output.

On x86 they work around this by detecting when the first secondary CPU
comes up and printing a message (see announce_cpu()). But doing it in
smp_init() is simpler and works for all arches.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 kernel/smp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/smp.c b/kernel/smp.c
index 6f5696d260c8..6d9af104de37 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -552,6 +552,8 @@ void __init smp_init(void)
 	idle_threads_init();
 	cpuhp_threads_init();
 
+	pr_info("smp: Bringing up secondary CPUs ...\n");
+
 	/* FIXME: This should be done in userspace --RR */
 	for_each_present_cpu(cpu) {
 		if (num_online_cpus() >= setup_max_cpus)
-- 
2.7.4

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

* Re: [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
  2016-10-13  8:55 [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches Michael Ellerman
  2016-10-13  8:55 ` [PATCH 2/2] kernel/smp: Tell the user we're bringing up secondary CPUs Michael Ellerman
@ 2016-10-14 11:54 ` Borislav Petkov
  2016-10-26  5:32   ` Michael Ellerman
  2016-10-16  7:04 ` Ingo Molnar
  2 siblings, 1 reply; 6+ messages in thread
From: Borislav Petkov @ 2016-10-14 11:54 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: akpm, tglx, mingo, hpa, x86, peterz, jgross, mgorman, richard,
	len.brown, boris.ostrovsky, tim.c.chen, ak, jolsa, linux-kernel

On Thu, Oct 13, 2016 at 07:55:19PM +1100, Michael Ellerman wrote:
> Currently after bringing up secondary CPUs all arches print "Brought up
> %d CPUs". On x86 they also print the number of nodes that were brought
> online.
> 
> It would be nice to also print the number of nodes on other arches.
> Although we could override smp_announce() on the other ~10 NUMA aware
> arches, it seems simpler to just always print the number of nodes. On
> non-NUMA arches there is just always 1 node.
> 
> Having done that, smp_announce() is no longer weak, and seems small
> enough to just pull directly into smp_init().
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/x86/kernel/smpboot.c |  8 --------
>  kernel/smp.c              | 11 +++++------
>  2 files changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 42a93621f5b0..7eb8dfa56d34 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -821,14 +821,6 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
>  	return (send_status | accept_status);
>  }
>  
> -void smp_announce(void)
> -{
> -	int num_nodes = num_online_nodes();
> -
> -	printk(KERN_INFO "x86: Booted up %d node%s, %d CPUs\n",
> -	       num_nodes, (num_nodes > 1 ? "s" : ""), num_online_cpus());
> -}
> -
>  /* reduce the number of lines printed when booting a large cpu count system */
>  static void announce_cpu(int cpu, int apicid)
>  {
> diff --git a/kernel/smp.c b/kernel/smp.c
> index bba3b201668d..6f5696d260c8 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -543,15 +543,11 @@ void __init setup_nr_cpu_ids(void)
>  	nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask),NR_CPUS) + 1;
>  }
>  
> -void __weak smp_announce(void)
> -{
> -	printk(KERN_INFO "Brought up %d CPUs\n", num_online_cpus());
> -}
> -
>  /* Called by boot processor to activate the rest. */
>  void __init smp_init(void)
>  {
>  	unsigned int cpu;
> +	int num_nodes;
>  
>  	idle_threads_init();
>  	cpuhp_threads_init();
> @@ -564,8 +560,11 @@ void __init smp_init(void)
>  			cpu_up(cpu);
>  	}
>  
> +	num_nodes = num_online_nodes();
> +	pr_info("smp: Brought up %d node%s, %d CPUs\n",
> +		num_nodes, (num_nodes > 1 ? "s" : ""), num_online_cpus());

Please define pr_fmt for this file so that pr_info adds the prefix
automatically. I guess

	#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

at the top, before all the include directives should suffice.

Other than that, for both patches:

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

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

* Re: [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
  2016-10-13  8:55 [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches Michael Ellerman
  2016-10-13  8:55 ` [PATCH 2/2] kernel/smp: Tell the user we're bringing up secondary CPUs Michael Ellerman
  2016-10-14 11:54 ` [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches Borislav Petkov
@ 2016-10-16  7:04 ` Ingo Molnar
  2016-10-26  5:34   ` Michael Ellerman
  2 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2016-10-16  7:04 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: akpm, tglx, mingo, hpa, x86, peterz, jgross, mgorman, richard,
	len.brown, bp, boris.ostrovsky, tim.c.chen, ak, jolsa,
	linux-kernel


* Michael Ellerman <mpe@ellerman.id.au> wrote:

> Currently after bringing up secondary CPUs all arches print "Brought up
> %d CPUs". On x86 they also print the number of nodes that were brought
> online.
> 
> It would be nice to also print the number of nodes on other arches.
> Although we could override smp_announce() on the other ~10 NUMA aware
> arches, it seems simpler to just always print the number of nodes. On
> non-NUMA arches there is just always 1 node.
> 
> Having done that, smp_announce() is no longer weak, and seems small
> enough to just pull directly into smp_init().
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/x86/kernel/smpboot.c |  8 --------
>  kernel/smp.c              | 11 +++++------
>  2 files changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 42a93621f5b0..7eb8dfa56d34 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -821,14 +821,6 @@ wakeup_secondary_cpu_via_init(int phys_apicid, unsigned long start_eip)
>  	return (send_status | accept_status);
>  }
>  
> -void smp_announce(void)
> -{
> -	int num_nodes = num_online_nodes();
> -
> -	printk(KERN_INFO "x86: Booted up %d node%s, %d CPUs\n",
> -	       num_nodes, (num_nodes > 1 ? "s" : ""), num_online_cpus());
> -}
> -
>  /* reduce the number of lines printed when booting a large cpu count system */
>  static void announce_cpu(int cpu, int apicid)
>  {
> diff --git a/kernel/smp.c b/kernel/smp.c
> index bba3b201668d..6f5696d260c8 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -543,15 +543,11 @@ void __init setup_nr_cpu_ids(void)
>  	nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask),NR_CPUS) + 1;
>  }
>  
> -void __weak smp_announce(void)
> -{
> -	printk(KERN_INFO "Brought up %d CPUs\n", num_online_cpus());
> -}
> -
>  /* Called by boot processor to activate the rest. */
>  void __init smp_init(void)
>  {
>  	unsigned int cpu;
> +	int num_nodes;
>  
>  	idle_threads_init();
>  	cpuhp_threads_init();
> @@ -564,8 +560,11 @@ void __init smp_init(void)
>  			cpu_up(cpu);
>  	}
>  
> +	num_nodes = num_online_nodes();
> +	pr_info("smp: Brought up %d node%s, %d CPUs\n",
> +		num_nodes, (num_nodes > 1 ? "s" : ""), num_online_cpus());

No objections - but pedantry requires me to mention that while we are evolving 
this code and changing the strings I think we should make the CPU announcement 
CPU%s smart as well: an SMP kernel on a single CPU bootup will result in 
num_online_cpus() == 1, right?

Thanks,

	Ingo

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

* Re: [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
  2016-10-14 11:54 ` [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches Borislav Petkov
@ 2016-10-26  5:32   ` Michael Ellerman
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2016-10-26  5:32 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: akpm, tglx, mingo, hpa, x86, peterz, jgross, mgorman, richard,
	len.brown, boris.ostrovsky, tim.c.chen, ak, jolsa, linux-kernel

Borislav Petkov <bp@suse.de> writes:
> On Thu, Oct 13, 2016 at 07:55:19PM +1100, Michael Ellerman wrote:
>> @@ -564,8 +560,11 @@ void __init smp_init(void)
>>  			cpu_up(cpu);
>>  	}
>>  
>> +	num_nodes = num_online_nodes();
>> +	pr_info("smp: Brought up %d node%s, %d CPUs\n",
>> +		num_nodes, (num_nodes > 1 ? "s" : ""), num_online_cpus());
>
> Please define pr_fmt for this file so that pr_info adds the prefix
> automatically. I guess
>
> 	#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> at the top, before all the include directives should suffice.

Sure thing.

> Other than that, for both patches:
>
> Reviewed-by: Borislav Petkov <bp@suse.de>

Thanks, v2 coming soon.

cheers

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

* Re: [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
  2016-10-16  7:04 ` Ingo Molnar
@ 2016-10-26  5:34   ` Michael Ellerman
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2016-10-26  5:34 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: akpm, tglx, mingo, hpa, x86, peterz, jgross, mgorman, richard,
	len.brown, bp, boris.ostrovsky, tim.c.chen, ak, jolsa,
	linux-kernel

Ingo Molnar <mingo@kernel.org> writes:
> * Michael Ellerman <mpe@ellerman.id.au> wrote:
>> @@ -564,8 +560,11 @@ void __init smp_init(void)
>>  			cpu_up(cpu);
>>  	}
>>  
>> +	num_nodes = num_online_nodes();
>> +	pr_info("smp: Brought up %d node%s, %d CPUs\n",
>> +		num_nodes, (num_nodes > 1 ? "s" : ""), num_online_cpus());
>
> No objections - but pedantry requires me to mention that while we are evolving 
> this code and changing the strings I think we should make the CPU announcement 
> CPU%s smart as well: an SMP kernel on a single CPU bootup will result in 
> num_online_cpus() == 1, right?

Yeah that makes sense. I don't often boot any single CPU systems, but I
tested with maxcpus=1 and it does look nicer:

    smp: Brought up 2 nodes, 1 CPU


Will send a v2.

cheers

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

end of thread, other threads:[~2016-10-26  5:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-13  8:55 [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches Michael Ellerman
2016-10-13  8:55 ` [PATCH 2/2] kernel/smp: Tell the user we're bringing up secondary CPUs Michael Ellerman
2016-10-14 11:54 ` [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches Borislav Petkov
2016-10-26  5:32   ` Michael Ellerman
2016-10-16  7:04 ` Ingo Molnar
2016-10-26  5:34   ` Michael Ellerman

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