[PATCH] ppc64 boot: Wait for boot cpu to show up if nr_cpus limit is about to hit.

[PATCH] ppc64 boot: Wait for boot cpu to show up if nr_cpus limit is about to hit.

[Mahesh J Salgaonkar]

From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

The kernel boot parameter 'nr_cpus=' allows one to specify number of
possible cpus in the system. In the normal scenario the first cpu (cpu0)
that shows up is the boot cpu and hence it gets covered under nr_cpus
limit.

But this assumption will be broken in kdump scenario where kdump kenrel
after a crash can boot up on an non-zero boot cpu. The paca structure
allocation depends on value of nr_cpus and is indexed using logical cpu
ids. This definetly will be an issue if boot cpu id > nr_cpus

This patch modifies allocate_pacas() and smp_setup_cpu_maps() to
accommodate boot cpu for the case where boot_cpuid > nr_cpu_ids.

This change would help to reduce the memory reservation requirement for
kdump on ppc64.

Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/paca.h    |    3 +++
 arch/powerpc/include/asm/smp.h     |    1 +
 arch/powerpc/kernel/paca.c         |   23 ++++++++++++++++++----
 arch/powerpc/kernel/prom.c         |   37 +++++++++++++++++++++++++++++++++++-
 arch/powerpc/kernel/setup-common.c |   25 ++++++++++++++++++++++++
 5 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 70bd438..9be48b4 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -41,6 +41,9 @@ extern unsigned int debug_smp_processor_id(void); /* from linux/smp.h */
 #define get_lppaca()	(get_paca()->lppaca_ptr)
 #define get_slb_shadow()	(get_paca()->slb_shadow_ptr)
 
+/* Maximum number of threads per core. */
+#define	MAX_SMT		8
+
 struct task_struct;
 
 /*
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 825663c..0a5b99f 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -30,6 +30,7 @@
 #include <asm/percpu.h>
 
 extern int boot_cpuid;
+extern int boot_hw_cpuid;
 extern int spinning_secondaries;
 
 extern void cpu_die(void);
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 01ea0ed..96e5715 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -206,6 +206,7 @@ void __init allocate_pacas(void)
 {
 	u64 limit;
 	int cpu;
+	int nr_cpus;
 
 	limit = ppc64_rma_size;
 
@@ -218,20 +219,32 @@ void __init allocate_pacas(void)
 	limit = min(0x10000000ULL, limit);
 #endif
 
-	paca_size = PAGE_ALIGN(sizeof(struct paca_struct) * nr_cpu_ids);
+	/*
+	 * Always align up the nr_cpu_ids to SMT threads and allocate
+	 * the paca. This will help us to prepare for a situation where
+	 * boot cpu id > nr_cpus_id. We will use the last nthreads
+	 * slots (nthreads == threads per core) to accommodate a core
+	 * that contains boot cpu thread.
+	 *
+	 * Do not change nr_cpu_ids value here. Let us do that in
+	 * early_init_dt_scan_cpus() where we know exact value
+	 * of threads per core.
+	 */
+	nr_cpus = _ALIGN_UP(nr_cpu_ids, MAX_SMT);
+	paca_size = PAGE_ALIGN(sizeof(struct paca_struct) * nr_cpus);
 
 	paca = __va(memblock_alloc_base(paca_size, PAGE_SIZE, limit));
 	memset(paca, 0, paca_size);
 
 	printk(KERN_DEBUG "Allocated %u bytes for %d pacas at %p\n",
-		paca_size, nr_cpu_ids, paca);
+		paca_size, nr_cpus, paca);
 
-	allocate_lppacas(nr_cpu_ids, limit);
+	allocate_lppacas(nr_cpus, limit);
 
-	allocate_slb_shadows(nr_cpu_ids, limit);
+	allocate_slb_shadows(nr_cpus, limit);
 
 	/* Can't use for_each_*_cpu, as they aren't functional yet */
-	for (cpu = 0; cpu < nr_cpu_ids; cpu++)
+	for (cpu = 0; cpu < nr_cpus; cpu++)
 		initialise_paca(&paca[cpu], cpu);
 }
 
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 7030b03..9d1568f 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -291,6 +291,29 @@ static void __init check_cpu_feature_properties(unsigned long node)
 	}
 }
 
