[PATCH v2] smpboot: Check for successfull allocation of cpumask vars

[PATCH v2] smpboot: Check for successfull allocation of cpumask vars

[Pranith Kumar]

zalloc_cpumask_var() can return 0 on allocation failure when
CONFIG_CPUMASK_OFFSTACK is set. Check for the return value and warn on failure
of an allocation in such cases. Also remove the cpus from cpu masks so
that they are not accessed later on.

Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
 arch/x86/kernel/smpboot.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 7a8f584..35bc3f1 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1083,6 +1083,7 @@ static void __init smp_cpu_index_default(void)
 void __init native_smp_prepare_cpus(unsigned int max_cpus)
 {
 	unsigned int i;
+	bool ret = true;
 
 	preempt_disable();
 	smp_cpu_index_default();
@@ -1096,9 +1097,23 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
 
 	current_thread_info()->cpu = 0;  /* needed? */
 	for_each_possible_cpu(i) {
-		zalloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
-		zalloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
-		zalloc_cpumask_var(&per_cpu(cpu_llc_shared_map, i), GFP_KERNEL);
+		ret &= zalloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
+		ret &= zalloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
+		ret &= zalloc_cpumask_var(&per_cpu(cpu_llc_shared_map, i), GFP_KERNEL);
+
+		if (!ret) {
+			/* cpumask allocation failed, remove this and next cpus from
+			 * possible/present/online/active masks
+			 */
+			pr_warn("cpumask allocation failed!\n");
+			for (; i < nr_cpu_ids; i = cpumask_next(i, &cpu_possible_mask)) {
+				set_cpu_possible(i, false);
+				set_cpu_active(i, false);
+				set_cpu_online(i, false);
+				set_cpu_present(i, false);
+			}
+			break;
+		}
 	}
 	set_cpu_sibling_map(0);
 
-- 
1.9.1

Re: [PATCH v2] smpboot: Check for successfull allocation of cpumask vars

[David Rientjes]

On Tue, 9 Dec 2014, Pranith Kumar wrote:

> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 7a8f584..35bc3f1 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1083,6 +1083,7 @@ static void __init smp_cpu_index_default(void)
>  void __init native_smp_prepare_cpus(unsigned int max_cpus)
>  {
>  	unsigned int i;
> +	bool ret = true;

It's better to put this in the scope of the for_each_possible_cpu() loop 
so we don't implicitly rely on the fact that we always assume this to 
remain true from the previous iteration if the implementation subsequently 
changes.

>  
>  	preempt_disable();
>  	smp_cpu_index_default();
> @@ -1096,9 +1097,23 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
>  
>  	current_thread_info()->cpu = 0;  /* needed? */
>  	for_each_possible_cpu(i) {
> -		zalloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
> -		zalloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
> -		zalloc_cpumask_var(&per_cpu(cpu_llc_shared_map, i), GFP_KERNEL);
> +		ret &= zalloc_cpumask_var(&per_cpu(cpu_sibling_map, i), GFP_KERNEL);
> +		ret &= zalloc_cpumask_var(&per_cpu(cpu_core_map, i), GFP_KERNEL);
> +		ret &= zalloc_cpumask_var(&per_cpu(cpu_llc_shared_map, i), GFP_KERNEL);
> +
> +		if (!ret) {
> +			/* cpumask allocation failed, remove this and next cpus from
> +			 * possible/present/online/active masks
> +			 */

Please read Documentation/CodingStyle.

> +			pr_warn("cpumask allocation failed!\n");

This is unnecessary since zalloc_cpumask_var() failure will already report 
this incident and provide a stacktrace.

> +			for (; i < nr_cpu_ids; i = cpumask_next(i, &cpu_possible_mask)) {
> +				set_cpu_possible(i, false);
> +				set_cpu_active(i, false);
> +				set_cpu_online(i, false);
> +				set_cpu_present(i, false);
> +			}
> +			break;

Looks like a memory leak of some maps that may have been successfully 
allocated.

> +		}
>  	}
>  	set_cpu_sibling_map(0);
>