| Message ID | 1444064531-25607-12-git-send-email-suzuki.poulose@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 05, 2015 at 06:02:00PM +0100, Suzuki K. Poulose wrote: > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c > index cb3e0d8..6987de4 100644 > --- a/arch/arm64/kernel/smp.c > +++ b/arch/arm64/kernel/smp.c > @@ -163,14 +163,14 @@ asmlinkage void secondary_start_kernel(void) > cpu_ops[cpu]->cpu_postboot(); > > /* > - * Log the CPU info before it is marked online and might get read. > + * Enable GIC and timers. > */ > - cpuinfo_store_cpu(); > + notify_cpu_starting(cpu); > > /* > - * Enable GIC and timers. > + * Log the CPU info before it is marked online and might get read. > */ > - notify_cpu_starting(cpu); > + cpuinfo_store_cpu(); > > smp_store_cpu_info(cpu); You can move the cpuinfo_store_cpu() call directly to smp_store_cpu_info().
On 08/10/15 11:15, Catalin Marinas wrote: > On Mon, Oct 05, 2015 at 06:02:00PM +0100, Suzuki K. Poulose wrote: >> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c >> index cb3e0d8..6987de4 100644 >> --- a/arch/arm64/kernel/smp.c >> +++ b/arch/arm64/kernel/smp.c >> @@ -163,14 +163,14 @@ asmlinkage void secondary_start_kernel(void) >> cpu_ops[cpu]->cpu_postboot(); >> >> /* >> - * Log the CPU info before it is marked online and might get read. >> + * Enable GIC and timers. >> */ >> - cpuinfo_store_cpu(); >> + notify_cpu_starting(cpu); >> >> /* >> - * Enable GIC and timers. >> + * Log the CPU info before it is marked online and might get read. >> */ >> - notify_cpu_starting(cpu); >> + cpuinfo_store_cpu(); >> >> smp_store_cpu_info(cpu); > > You can move the cpuinfo_store_cpu() call directly to > smp_store_cpu_info(). > That looks better, Thanks. Suzuki
On 08/10/15 11:46, Suzuki K. Poulose wrote: > On 08/10/15 11:15, Catalin Marinas wrote: >> On Mon, Oct 05, 2015 at 06:02:00PM +0100, Suzuki K. Poulose wrote: >>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c >>> index cb3e0d8..6987de4 100644 >>> --- a/arch/arm64/kernel/smp.c >>> +++ b/arch/arm64/kernel/smp.c >>> @@ -163,14 +163,14 @@ asmlinkage void secondary_start_kernel(void) >>> cpu_ops[cpu]->cpu_postboot(); >>> >>> /* >>> - * Log the CPU info before it is marked online and might get read. >>> + * Enable GIC and timers. >>> */ >>> - cpuinfo_store_cpu(); >>> + notify_cpu_starting(cpu); >>> >>> /* >>> - * Enable GIC and timers. >>> + * Log the CPU info before it is marked online and might get read. >>> */ >>> - notify_cpu_starting(cpu); >>> + cpuinfo_store_cpu(); >>> >>> smp_store_cpu_info(cpu); >> >> You can move the cpuinfo_store_cpu() call directly to >> smp_store_cpu_info(). >> > > That looks better, Thanks. On a second look, smp_store_cpu_info() is also called from smp_prepare_cpus() by the Boot CPU to update its topology information, just after init_cpu_topology(). So moving the cpuinfo_store_cpu() could be called for the boot CPU (which we don't want, as it takes a different route to storing the info). One thing we could do is: - Move init_cpu_topology() to setup_processor() - Have the boot CPU update the topology from setup_processor() there, without going via the smp_store_cpu_info() - Remove the smp_store_cpu_info() from smp_prepare_cpus(). Should we do that ? Or retain the code as above ? Suzuki
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index cb3e0d8..6987de4 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -163,14 +163,14 @@ asmlinkage void secondary_start_kernel(void) cpu_ops[cpu]->cpu_postboot(); /* - * Log the CPU info before it is marked online and might get read. + * Enable GIC and timers. */ - cpuinfo_store_cpu(); + notify_cpu_starting(cpu); /* - * Enable GIC and timers. + * Log the CPU info before it is marked online and might get read. */ - notify_cpu_starting(cpu); + cpuinfo_store_cpu(); smp_store_cpu_info(cpu);
This patch delays populating the cpuinfo for a new (hotplugged) CPU until the notifiers have executed. This will enable us to verify if the new (hotplugged) CPU has all the capabilities which the system already has. If it doesn't, we could prevent it from turning online and also modifying the system wide feature register status. Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com> --- arch/arm64/kernel/smp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)