[PATCH] x86/tlb_uv: Fixing some memory allocation failure in x86 UV

[PATCH] x86/tlb_uv: Fixing some memory allocation failure in x86 UV

[Zhouyi Zhou]

Fixing some memory allocation failure handling in x86 UV

Signed-off-by: Zhouyi Zhou <yizhouzhou@ict.ac.cn>
---
 arch/x86/platform/uv/tlb_uv.c |   17 +++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
index dfe605a..a434768 100644
--- a/arch/x86/platform/uv/tlb_uv.c
+++ b/arch/x86/platform/uv/tlb_uv.c
@@ -1965,6 +1966,10 @@ static void make_per_cpu_thp(struct bau_control *smaster)
 	size_t hpsz = sizeof(struct hub_and_pnode) * num_possible_cpus();
 
 	smaster->thp = kmalloc_node(hpsz, GFP_KERNEL, smaster->osnode);
+	if (!smaster->thp) {
+		pr_err("ERROR: out of memory, can not create hub and pnode!\n");
+		return;
+	}
 	memset(smaster->thp, 0, hpsz);
 	for_each_present_cpu(cpu) {
 		smaster->thp[cpu].pnode = uv_cpu_hub_info(cpu)->pnode;
@@ -1980,6 +1985,8 @@ static void make_per_hub_cpumask(struct bau_control *hmaster)
 	int sz = sizeof(cpumask_t);
 
 	hmaster->cpumask = kzalloc_node(sz, GFP_KERNEL, hmaster->osnode);
+	if (!hmaster->cpumask)
+		pr_err("ERROR: out of memory, can not create cpumask!\n");
 }
 
 /*
@@ -2056,11 +2063,15 @@ static int __init summarize_uvhub_sockets(int nuvhubs,
 				if (scan_sock(sdp, bdp, &smaster, &hmaster))
 					return 1;
 				make_per_cpu_thp(smaster);
+				if (!smaster->thp)
+					return 1;
 			}
 			socket++;
 			socket_mask = (socket_mask >> 1);
 		}
 		make_per_hub_cpumask(hmaster);
+		if (!hmaster->cpumask)
+			return 1;
 	}
 	return 0;
 }
@@ -2077,9 +2088,16 @@ static int __init init_per_cpu(int nuvhubs, int base_part_pnode)
 	timeout_us = calculate_destination_timeout();
 
 	vp = kmalloc(nuvhubs * sizeof(struct uvhub_desc), GFP_KERNEL);
+	if (!vp)
+		return 1;
+
 	uvhub_descs = (struct uvhub_desc *)vp;
 	memset(uvhub_descs, 0, nuvhubs * sizeof(struct uvhub_desc));
 	uvhub_mask = kzalloc((nuvhubs+7)/8, GFP_KERNEL);
+	if (!uvhub_mask) {
+		kfree(uvhub_descs);
+		return 1;
+	}
 
 	if (get_cpu_topology(base_part_pnode, uvhub_descs, uvhub_mask))
 		goto fail;
-- 
1.7.10.4

Re: [PATCH] x86/tlb_uv: Fixing some memory allocation failure in x86 UV

[H. Peter Anvin]

On 06/10/2014 12:35 AM, Zhouyi Zhou wrote:
> Fixing some memory allocation failure handling in x86 UV
> 
> Signed-off-by: Zhouyi Zhou <yizhouzhou@ict.ac.cn>

Sorry, this really isn't enough description for this size of a patch.

	-hpa

Re: [PATCH] x86/tlb_uv: Fixing some memory allocation failure in x86 UV

[Thomas Gleixner]

On Tue, 10 Jun 2014, H. Peter Anvin wrote:

> On 06/10/2014 12:35 AM, Zhouyi Zhou wrote:
> > Fixing some memory allocation failure handling in x86 UV
> > 
> > Signed-off-by: Zhouyi Zhou <yizhouzhou@ict.ac.cn>
> 
> Sorry, this really isn't enough description for this size of a patch.

Correction: This is not a proper description for any patch.

"some" is wrong to begin with.

Either we fix a particular issue or we address all of them, but "some"
means: We fixed a few, but we did not care about the rest.

Aside of that, I agree. The changelog is disjunct from the patch
itself.

Thanks,

	tglx

Re: [PATCH] x86/tlb_uv: Fixing some memory allocation failure in x86 UV

[Zhouyi Zhou]

Thanks for reviewing, I will work on a new version

On Wed, Jun 11, 2014 at 7:28 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Tue, 10 Jun 2014, H. Peter Anvin wrote:
>
>> On 06/10/2014 12:35 AM, Zhouyi Zhou wrote:
>> > Fixing some memory allocation failure handling in x86 UV
>> >
>> > Signed-off-by: Zhouyi Zhou <yizhouzhou@ict.ac.cn>
>>
>> Sorry, this really isn't enough description for this size of a patch.
>
> Correction: This is not a proper description for any patch.
>
> "some" is wrong to begin with.
>
> Either we fix a particular issue or we address all of them, but "some"
> means: We fixed a few, but we did not care about the rest.
>
> Aside of that, I agree. The changelog is disjunct from the patch
> itself.
>
> Thanks,
>
>         tglx
>
>

Re: [PATCH] x86/tlb_uv: Fixing some memory allocation failure in x86 UV

[Joe Perches]

On Wed, 2014-06-11 at 09:46 +0800, Zhouyi Zhou wrote:
> Thanks for reviewing, I will work on a new version

If you do, please remove the "out of memory" messages.

These messages are redundant to a generic OOM and
stack dump from the memory subsystem.

Less code is also makes it less likely to have an OOM.

btw: I added a new checkpatch test based on your patch.

https://lkml.org/lkml/2014/6/10/382

Re: [PATCH] x86/tlb_uv: Fixing some memory allocation failure in x86 UV

[David Rientjes]

On Tue, 10 Jun 2014, Zhouyi Zhou wrote:

> Fixing some memory allocation failure handling in x86 UV
> 
> Signed-off-by: Zhouyi Zhou <yizhouzhou@ict.ac.cn>
> ---
>  arch/x86/platform/uv/tlb_uv.c |   17 +++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
> index dfe605a..a434768 100644
> --- a/arch/x86/platform/uv/tlb_uv.c
> +++ b/arch/x86/platform/uv/tlb_uv.c
> @@ -1965,6 +1966,10 @@ static void make_per_cpu_thp(struct bau_control *smaster)
>  	size_t hpsz = sizeof(struct hub_and_pnode) * num_possible_cpus();
>  
>  	smaster->thp = kmalloc_node(hpsz, GFP_KERNEL, smaster->osnode);
> +	if (!smaster->thp) {
> +		pr_err("ERROR: out of memory, can not create hub and pnode!\n");
> +		return;
> +	}
>  	memset(smaster->thp, 0, hpsz);
>  	for_each_present_cpu(cpu) {
>  		smaster->thp[cpu].pnode = uv_cpu_hub_info(cpu)->pnode;
> @@ -1980,6 +1985,8 @@ static void make_per_hub_cpumask(struct bau_control *hmaster)
>  	int sz = sizeof(cpumask_t);
>  
>  	hmaster->cpumask = kzalloc_node(sz, GFP_KERNEL, hmaster->osnode);
> +	if (!hmaster->cpumask)
> +		pr_err("ERROR: out of memory, can not create cpumask!\n");
>  }
>  
>  /*
> @@ -2056,11 +2063,15 @@ static int __init summarize_uvhub_sockets(int nuvhubs,
>  				if (scan_sock(sdp, bdp, &smaster, &hmaster))
>  					return 1;
>  				make_per_cpu_thp(smaster);
> +				if (!smaster->thp)
> +					return 1;
>  			}
>  			socket++;
>  			socket_mask = (socket_mask >> 1);
>  		}
>  		make_per_hub_cpumask(hmaster);
> +		if (!hmaster->cpumask)
> +			return 1;
>  	}
>  	return 0;
>  }

You're missing that make_per_hub_cpumask() isn't marked __init or freed 
after bootstrap so the correct fix would be to fold its implementation 
into summarize_uvhub_sockets() and return non-zero when the kzalloc_node() 
fails.

> @@ -2077,9 +2088,16 @@ static int __init init_per_cpu(int nuvhubs, int base_part_pnode)
>  	timeout_us = calculate_destination_timeout();
>  
>  	vp = kmalloc(nuvhubs * sizeof(struct uvhub_desc), GFP_KERNEL);

This should also be a kcalloc().

> +	if (!vp)
> +		return 1;
> +
>  	uvhub_descs = (struct uvhub_desc *)vp;
>  	memset(uvhub_descs, 0, nuvhubs * sizeof(struct uvhub_desc));
>  	uvhub_mask = kzalloc((nuvhubs+7)/8, GFP_KERNEL);
> +	if (!uvhub_mask) {
> +		kfree(uvhub_descs);
> +		return 1;
> +	}
>  
>  	if (get_cpu_topology(base_part_pnode, uvhub_descs, uvhub_mask))
>  		goto fail;