+/*
+ * Adjust the logical id of a boot cpu to fall under nr_cpu_ids. Map it to
+ * last core slot in the allocated paca array.
+ *
+ * e.g. on SMT=8 system, kernel booted with nr_cpus=1 and boot cpu = 33,
+ * align nr_cpu_ids to MAX_SMT value 8. Allocate paca array to hold up-to
+ * MAX_SMT=8 cpus. Since boot cpu 33 is greater than nr_cpus (8), adjust
+ * its logical id so that new id becomes less than nr_cpu_ids. Make sure
+ * that boot cpu's new logical id is aligned to its thread id and falls
+ * under last nthreads slots available in paca array. In this case the
+ * boot cpu 33 is adjusted to new boot cpu id 1.
+ *
+ */
+static inline void adjust_boot_cpuid(int nthreads, int phys_id)
+{
+	boot_hw_cpuid = phys_id;
+	if (boot_cpuid >= nr_cpu_ids) {
+		boot_cpuid = (boot_cpuid % nthreads) + (nr_cpu_ids - nthreads);
+		pr_info("Adjusted logical boot cpu id: logical %d physical %d\n",
+			boot_cpuid, phys_id);
+	}
+}
+
 static int __init early_init_dt_scan_cpus(unsigned long node,
 					  const char *uname, int depth,
 					  void *data)
@@ -315,6 +338,16 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
 	nthreads = len / sizeof(int);
 
 	/*
+	 * Now that we know threads per core lets align nr_cpu_ids to
+	 * correct SMT value.
+	 */
+	if (nr_cpu_ids % nthreads) {
+		nr_cpu_ids = _ALIGN_UP(nr_cpu_ids, nthreads);
+		pr_info("Aligned nr_cpus to SMT=%d, nr_cpu_ids = %d\n",
+				 nthreads, nr_cpu_ids);
+	}
+
+	/*
 	 * Now see if any of these threads match our boot cpu.
 	 * NOTE: This must match the parsing done in smp_setup_cpu_maps.
 	 */
@@ -352,7 +385,9 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
 	DBG("boot cpu: logical %d physical %d\n", found,
 	    be32_to_cpu(intserv[found_thread]));
 	boot_cpuid = found;
-	set_hard_smp_processor_id(found, be32_to_cpu(intserv[found_thread]));
+	adjust_boot_cpuid(nthreads, be32_to_cpu(intserv[found_thread]));
+	set_hard_smp_processor_id(boot_cpuid,
+					be32_to_cpu(intserv[found_thread]));
 
 	/*
 	 * PAPR defines "logical" PVR values for cpus that
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 44c8d03..93c7e5f 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -77,6 +77,7 @@ struct machdep_calls *machine_id;
 EXPORT_SYMBOL(machine_id);
 
 int boot_cpuid = -1;
+int boot_hw_cpuid = -1;
 EXPORT_SYMBOL_GPL(boot_cpuid);
 
 unsigned long klimit = (unsigned long) _end;
@@ -444,6 +445,7 @@ void __init smp_setup_cpu_maps(void)
 	struct device_node *dn = NULL;
 	int cpu = 0;
 	int nthreads = 1;
+	bool boot_cpu_added = false;
 
 	DBG("smp_setup_cpu_maps()\n");
 
@@ -470,6 +472,24 @@ void __init smp_setup_cpu_maps(void)
 		}
 
 		nthreads = len / sizeof(int);
+		/*
+		 * If boot cpu hasn't been added to paca and there are only
+		 * last nthreads slots available in paca array then wait
+		 * for boot cpu to show up.
+		 */
+		if (!boot_cpu_added && (cpu + nthreads) >= nr_cpu_ids) {
+			int found = 0;
+
+			DBG("Holding last nthreads paca slots for boot cpu\n");
+			for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) {
+				if (boot_hw_cpuid == be32_to_cpu(intserv[j])) {
+					found = 1;
+					break;
+				}
+			}
+			if (!found)
+				continue;
+		}
 
 		for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) {
 			bool avail;
@@ -485,6 +505,11 @@ void __init smp_setup_cpu_maps(void)
 			set_cpu_present(cpu, avail);
 			set_hard_smp_processor_id(cpu, be32_to_cpu(intserv[j]));
 			set_cpu_possible(cpu, true);
+			if (boot_hw_cpuid == be32_to_cpu(intserv[j])) {
+				DBG("Boot cpu %d (hard id %d) added to paca\n",
+				    cpu, be32_to_cpu(intserv[j]));
+				boot_cpu_added = true;
+			}
 			cpu++;
 		}
 	}

