mm/cgroup: use N_MEMORY instead of N_HIGH_MEMORY

PATCH In review Jianguo Wu 4 participants 6 messages Aug 30, 2013 2 messages not in the archive

Subsystems (2)

Memory management cgroup

lore

Share & tools

lore

Jianguo Wu

author

2013-08-30 03:44 UTC #0 lore raw

PATCH 5/5 mm/cgroup: use N_MEMORY instead of N_HIGH_MEMORY

Since commit 8219fc48a(mm: node_states: introduce N_MEMORY),
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 <[email protected]>

---
 mm/page_cgroup.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

mm/page_cgroup.c +1 -1

index 6d757e3..f6f7603 100644
@@ -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);

Signature · 1 line

1.7.1

Jianguo Wu

author

2013-08-30 04:03 UTC #1 lore raw

Quoted 8 lines · Jianguo Wu

Since commit 8219fc48a(mm: node_states: introduce N_MEMORY),
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 <[email protected]>

Sorry, it's should be "Signed-off-by: Jianguo Wu <[email protected]>"

Quoted 17 lines

---
 mm/page_cgroup.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

mm/page_cgroup.c +1 -1

index 6d757e3..f6f7603 100644
@@ -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);

Michal Hocko

2013-08-30 07:41 UTC #2 lore raw

In reply to Jianguo Wu

Quoted 1 line · Jianguo Wu

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.

Quoted 28 lines

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 <[email protected]>
---
 mm/page_cgroup.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
mm/page_cgroup.c +1 -1
index 6d757e3..f6f7603 100644
@@ -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);
Signature · 1 line
1.7.1

Signature · 2 lines

Michal Hocko
SUSE Labs

Jianguo Wu

author

2013-08-30 08:29 UTC #3 lore raw

Quoted 17 lines · Michal Hocko

Quoted 1 line · Jianguo Wu
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);

Quoted 6 lines

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,

Hi Michal,

You are right, here we need normal or high memory, but not movable memory, so N_HIGH_MEMORY should be kept here, the same as other patches, please drop this series.

Thank you for your point out.

Thanks,
Jianguo Wu.

Quoted 29 lines

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.
Quoted 19 lines
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 <[email protected]>
---
 mm/page_cgroup.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
mm/page_cgroup.c +0 -0
index 6d757e3..f6f7603 100644
@@ -116,7 +116,7 @@ static void *__meminit alloc_page_cgroup(size_t size, int nid)
 		return addr;
 	}
 

Quoted 10 lines

Quoted 9 lines

mm/page_cgroup.c +1 -1

-	if (node_state(nid, N_HIGH_MEMORY))
+	if (node_state(nid, N_MEMORY))
 		addr = vzalloc_node(size, nid);
 	else
 		addr = vzalloc(size);

Signature · 1 line

1.7.1

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jianguo Wu

2013-08-30 08:29 UTC #5 lore raw

Quoted 17 lines · Michal Hocko

Quoted 1 line · Jianguo Wu
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);

Quoted 6 lines

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,

Hi Michal,

You are right, here we need normal or high memory, but not movable memory, so N_HIGH_MEMORY should be kept here, the same as other patches, please drop this series.

Thank you for your point out.

Thanks,
Jianguo Wu.

Quoted 39 lines

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.
Quoted 28 lines
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 <[email protected]>
---
 mm/page_cgroup.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
mm/page_cgroup.c +1 -1
index 6d757e3..f6f7603 100644
@@ -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);
Signature · 1 line
1.7.1

A message that is not in the archive (sent off-list or not yet received): [email protected]

Michal Hocko

2013-08-30 07:41 UTC #7 lore raw

Quoted 1 line · Jianguo Wu

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);

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.

Quoted 28 lines

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 <[email protected]>
---
 mm/page_cgroup.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
mm/page_cgroup.c +1 -1
index 6d757e3..f6f7603 100644
@@ -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);
Signature · 1 line
1.7.1

Signature · 2 lines

Michal Hocko
SUSE Labs