linux-kernel - Re: [PATCH] smp: force all cpu to boot once under maxcpus option

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190724083936.GA3965@dhcp-128-55.nay.redhat.com>
Date:   Wed, 24 Jul 2019 16:39:36 +0800
From:   Pingfan Liu <kernelfans@...il.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     x86@...nel.org, "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Rik van Riel <riel@...riel.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Ingo Molnar <mingo@...nel.org>, Jiri Kosina <jkosina@...e.cz>,
        Mukesh Ojha <mojha@...eaurora.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andy Lutomirski <luto@...capital.net>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] smp: force all cpu to boot once under maxcpus option

On Mon, Jul 22, 2019 at 11:41:29AM +0200, Thomas Gleixner wrote:
> On Wed, 10 Jul 2019, Pingfan Liu wrote:
> >  
> > +static inline bool maxcpus_allowed(unsigned int cpu)
> > +{
> > +	/* maxcpus only takes effect during system bootup */
> > +	if (smp_boot_done)
> > +		return true;
> > +	if (num_online_cpus() < setup_max_cpus)
> > +		return true;
> > +	/*
> > +	 * maxcpus should allow cpu to set CR4.MCE asap, otherwise the set may
> > +	 * be deferred indefinitely.
> > +	 */
> > +	if (!per_cpu(cpuhp_state, cpu).booted_once)
> > +		return true;
> 
> As this is a x86 only issue, you cannot inflict this magic on every
> architecture.
OK.
In my developing patch which fixes nr_cpus issue, I takes the following way:
(I am diverted to other things, have not finished test yet, hope to
turn back soon.)
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 362dd89..c009169 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -956,6 +956,87 @@ int common_cpu_up(unsigned int cpu, struct task_struct *idle)
        return 0;
 }

+void __init bring_capped_cpu_steady(void)
+{
[...]
+}
+
 /*
  * NOTE - on most systems this is a PHYSICAL apic ID, but on multiquad
  * (ie clustered apic addressing mode), this is a LOGICAL apic ID.
diff --git a/kernel/smp.c b/kernel/smp.c
index d155374..b04961c 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -560,6 +560,10 @@ void __init setup_nr_cpu_ids(void)
        nr_cpu_ids = find_last_bit(cpumask_bits(cpu_possible_mask),NR_CPUS) + 1;
 }

+void __weak bring_capped_cpu_steady(void)
+{
+}
+
 /* Called by boot processor to activate the rest. */
 void __init smp_init(void)
 {
@@ -579,6 +583,8 @@ void __init smp_init(void)
                        cpu_up(cpu);
        }

+       /* force cpus capped by nr_cpus option into steady state */
+       bring_capped_cpu_steady();
        num_nodes = num_online_nodes();


The initial motivation is to step around percpu area required by cpu hotplug
framework. But it also provide the abstraction of archs.

What do you think about resolving maxcpus by the similar abstraction?

> 
> Aside of that this does not solve the problem at all because smp_init()
> still does:
> 
>         for_each_present_cpu(cpu) {
>                 if (num_online_cpus() >= setup_max_cpus)
>                         break;
Yes, this logic should be removed, then maxcpus_allowed() can take effect. But
now, it may take a quite different way to resolve it.

Thanks for your kindly review.

Regards,
  Pingfan
>                 if (!cpu_online(cpu))
>                         cpu_up(cpu);
>         }
> 
> So the remaining CPUs are not onlined at all.
> 
> Thanks,
> 
> 	tglx