linux-kernel - Re: [PATCH 2/6] change alloc function in pcpu_alloc

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]

Message-ID: <alpine.DEB.2.00.1004191238450.9855@router.home>
Date:	Mon, 19 Apr 2010 12:45:04 -0500 (CDT)
From:	Christoph Lameter <cl@...ux-foundation.org>
To:	Minchan Kim <minchan.kim@...il.com>
cc:	Tejun Heo <tj@...nel.org>, Mel Gorman <mel@....ul.ie>,
	Andrew Morton <akpm@...ux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Bob Liu <lliubbo@...il.com>, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org
Subject: Re: [PATCH 2/6] change alloc function in pcpu_alloc_pages

On Mon, 19 Apr 2010, Minchan Kim wrote:

> Let's tidy my table.
>
> I made quick patch to show the concept with one example of pci-dma.
> (Sorry but I attach patch since web gmail's mangling.)
>
> On UMA, we can change alloc_pages with
> alloc_pages_exact_node(numa_node_id(),....)
> (Actually, the patch is already merged mmotm)

UMA does not have the concept of nodes. Whatever node you specify is
irrelevant. Please remove the patch from mmotm.

> on NUMA, alloc_pages is some different meaning, so I don't want to change it.

No it has the same meaning. It means allocate a page.

> on NUMA, alloc_pages_node means _ANY_NODE_.

It means allocate on the indicated node if possible. Memory could come
from any node due to fallback (in order of node preference).

> So let's remove nid argument and change naming with alloc_pages_any_node.

??? What in the world are you doing?

> Then, whole users of alloc_pages_node can be changed between
> alloc_pages_exact_node and alloc_pages_any_node.
>
> It was my intention. What's your concern?

I dont see the point.

>  again:
> -       page = alloc_pages_node(dev_to_node(dev), flag, get_order(size));
> +       nid = dev_to_node(dev);
> +       /*
> +        * If pci-dma maintainer makes sure nid never has NUMA_NO_NODE
> +        * we can remove this ugly checking.
> +        */
> +       if (nid == NUMA_NO_NODE)
> +               page = alloc_pages_any_node(flag, get_order(size));

s/alloc_pages_any_node/alloc_pages/

> +       else
> +               page = alloc_pages_exact_node(nid, flag, get_order(size));

s/alloc_pages_exact_node/alloc_pages_node/

> -static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask,
> +static inline struct page *alloc_pagse_any_node(gfp_t gfp_mask,
>                                                 unsigned int order)
>  {
> -       /* Unknown node is current node */
> -       if (nid < 0)
> -               nid = numa_node_id();
> -
> +       int nid = numa_node_id();
>         return __alloc_pages(gfp_mask, order, node_zonelist(nid, gfp_mask));
>  }
>

This is very confusing. Because it is

	alloc_pages_numa_node_id()


alloca_pages_any_node suggests that the kernel randomly picks a node?

View attachment "change_alloc_functions_naming.patch" of type "TEXT/X-DIFF" (1844 bytes)