Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751475AbdFZPcQ (ORCPT ); Mon, 26 Jun 2017 11:32:16 -0400 Received: from mail.skyhub.de ([5.9.137.197]:52512 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751101AbdFZPcL (ORCPT ); Mon, 26 Jun 2017 11:32:11 -0400 Date: Mon, 26 Jun 2017 17:31:49 +0200 From: Borislav Petkov To: Wei Yang Cc: kirill@shutemov.name, tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, tj@kernel.org, rientjes@google.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH V2 1/3] x86/numa_emulation: fix potential memory leak Message-ID: <20170626153149.b2x5pcipzuzaguuw@pd.tnic> References: <20170502130453.5933-1-richard.weiyang@gmail.com> <20170502130453.5933-2-richard.weiyang@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170502130453.5933-2-richard.weiyang@gmail.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1856 Lines: 58 On Tue, May 02, 2017 at 09:04:51PM +0800, Wei Yang wrote: > numa_emulation() needs to allocate a space for phys_dist[] temporarily, s/a // > while current code may miss to release this when dfl_phys_nid == > NUMA_NO_NODE. And when is "dfl_phys_nid == NUMA_NO_NODE"? What does it mean actually? > It is observed in code review instead of in a real case. > This patch fixes this by re-order the code path. > > Signed-off-by: Wei Yang > Acked-by: David Rientjes > --- > arch/x86/mm/numa_emulation.c | 36 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/mm/numa_emulation.c b/arch/x86/mm/numa_emulation.c > index a8f90ce3dedf..eb017c816de6 100644 > --- a/arch/x86/mm/numa_emulation.c > +++ b/arch/x86/mm/numa_emulation.c > @@ -353,6 +353,24 @@ void __init numa_emulation(struct numa_meminfo *numa_meminfo, int numa_dist_cnt) > goto no_emu; > } > > + /* > + * Determine the max emulated nid and the default phys nid to use > + * for unmapped nodes. > + */ > + max_emu_nid = 0; > + dfl_phys_nid = NUMA_NO_NODE; > + for (i = 0; i < ARRAY_SIZE(emu_nid_to_phys); i++) { > + if (emu_nid_to_phys[i] != NUMA_NO_NODE) { > + max_emu_nid = i; > + if (dfl_phys_nid == NUMA_NO_NODE) > + dfl_phys_nid = emu_nid_to_phys[i]; > + } > + } > + if (dfl_phys_nid == NUMA_NO_NODE) { > + pr_warn("NUMA: Warning: can't determine default physical node, disabling emulation\n"); > + goto no_emu; > + } > + Well, that function numa_emulation() does a looot of things and could very well be split into subfunctions, which should make the whole path more readable. And this chunk you're moving is kinda begging to be a separate function... -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.