2014-06-10 07:35:57

by Zhouyi Zhou

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

Fixing some memory allocation failure handling in x86 UV

Signed-off-by: Zhouyi Zhou <[email protected]>
---
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


2014-06-10 23:03:49

by H. Peter Anvin

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

On 06/10/2014 12:35 AM, Zhouyi Zhou wrote:
> Fixing some memory allocation failure handling in x86 UV
>
> Signed-off-by: Zhouyi Zhou <[email protected]>

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

-hpa

2014-06-10 23:27:10

by David Rientjes

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

On Tue, 10 Jun 2014, Zhouyi Zhou wrote:

> Fixing some memory allocation failure handling in x86 UV
>
> Signed-off-by: Zhouyi Zhou <[email protected]>
> ---
> 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;

2014-06-10 23:28:24

by Thomas Gleixner

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

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 <[email protected]>
>
> 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

2014-06-11 01:46:13

by Zhouyi Zhou

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

Thanks for reviewing, I will work on a new version

On Wed, Jun 11, 2014 at 7:28 AM, Thomas Gleixner <[email protected]> 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 <[email protected]>
>>
>> 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
>
>

2014-06-11 02:50:38

by Joe Perches

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

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