lkml.org 
[lkml]   [2012]   [Oct]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [V5 PATCH 05/26] node_states: introduce N_MEMORY
On Mon, 29 Oct 2012, Lai Jiangshan wrote:

> We have N_NORMAL_MEMORY for standing for the nodes that have normal memory with
> zone_type <= ZONE_NORMAL.
>
> And we have N_HIGH_MEMORY for standing for the nodes that have normal or high
> memory.
>

(In other words, all memory.)

> But we don't have any word to stand for the nodes that have *any* memory.
>

It's N_HIGH_MEMORY, or at least it's supposed to be. Is there a problem
where the bit isn't getting set for a node with memory?

> A) But this reusing is bad for *readability*. Because the name
> N_HIGH_MEMORY just stands for high or normal:
>
> A.example 1)
> mem_cgroup_nr_lru_pages():
> for_each_node_state(nid, N_HIGH_MEMORY)
>
> The user will be confused(why this function just counts for high or
> normal memory node? does it counts for ZONE_MOVABLE's lru pages?)
> until someone else tell them N_HIGH_MEMORY is reused to stand for
> nodes that have any memory.
>
> A.cont) If we introduce N_MEMORY, we can reduce this confusing
> AND make the code more clearly:
>
> 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.
>
> B) This reusing is out-dated if we introduce MOVABLE-dedicated node.
> The MOVABLE-dedicated node should not appear in
> node_stats[N_HIGH_MEMORY] nor node_stats[N_NORMAL_MEMORY],
> because MOVABLE-dedicated node has no high or normal memory.
>
> In x86_64, N_HIGH_MEMORY=N_NORMAL_MEMORY, if a MOVABLE-dedicated node
> is in node_stats[N_HIGH_MEMORY], it is also means it is in
> node_stats[N_NORMAL_MEMORY], it causes SLUB wrong.
>
> The slub uses
> for_each_node_state(nid, N_NORMAL_MEMORY)
> and creates kmem_cache_node for MOVABLE-dedicated node and cause problem.
>
> In one word, we need a N_MEMORY. We just intrude it as an alias to
> N_HIGH_MEMORY and fix all im-proper usages of N_HIGH_MEMORY in late patches.
>

If this is really that problematic (and it appears it's not given that
there are many use cases of it and people tend to get it right), then why
not simply rename N_HIGH_MEMORY instead of introducing yet another
nodemask to the equation?


\
 
 \ /
  Last update: 2012-10-30 00:21    [W:0.152 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site