Re: [PATCH] ppc64 boot: Wait for boot cpu to show up if nr_cpus limit is about to hit.

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]

Hi Mahesh,

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.5-rc2 next-20160201]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Mahesh-J-Salgaonkar/ppc64-boot-Wait-for-boot-cpu-to-show-up-if-nr_cpus-limit-is-about-to-hit/20160202-012114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/kernel/prom.c: In function 'early_init_dt_scan_cpus':
>> arch/powerpc/kernel/prom.c:345:14: error: lvalue required as left operand of assignment
      nr_cpu_ids = _ALIGN_UP(nr_cpu_ids, nthreads);
                 ^

vim +345 arch/powerpc/kernel/prom.c

   339	
   340		/*
   341		 * Now that we know threads per core lets align nr_cpu_ids to
   342		 * correct SMT value.
   343		 */
   344		if (nr_cpu_ids % nthreads) {
 > 345			nr_cpu_ids = _ALIGN_UP(nr_cpu_ids, nthreads);
   346			pr_info("Aligned nr_cpus to SMT=%d, nr_cpu_ids = %d\n",
   347					 nthreads, nr_cpu_ids);
   348		}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 5708 bytes --]

Re: [PATCH] ppc64 boot: Wait for boot cpu to show up if nr_cpus limit is about to hit.

[Denis Kirjanov]

