linux-kernel - Re: [PATCH 5/5] mm/cgroup: use N_MEMORY instead of N_HIGH

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: <20130830074152.GA28658@dhcp22.suse.cz>
Date:	Fri, 30 Aug 2013 09:41:52 +0200
From:	Michal Hocko <mhocko@...e.cz>
To:	Jianguo Wu <wujianguo@...wei.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Li Zefan <lizefan@...wei.com>,
	Johannes Weiner <hannes@...xchg.org>, tj@...nel.org,
	laijs@...fujitsu.com, cgroups@...r.kernel.org,
	containers@...ts.linux-foundation.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, Wen Congyang <wency@...fujitsu.com>
Subject: Re: [PATCH 5/5] mm/cgroup: use N_MEMORY instead of N_HIGH_MEMORY

On Fri 30-08-13 11:44:57, Jianguo Wu wrote:
> Since commit 8219fc48a(mm: node_states: introduce N_MEMORY),

But this very same commit also says:
"
    A.example 2) mm/page_cgroup.c use N_HIGH_MEMORY twice:
    
        One is in page_cgroup_init(void):
                for_each_node_state(nid, N_HIGH_MEMORY) {
    
        It means if the node have memory, we will allocate page_cgroup map for
        the node. We should use N_MEMORY instead here to gaim more clearly.
    
        The second using is in alloc_page_cgroup():
                if (node_state(nid, N_HIGH_MEMORY))
                        addr = vzalloc_node(size, nid);
    
        It means if the node has high or normal memory that can be allocated
        from kernel. We should keep N_HIGH_MEMORY here, and it will be better
        if the "any memory" semantic of N_HIGH_MEMORY is removed.
"

Which to me sounds like N_HIGH_MEMORY should be kept here. To be honest,
the distinction is not entirely clear to me. It was supposed to make
code cleaner but it apparently causes confusion.

It would also help if you CCed Lai Jiangshan who has introduced this
distinction. CCed now.

I wasn't CCed on the rest of the series but if you do the same
conversion, please make sure that this is not the case for others as
well.

> we introduced N_MEMORY, now N_MEMORY stands for the nodes that has any memory,
> and N_HIGH_MEMORY stands for the nodes that has normal or high memory.
> 
> The code here need to handle with the nodes which have memory,
> we should use N_MEMORY instead.
> 
> Signed-off-by: Xishi Qiu <qiuxishi@...wei.com>
> ---
>  mm/page_cgroup.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
> index 6d757e3..f6f7603 100644
> --- a/mm/page_cgroup.c
> +++ b/mm/page_cgroup.c
> @@ -116,7 +116,7 @@ static void *__meminit alloc_page_cgroup(size_t size, int nid)
>  		return addr;
>  	}
>  
> -	if (node_state(nid, N_HIGH_MEMORY))
> +	if (node_state(nid, N_MEMORY))
>  		addr = vzalloc_node(size, nid);
>  	else
>  		addr = vzalloc(size);
> -- 
> 1.7.1
> 
> 

-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/