linux-kernel - Re: [PATCH v3 7/8] mips: numa: make node_to_cpumask_map() NUMA_NO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 15 Sep 2019 09:46:48 +0300
From:   Mike Rapoport <rppt@...ux.ibm.com>
To:     Yunsheng Lin <linyunsheng@...wei.com>
Cc:     catalin.marinas@....com, will@...nel.org, mingo@...hat.com,
        bp@...en8.de, rth@...ddle.net, ink@...assic.park.msu.ru,
        mattst88@...il.com, benh@...nel.crashing.org, paulus@...ba.org,
        mpe@...erman.id.au, heiko.carstens@...ibm.com, gor@...ux.ibm.com,
        borntraeger@...ibm.com, ysato@...rs.sourceforge.jp,
        dalias@...c.org, davem@...emloft.net, ralf@...ux-mips.org,
        paul.burton@...s.com, jhogan@...nel.org, jiaxun.yang@...goat.com,
        chenhc@...ote.com, akpm@...ux-foundation.org,
        anshuman.khandual@....com, tglx@...utronix.de, cai@....pw,
        robin.murphy@....com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, hpa@...or.com, x86@...nel.org,
        dave.hansen@...ux.intel.com, luto@...nel.org, peterz@...radead.org,
        len.brown@...el.com, axboe@...nel.dk, dledford@...hat.com,
        jeffrey.t.kirsher@...el.com, linux-alpha@...r.kernel.org,
        naveen.n.rao@...ux.vnet.ibm.com, mwb@...ux.vnet.ibm.com,
        linuxppc-dev@...ts.ozlabs.org, linux-s390@...r.kernel.org,
        linux-sh@...r.kernel.org, sparclinux@...r.kernel.org,
        tbogendoerfer@...e.de, linux-mips@...r.kernel.org,
        rafael@...nel.org, mhocko@...nel.org, gregkh@...uxfoundation.org
Subject: Re: [PATCH v3 7/8] mips: numa: make node_to_cpumask_map()
 NUMA_NO_NODE aware for mips

On Sun, Sep 15, 2019 at 02:13:51PM +0800, Yunsheng Lin wrote:
> On 2019/9/15 13:49, Mike Rapoport wrote:
> > Hi,
> > 
> > On Thu, Sep 12, 2019 at 06:15:33PM +0800, Yunsheng Lin wrote:
> >> When passing the return value of dev_to_node() to cpumask_of_node()
> >> without checking the node id if the node id is NUMA_NO_NODE, there is
> >> global-out-of-bounds detected by KASAN.
> >>
> >> From the discussion [1], NUMA_NO_NODE really means no node affinity,
> >> which also means all cpus should be usable. So the cpumask_of_node()
> >> should always return all cpus online when user passes the node id
> >> as NUMA_NO_NODE, just like similar semantic that page allocator handles
> >> NUMA_NO_NODE.
> >>
> >> But we cannot really copy the page allocator logic. Simply because the
> >> page allocator doesn't enforce the near node affinity. It just picks it
> >> up as a preferred node but then it is free to fallback to any other numa
> >> node. This is not the case here and node_to_cpumask_map will only restrict
> >> to the particular node's cpus which would have really non deterministic
> >> behavior depending on where the code is executed. So in fact we really
> >> want to return cpu_online_mask for NUMA_NO_NODE.
> >>
> >> Since this arch was already NUMA_NO_NODE aware, this patch only changes
> >> it to return cpu_online_mask and use NUMA_NO_NODE instead of "-1".
> >>
> >> [1] https://lore.kernel.org/patchwork/patch/1125789/
> >> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>
> >> Suggested-by: Michal Hocko <mhocko@...nel.org>
> >> ---
> >> V3: Change to only handle NUMA_NO_NODE, and return cpu_online_mask
> >>     for NUMA_NO_NODE case, and change the commit log to better justify
> >>     the change.
> >> ---
> >>  arch/mips/include/asm/mach-ip27/topology.h | 4 ++--
> > 
> > Nit: the subject says "mips:", but this patch only touches sgi-ip27 and
> > loongson is updated as a separate patch. I don't see why both patches
> > cannot be merged. Moreover, the whole set can be made as a single patch,
> > IMHO.
> 
> Thanks for reviewing.
> 
> As this patchset touches a few files, which may has different maintainer.
> I am not sure if a separate patch for different arch will make the merging
> process easy, or a single patch will make the merging process easy?

The set makes the same logical change to several definitions of
cpumask_of_node(). It's appropriate to have all these changes in a single
patch.
 
> It can be made as a single patch if a single patch will make the merging
> process easy.
> 
> > 
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/mips/include/asm/mach-ip27/topology.h b/arch/mips/include/asm/mach-ip27/topology.h
> >> index 965f079..04505e6 100644
> >> --- a/arch/mips/include/asm/mach-ip27/topology.h
> >> +++ b/arch/mips/include/asm/mach-ip27/topology.h
> >> @@ -15,8 +15,8 @@ struct cpuinfo_ip27 {
> >>  extern struct cpuinfo_ip27 sn_cpu_info[NR_CPUS];
> >>  
> >>  #define cpu_to_node(cpu)	(sn_cpu_info[(cpu)].p_nodeid)
> >> -#define cpumask_of_node(node)	((node) == -1 ?				\
> >> -				 cpu_all_mask :				\
> >> +#define cpumask_of_node(node)	((node) == NUMA_NO_NODE ?		\
> >> +				 cpu_online_mask :			\
> >>  				 &hub_data(node)->h_cpus)
> >>  struct pci_bus;
> >>  extern int pcibus_to_node(struct pci_bus *);
> >> -- 
> >> 2.8.1
> >>
> > 
> 

-- 
Sincerely yours,
Mike.