On 2/1/16, Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>
> The kernel boot parameter 'nr_cpus=' allows one to specify number of
> possible cpus in the system. In the normal scenario the first cpu (cpu0)
> that shows up is the boot cpu and hence it gets covered under nr_cpus
> limit.
>
> But this assumption will be broken in kdump scenario where kdump kenrel
> after a crash can boot up on an non-zero boot cpu. The paca structure
> allocation depends on value of nr_cpus and is indexed using logical cpu
> ids. This definetly will be an issue if boot cpu id > nr_cpus
And what happend in this case? Have you tried it out?
>
> This patch modifies allocate_pacas() and smp_setup_cpu_maps() to
> accommodate boot cpu for the case where boot_cpuid > nr_cpu_ids.
>
> This change would help to reduce the memory reservation requirement for
> kdump on ppc64.
>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/paca.h    |    3 +++
>  arch/powerpc/include/asm/smp.h     |    1 +
>  arch/powerpc/kernel/paca.c         |   23 ++++++++++++++++++----
>  arch/powerpc/kernel/prom.c         |   37
> +++++++++++++++++++++++++++++++++++-
>  arch/powerpc/kernel/setup-common.c |   25 ++++++++++++++++++++++++
>  5 files changed, 83 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/paca.h
> b/arch/powerpc/include/asm/paca.h
> index 70bd438..9be48b4 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -41,6 +41,9 @@ extern unsigned int debug_smp_processor_id(void); /* from
> linux/smp.h */
>  #define get_lppaca()	(get_paca()->lppaca_ptr)
>  #define get_slb_shadow()	(get_paca()->slb_shadow_ptr)
>
> +/* Maximum number of threads per core. */
> +#define	MAX_SMT		8
> +
>  struct task_struct;
>
>  /*
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 825663c..0a5b99f 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -30,6 +30,7 @@
>  #include <asm/percpu.h>
>
>  extern int boot_cpuid;
> +extern int boot_hw_cpuid;
>  extern int spinning_secondaries;
>
>  extern void cpu_die(void);
> diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
> index 01ea0ed..96e5715 100644
> --- a/arch/powerpc/kernel/paca.c
> +++ b/arch/powerpc/kernel/paca.c
> @@ -206,6 +206,7 @@ void __init allocate_pacas(void)
>  {
>  	u64 limit;
>  	int cpu;
> +	int nr_cpus;
>
>  	limit = ppc64_rma_size;
>
> @@ -218,20 +219,32 @@ void __init allocate_pacas(void)
>  	limit = min(0x10000000ULL, limit);
>  #endif
>
> -	paca_size = PAGE_ALIGN(sizeof(struct paca_struct) * nr_cpu_ids);
> +	/*
> +	 * Always align up the nr_cpu_ids to SMT threads and allocate
> +	 * the paca. This will help us to prepare for a situation where
> +	 * boot cpu id > nr_cpus_id. We will use the last nthreads
> +	 * slots (nthreads == threads per core) to accommodate a core
> +	 * that contains boot cpu thread.
> +	 *
> +	 * Do not change nr_cpu_ids value here. Let us do that in
> +	 * early_init_dt_scan_cpus() where we know exact value
> +	 * of threads per core.
> +	 */
> +	nr_cpus = _ALIGN_UP(nr_cpu_ids, MAX_SMT);
> +	paca_size = PAGE_ALIGN(sizeof(struct paca_struct) * nr_cpus);
>
>  	paca = __va(memblock_alloc_base(paca_size, PAGE_SIZE, limit));
>  	memset(paca, 0, paca_size);
>
>  	printk(KERN_DEBUG "Allocated %u bytes for %d pacas at %p\n",
> -		paca_size, nr_cpu_ids, paca);
> +		paca_size, nr_cpus, paca);
>
> -	allocate_lppacas(nr_cpu_ids, limit);
> +	allocate_lppacas(nr_cpus, limit);
>
> -	allocate_slb_shadows(nr_cpu_ids, limit);
> +	allocate_slb_shadows(nr_cpus, limit);
>
>  	/* Can't use for_each_*_cpu, as they aren't functional yet */
> -	for (cpu = 0; cpu < nr_cpu_ids; cpu++)
> +	for (cpu = 0; cpu < nr_cpus; cpu++)
>  		initialise_paca(&paca[cpu], cpu);
>  }
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 7030b03..9d1568f 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,6 +291,29 @@ static void __init
> check_cpu_feature_properties(unsigned long node)
>  	}
>  }
>
> +/*
> + * Adjust the logical id of a boot cpu to fall under nr_cpu_ids. Map it to
> + * last core slot in the allocated paca array.
> + *
> + * e.g. on SMT=8 system, kernel booted with nr_cpus=1 and boot cpu = 33,
> + * align nr_cpu_ids to MAX_SMT value 8. Allocate paca array to hold up-to
> + * MAX_SMT=8 cpus. Since boot cpu 33 is greater than nr_cpus (8), adjust
> + * its logical id so that new id becomes less than nr_cpu_ids. Make sure
> + * that boot cpu's new logical id is aligned to its thread id and falls
> + * under last nthreads slots available in paca array. In this case the
> + * boot cpu 33 is adjusted to new boot cpu id 1.
> + *
> + */
> +static inline void adjust_boot_cpuid(int nthreads, int phys_id)
> +{
> +	boot_hw_cpuid = phys_id;
> +	if (boot_cpuid >= nr_cpu_ids) {
> +		boot_cpuid = (boot_cpuid % nthreads) + (nr_cpu_ids - nthreads);
> +		pr_info("Adjusted logical boot cpu id: logical %d physical %d\n",
> +			boot_cpuid, phys_id);
> +	}
> +}
> +
>  static int __init early_init_dt_scan_cpus(unsigned long node,
>  					  const char *uname, int depth,
>  					  void *data)
> @@ -315,6 +338,16 @@ static int __init early_init_dt_scan_cpus(unsigned long
> node,
>  	nthreads = len / sizeof(int);
>
>  	/*
> +	 * Now that we know threads per core lets align nr_cpu_ids to
> +	 * correct SMT value.
> +	 */
> +	if (nr_cpu_ids % nthreads) {
> +		nr_cpu_ids = _ALIGN_UP(nr_cpu_ids, nthreads);
> +		pr_info("Aligned nr_cpus to SMT=%d, nr_cpu_ids = %d\n",
> +				 nthreads, nr_cpu_ids);
> +	}
> +
> +	/*
>  	 * Now see if any of these threads match our boot cpu.
>  	 * NOTE: This must match the parsing done in smp_setup_cpu_maps.
>  	 */
> @@ -352,7 +385,9 @@ static int __init early_init_dt_scan_cpus(unsigned long
> node,
>  	DBG("boot cpu: logical %d physical %d\n", found,
>  	    be32_to_cpu(intserv[found_thread]));
>  	boot_cpuid = found;
> -	set_hard_smp_processor_id(found, be32_to_cpu(intserv[found_thread]));
> +	adjust_boot_cpuid(nthreads, be32_to_cpu(intserv[found_thread]));
> +	set_hard_smp_processor_id(boot_cpuid,
> +					be32_to_cpu(intserv[found_thread]));
>
>  	/*
>  	 * PAPR defines "logical" PVR values for cpus that
> diff --git a/arch/powerpc/kernel/setup-common.c
> b/arch/powerpc/kernel/setup-common.c
> index 44c8d03..93c7e5f 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -77,6 +77,7 @@ struct machdep_calls *machine_id;
>  EXPORT_SYMBOL(machine_id);
>
>  int boot_cpuid = -1;
> +int boot_hw_cpuid = -1;
>  EXPORT_SYMBOL_GPL(boot_cpuid);
>
>  unsigned long klimit = (unsigned long) _end;
> @@ -444,6 +445,7 @@ void __init smp_setup_cpu_maps(void)
>  	struct device_node *dn = NULL;
>  	int cpu = 0;
>  	int nthreads = 1;
> +	bool boot_cpu_added = false;
>
>  	DBG("smp_setup_cpu_maps()\n");
>
> @@ -470,6 +472,24 @@ void __init smp_setup_cpu_maps(void)
>  		}
>
>  		nthreads = len / sizeof(int);
> +		/*
> +		 * If boot cpu hasn't been added to paca and there are only
> +		 * last nthreads slots available in paca array then wait
> +		 * for boot cpu to show up.
> +		 */
> +		if (!boot_cpu_added && (cpu + nthreads) >= nr_cpu_ids) {
> +			int found = 0;
> +
> +			DBG("Holding last nthreads paca slots for boot cpu\n");
> +			for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) {
> +				if (boot_hw_cpuid == be32_to_cpu(intserv[j])) {
> +					found = 1;
> +					break;
> +				}
> +			}
> +			if (!found)
> +				continue;
> +		}
>
>  		for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) {
>  			bool avail;
> @@ -485,6 +505,11 @@ void __init smp_setup_cpu_maps(void)
>  			set_cpu_present(cpu, avail);
>  			set_hard_smp_processor_id(cpu, be32_to_cpu(intserv[j]));
>  			set_cpu_possible(cpu, true);
> +			if (boot_hw_cpuid == be32_to_cpu(intserv[j])) {
> +				DBG("Boot cpu %d (hard id %d) added to paca\n",
> +				    cpu, be32_to_cpu(intserv[j]));
> +				boot_cpu_added = true;
> +			}
>  			cpu++;
>  		}
>  	}
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] ppc64 boot: Wait for boot cpu to show up if nr_cpus limit is about to hit.

[Mahesh Jagannath Salgaonkar]

On 02/02/2016 07:28 PM, Denis Kirjanov wrote:
> On 2/1/16, Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> The kernel boot parameter 'nr_cpus=' allows one to specify number of
>> possible cpus in the system. In the normal scenario the first cpu (cpu0)
>> that shows up is the boot cpu and hence it gets covered under nr_cpus
>> limit.
>>
>> But this assumption will be broken in kdump scenario where kdump kenrel
>> after a crash can boot up on an non-zero boot cpu. The paca structure
>> allocation depends on value of nr_cpus and is indexed using logical cpu
>> ids. This definetly will be an issue if boot cpu id > nr_cpus
> And what happend in this case? Have you tried it out?

Yes I have. It results into memory corruption when
set_hard_smp_processor_id(boot_cpu_id,..) is called from
early_init_dt_scan_cpus() and then kernel fails to boot. Nothing shows
up in console and system hangs forever.

You can easily reproduce this by configuring kdump service to use
'nr_cpus=1' instead of 'maxcpus=1' and then trigger system crash from
any cpu other than 0

e.g.  $ taskset -c 10 echo c > /proc/sysrq-trigger

Thanks,
-Mahesh.