Re: [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
Ingo Molnar writes:
> * 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());
>
> 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
Re: [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
Borislav Petkov 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
Thanks, v2 coming soon.
cheers
Re: [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
* 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
> ---
> 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
Re: [PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
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
> ---
> 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
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284
(AG Nürnberg)
--
[PATCH 1/2] kernel/smp: Make the SMP boot message common on all arches
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
---
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
