Messages in this thread | | | Date | Wed, 31 Oct 2012 15:03:36 +0800 | From | Wen Congyang <> | Subject | Re: [V5 PATCH 05/26] node_states: introduce N_MEMORY |
| |
At 10/30/2012 04:46 AM, David Rientjes Wrote: > 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?
The reason is that we need a node which only contains movable memory. This feature is very important for node hotplug. So we will add a new nodemask for movable memory. N_MEMORY contains movable memory but N_HIGH_MEMORY doesn't contain it.
Thanks Wen Congyang
> -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
| |