[V5 PATCH 08/26] memcontrol: use N_MEMORY instead N_HIGH_MEMORY

[V5 PATCH 08/26] memcontrol: use N_MEMORY instead N_HIGH_MEMORY

[Lai Jiangshan]

N_HIGH_MEMORY stands for the nodes that has normal or high memory.
N_MEMORY stands for the nodes that has any memory.

The code here need to handle with the nodes which have memory, we should
use N_MEMORY instead.

Signed-off-by: Lai Jiangshan <laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
 mm/memcontrol.c  |   18 +++++++++---------
 mm/page_cgroup.c |    2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7acf43b..1b69665 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -800,7 +800,7 @@ static unsigned long mem_cgroup_nr_lru_pages(struct mem_cgroup *memcg,
 	int nid;
 	u64 total = 0;
 
-	for_each_node_state(nid, N_HIGH_MEMORY)
+	for_each_node_state(nid, N_MEMORY)
 		total += mem_cgroup_node_nr_lru_pages(memcg, nid, lru_mask);
 	return total;
 }
@@ -1611,9 +1611,9 @@ static void mem_cgroup_may_update_nodemask(struct mem_cgroup *memcg)
 		return;
 
 	/* make a nodemask where this memcg uses memory from */
-	memcg->scan_nodes = node_states[N_HIGH_MEMORY];
+	memcg->scan_nodes = node_states[N_MEMORY];
 
-	for_each_node_mask(nid, node_states[N_HIGH_MEMORY]) {
+	for_each_node_mask(nid, node_states[N_MEMORY]) {
 
 		if (!test_mem_cgroup_node_reclaimable(memcg, nid, false))
 			node_clear(nid, memcg->scan_nodes);
@@ -1684,7 +1684,7 @@ static bool mem_cgroup_reclaimable(struct mem_cgroup *memcg, bool noswap)
 	/*
 	 * Check rest of nodes.
 	 */
-	for_each_node_state(nid, N_HIGH_MEMORY) {
+	for_each_node_state(nid, N_MEMORY) {
 		if (node_isset(nid, memcg->scan_nodes))
 			continue;
 		if (test_mem_cgroup_node_reclaimable(memcg, nid, noswap))
@@ -3759,7 +3759,7 @@ move_account:
 		drain_all_stock_sync(memcg);
 		ret = 0;
 		mem_cgroup_start_move(memcg);
-		for_each_node_state(node, N_HIGH_MEMORY) {
+		for_each_node_state(node, N_MEMORY) {
 			for (zid = 0; !ret && zid < MAX_NR_ZONES; zid++) {
 				enum lru_list lru;
 				for_each_lru(lru) {
@@ -4087,7 +4087,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
 
 	total_nr = mem_cgroup_nr_lru_pages(memcg, LRU_ALL);
 	seq_printf(m, "total=%lu", total_nr);
-	for_each_node_state(nid, N_HIGH_MEMORY) {
+	for_each_node_state(nid, N_MEMORY) {
 		node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid, LRU_ALL);
 		seq_printf(m, " N%d=%lu", nid, node_nr);
 	}
@@ -4095,7 +4095,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
 
 	file_nr = mem_cgroup_nr_lru_pages(memcg, LRU_ALL_FILE);
 	seq_printf(m, "file=%lu", file_nr);
-	for_each_node_state(nid, N_HIGH_MEMORY) {
+	for_each_node_state(nid, N_MEMORY) {
 		node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
 				LRU_ALL_FILE);
 		seq_printf(m, " N%d=%lu", nid, node_nr);
@@ -4104,7 +4104,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
 
 	anon_nr = mem_cgroup_nr_lru_pages(memcg, LRU_ALL_ANON);
 	seq_printf(m, "anon=%lu", anon_nr);
-	for_each_node_state(nid, N_HIGH_MEMORY) {
+	for_each_node_state(nid, N_MEMORY) {
 		node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
 				LRU_ALL_ANON);
 		seq_printf(m, " N%d=%lu", nid, node_nr);
@@ -4113,7 +4113,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
 
 	unevictable_nr = mem_cgroup_nr_lru_pages(memcg, BIT(LRU_UNEVICTABLE));
 	seq_printf(m, "unevictable=%lu", unevictable_nr);
-	for_each_node_state(nid, N_HIGH_MEMORY) {
+	for_each_node_state(nid, N_MEMORY) {
 		node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
 				BIT(LRU_UNEVICTABLE));
 		seq_printf(m, " N%d=%lu", nid, node_nr);
diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
index 5ddad0c..c1054ad 100644
--- a/mm/page_cgroup.c
+++ b/mm/page_cgroup.c
@@ -271,7 +271,7 @@ void __init page_cgroup_init(void)
 	if (mem_cgroup_disabled())
 		return;
 
-	for_each_node_state(nid, N_HIGH_MEMORY) {
+	for_each_node_state(nid, N_MEMORY) {
 		unsigned long start_pfn, end_pfn;
 
 		start_pfn = node_start_pfn(nid);
-- 
1.7.4.4

Re: [V5 PATCH 08/26] memcontrol: use N_MEMORY instead N_HIGH_MEMORY

[Michal Hocko]

On Mon 29-10-12 23:20:58, Lai Jiangshan wrote:
> N_HIGH_MEMORY stands for the nodes that has normal or high memory.
> N_MEMORY stands for the nodes that has any memory.

What is the difference of those two?

> The code here need to handle with the nodes which have memory, we should
> use N_MEMORY instead.
> 
> Signed-off-by: Lai Jiangshan <laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
> ---
>  mm/memcontrol.c  |   18 +++++++++---------
>  mm/page_cgroup.c |    2 +-
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 7acf43b..1b69665 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -800,7 +800,7 @@ static unsigned long mem_cgroup_nr_lru_pages(struct mem_cgroup *memcg,
>  	int nid;
>  	u64 total = 0;
>  
> -	for_each_node_state(nid, N_HIGH_MEMORY)
> +	for_each_node_state(nid, N_MEMORY)
>  		total += mem_cgroup_node_nr_lru_pages(memcg, nid, lru_mask);
>  	return total;
>  }
> @@ -1611,9 +1611,9 @@ static void mem_cgroup_may_update_nodemask(struct mem_cgroup *memcg)
>  		return;
>  
>  	/* make a nodemask where this memcg uses memory from */
> -	memcg->scan_nodes = node_states[N_HIGH_MEMORY];
> +	memcg->scan_nodes = node_states[N_MEMORY];
>  
> -	for_each_node_mask(nid, node_states[N_HIGH_MEMORY]) {
> +	for_each_node_mask(nid, node_states[N_MEMORY]) {
>  
>  		if (!test_mem_cgroup_node_reclaimable(memcg, nid, false))
>  			node_clear(nid, memcg->scan_nodes);
> @@ -1684,7 +1684,7 @@ static bool mem_cgroup_reclaimable(struct mem_cgroup *memcg, bool noswap)
>  	/*
>  	 * Check rest of nodes.
>  	 */
> -	for_each_node_state(nid, N_HIGH_MEMORY) {
> +	for_each_node_state(nid, N_MEMORY) {
>  		if (node_isset(nid, memcg->scan_nodes))
>  			continue;
>  		if (test_mem_cgroup_node_reclaimable(memcg, nid, noswap))
> @@ -3759,7 +3759,7 @@ move_account:
>  		drain_all_stock_sync(memcg);
>  		ret = 0;
>  		mem_cgroup_start_move(memcg);
> -		for_each_node_state(node, N_HIGH_MEMORY) {
> +		for_each_node_state(node, N_MEMORY) {
>  			for (zid = 0; !ret && zid < MAX_NR_ZONES; zid++) {
>  				enum lru_list lru;
>  				for_each_lru(lru) {
> @@ -4087,7 +4087,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
>  
>  	total_nr = mem_cgroup_nr_lru_pages(memcg, LRU_ALL);
>  	seq_printf(m, "total=%lu", total_nr);
> -	for_each_node_state(nid, N_HIGH_MEMORY) {
> +	for_each_node_state(nid, N_MEMORY) {
>  		node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid, LRU_ALL);
>  		seq_printf(m, " N%d=%lu", nid, node_nr);
>  	}
> @@ -4095,7 +4095,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
>  
>  	file_nr = mem_cgroup_nr_lru_pages(memcg, LRU_ALL_FILE);
>  	seq_printf(m, "file=%lu", file_nr);
> -	for_each_node_state(nid, N_HIGH_MEMORY) {
> +	for_each_node_state(nid, N_MEMORY) {
>  		node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
>  				LRU_ALL_FILE);
>  		seq_printf(m, " N%d=%lu", nid, node_nr);
> @@ -4104,7 +4104,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
>  
>  	anon_nr = mem_cgroup_nr_lru_pages(memcg, LRU_ALL_ANON);
>  	seq_printf(m, "anon=%lu", anon_nr);
> -	for_each_node_state(nid, N_HIGH_MEMORY) {
> +	for_each_node_state(nid, N_MEMORY) {
>  		node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
>  				LRU_ALL_ANON);
>  		seq_printf(m, " N%d=%lu", nid, node_nr);
> @@ -4113,7 +4113,7 @@ static int memcg_numa_stat_show(struct cgroup *cont, struct cftype *cft,
>  
>  	unevictable_nr = mem_cgroup_nr_lru_pages(memcg, BIT(LRU_UNEVICTABLE));
>  	seq_printf(m, "unevictable=%lu", unevictable_nr);
> -	for_each_node_state(nid, N_HIGH_MEMORY) {
> +	for_each_node_state(nid, N_MEMORY) {
>  		node_nr = mem_cgroup_node_nr_lru_pages(memcg, nid,
>  				BIT(LRU_UNEVICTABLE));
>  		seq_printf(m, " N%d=%lu", nid, node_nr);
> diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
> index 5ddad0c..c1054ad 100644
> --- a/mm/page_cgroup.c
> +++ b/mm/page_cgroup.c
> @@ -271,7 +271,7 @@ void __init page_cgroup_init(void)
>  	if (mem_cgroup_disabled())
>  		return;
>  
> -	for_each_node_state(nid, N_HIGH_MEMORY) {
> +	for_each_node_state(nid, N_MEMORY) {
>  		unsigned long start_pfn, end_pfn;
>  
>  		start_pfn = node_start_pfn(nid);
> -- 
> 1.7.4.4
> 

-- 
Michal Hocko
SUSE Labs

Re: [V5 PATCH 08/26] memcontrol: use N_MEMORY instead N_HIGH_MEMORY

[David Rientjes]

On Mon, 29 Oct 2012, Michal Hocko wrote:

> > N_HIGH_MEMORY stands for the nodes that has normal or high memory.
> > N_MEMORY stands for the nodes that has any memory.
> 
> What is the difference of those two?
> 

Patch 5 in the series introduces it to be equal to N_HIGH_MEMORY, so 
accepting this patch would be an implicit ack of the direction taken 
there.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [V5 PATCH 08/26] memcontrol: use N_MEMORY instead N_HIGH_MEMORY

[Michal Hocko]

On Mon 29-10-12 13:40:39, David Rientjes wrote:
> On Mon, 29 Oct 2012, Michal Hocko wrote:
> 
> > > N_HIGH_MEMORY stands for the nodes that has normal or high memory.
> > > N_MEMORY stands for the nodes that has any memory.
> > 
> > What is the difference of those two?
> > 
> 
> Patch 5 in the series 

Strange, I do not see that one at the mailing list.

> introduces it to be equal to N_HIGH_MEMORY, so 

So this is just a rename? If yes it would be much esier if it was
mentioned in the patch description.

> accepting this patch would be an implicit ack of the direction taken 
> there.

-- 
Michal Hocko
SUSE Labs

Re: [V5 PATCH 08/26] memcontrol: use N_MEMORY instead N_HIGH_MEMORY

[David Rientjes]

On Mon, 29 Oct 2012, Michal Hocko wrote:

> > > > N_HIGH_MEMORY stands for the nodes that has normal or high memory.
> > > > N_MEMORY stands for the nodes that has any memory.
> > > 
> > > What is the difference of those two?
> > > 
> > 
> > Patch 5 in the series 
> 
> Strange, I do not see that one at the mailing list.
> 

http://marc.info/?l=linux-kernel&m=135152595827692

> > introduces it to be equal to N_HIGH_MEMORY, so 
> 
> So this is just a rename? If yes it would be much esier if it was
> mentioned in the patch description.
> 

It's not even a rename even though it should be, it's adding yet another 
node_states that is equal to N_HIGH_MEMORY since that state already 
includes all memory.  It's just a matter of taste but I think we should be 
renaming it instead of aliasing it (unless you actually want to make 
N_HIGH_MEMORY only include nodes with highmem, but nothing depends on 
that).

Re: [V5 PATCH 08/26] memcontrol: use N_MEMORY instead N_HIGH_MEMORY

[Michal Hocko]

On Mon 29-10-12 14:08:05, David Rientjes wrote:
> On Mon, 29 Oct 2012, Michal Hocko wrote:
> 
> > > > > N_HIGH_MEMORY stands for the nodes that has normal or high memory.
> > > > > N_MEMORY stands for the nodes that has any memory.
> > > > 
> > > > What is the difference of those two?
> > > > 
> > > 
> > > Patch 5 in the series 
> > 
> > Strange, I do not see that one at the mailing list.
> > 
> 
> http://marc.info/?l=linux-kernel&m=135152595827692

Thanks!

> > > introduces it to be equal to N_HIGH_MEMORY, so 
> > 
> > So this is just a rename? If yes it would be much esier if it was
> > mentioned in the patch description.
> > 
> 
> It's not even a rename even though it should be, it's adding yet another 
> node_states that is equal to N_HIGH_MEMORY since that state already 
> includes all memory.  

Which is really strange because I do not see any reason for yet another
alias if the follow up patches rename all of them (I didn't try to apply
the whole series to check that so I might be wrong here).

> It's just a matter of taste but I think we should be renaming it
> instead of aliasing it (unless you actually want to make N_HIGH_MEMORY
> only include nodes with highmem, but nothing depends on that).

Agreed, I've always considered N_HIGH_MEMORY misleading and confusing so
renaming it would really make a lot of sense to me.
-- 
Michal Hocko
SUSE Labs

Re: [V5 PATCH 08/26] memcontrol: use N_MEMORY instead N_HIGH_MEMORY

[Michal Hocko]

On Wed 31-10-12 15:03:36, Wen Congyang wrote:
> At 10/30/2012 04:46 AM, David Rientjes Wrote:
> > On Mon, 29 Oct 2012, Lai Jiangshan wrote:
[...]
> >> 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.

OK, so the N_MOVABLE_MEMORY (or how you will call it) requires that all
the allocations will be migrateable?
How do you want to achieve that with the page_cgroup descriptors? (see
bellow)

On Mon 29-10-12 23:20:58, Lai Jiangshan wrote:
[...]
> diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
> index 5ddad0c..c1054ad 100644
> --- a/mm/page_cgroup.c
> +++ b/mm/page_cgroup.c
> @@ -271,7 +271,7 @@ void __init page_cgroup_init(void)
>  	if (mem_cgroup_disabled())
>  		return;
>  
> -	for_each_node_state(nid, N_HIGH_MEMORY) {
> +	for_each_node_state(nid, N_MEMORY) {
>  		unsigned long start_pfn, end_pfn;
>  
>  		start_pfn = node_start_pfn(nid);

This will call init_section_page_cgroup(pfn, nid) later which allocates
page_cgroup descriptors which are not movable. Or is there any code in
your patchset that handles this?
-- 
Michal Hocko
SUSE Labs