memory hotplug regression

-next New Heiko Carstens 2 participants 16 messages May 24, 2017 – Jun 1, 2017

Subsystems (3)

Memory management Driver core Memory hot(un)plug

lore

Share & tools

lore

Heiko Carstens

author

2017-05-24 08:20 UTC #0 lore raw

Hello Michal,

I just re-tested linux-next with respect to your memory hotplug changes and actually (finally) figured out that your patch ("mm, memory_hotplug: do not associate hotadded memory to zones until online)" changes behaviour on s390:

before your patch memory blocks that were offline and located behind the last online memory block were added by default to ZONE_MOVABLE:

# cat /sys/devices/system/memory/memory16/valid_zones
Movable Normal

With your patch this changes, so that they will be added to ZONE_NORMAL by default instead:

# cat /sys/devices/system/memory/memory16/valid_zones
Normal Movable

Sorry, that I didn't realize this earlier!

Having the ZONE_MOVABLE default was actually the only point why s390's arch_add_memory() was rather complex compared to other architectures.

We always had this behaviour, since we always wanted to be able to offline memory after it was brought online. Given that back then "online_movable" did not exist, the initial s390 memory hotplug support simply added all additional memory to ZONE_MOVABLE.

Keeping the default the same would be quite important.

FWIW, and a bit unrelated: we had/have very basic lsmem and chmem tools which can be used to list memory states and bring memory online and offline. These tools were part of the s390-tools package and only recently moved to util-linux.

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

Michal Hocko

2017-05-24 08:40 UTC #1 lore raw

Quoted 24 lines · Heiko Carstens

Hello Michal,
I just re-tested linux-next with respect to your memory hotplug changes and actually (finally) figured out that your patch ("mm, memory_hotplug: do not associate hotadded memory to zones until online)" changes behaviour on s390:
before your patch memory blocks that were offline and located behind the last online memory block were added by default to ZONE_MOVABLE:
# cat /sys/devices/system/memory/memory16/valid_zones
Movable Normal
With your patch this changes, so that they will be added to ZONE_NORMAL by default instead:
# cat /sys/devices/system/memory/memory16/valid_zones
Normal Movable
Sorry, that I didn't realize this earlier!
Having the ZONE_MOVABLE default was actually the only point why s390's arch_add_memory() was rather complex compared to other architectures.

Quoted 6 lines

We always had this behaviour, since we always wanted to be able to offline memory after it was brought online. Given that back then "online_movable" did not exist, the initial s390 memory hotplug support simply added all additional memory to ZONE_MOVABLE.
Keeping the default the same would be quite important.

Hmm, that is really unfortunate because I would _really_ like to get rid of the previous semantic which was really awkward. The whole point of the rework is to get rid of the nasty zone shifting.

Is it an option to use online_movable' rather than online' in your setup? Btw. my long term plan is to remove the zone range constrains altogether so you could online each memblock to the type you want. Would that be sufficient for you in general?

Signature · 8 lines

Michal Hocko
SUSE Labs

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

Heiko Carstens

author

2017-05-26 12:25 UTC #2 lore raw

Quoted 11 lines · Michal Hocko

Quoted 9 lines · Heiko Carstens
Having the ZONE_MOVABLE default was actually the only point why s390's arch_add_memory() was rather complex compared to other architectures.
We always had this behaviour, since we always wanted to be able to offline memory after it was brought online. Given that back then "online_movable" did not exist, the initial s390 memory hotplug support simply added all additional memory to ZONE_MOVABLE.
Keeping the default the same would be quite important.

Quoted 8 lines

Hmm, that is really unfortunate because I would _really_ like to get rid of the previous semantic which was really awkward. The whole point of the rework is to get rid of the nasty zone shifting.
Is it an option to use online_movable' rather than online' in your setup? Btw. my long term plan is to remove the zone range constrains altogether so you could online each memblock to the type you want. Would that be sufficient for you in general?

Why is it a problem to change the default for 'online'? As far as I can see that doesn't have too much to do with the order of zones, no?

By the way: we played around a bit with the changes wrt memory hotplug. There are a two odd things:

1) With the new code I can generate overlapping zones for ZONE_DMA and ZONE_NORMAL:

--- new code:

DMA [mem 0x0000000000000000-0x000000007fffffff]
Normal [mem 0x0000000080000000-0x000000017fffffff]

# cat /sys/devices/system/memory/block_size_bytes
10000000
# cat /sys/devices/system/memory/memory5/valid_zones
DMA
# echo 0 > /sys/devices/system/memory/memory5/online
# cat /sys/devices/system/memory/memory5/valid_zones
Normal
# echo 1 > /sys/devices/system/memory/memory5/online
Normal

# cat /proc/zoneinfo
Node 0, zone      DMA
spanned  524288        <-----
present  458752
managed  455078
start_pfn:           0 <-----

Node 0, zone Normal

spanned  720896
present  589824
managed  571648

start_pfn: 327680 <-----

So ZONE_DMA ends within ZONE_NORMAL. This shouldn't be possible, unless this restriction is gone?

--- old code:

# echo 0 > /sys/devices/system/memory/memory5/online
# cat /sys/devices/system/memory/memory5/valid_zones
DMA
# echo online_movable > /sys/devices/system/memory/memory5/state
-bash: echo: write error: Invalid argument
# echo online_kernel > /sys/devices/system/memory/memory5/state
-bash: echo: write error: Invalid argument
# echo online > /sys/devices/system/memory/memory5/state
# cat /sys/devices/system/memory/memory5/valid_zones
DMA

2) Another oddity is that after a memory block was brought online it's association to ZONE_NORMAL or ZONE_MOVABLE seems to be fixed. Even if it is brought offline afterwards:

# cat /sys/devices/system/memory/memory16/valid_zones
Normal Movable
# echo online_movable > /sys/devices/system/memory/memory16/state
# echo offline > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Movable  <---- should be "Normal Movable"

I assume this happens because start_pfn and spanned pages of the zones aren't updated if a memory block at the beginning or end of a zone is brought offline.

Michal Hocko

2017-05-29 08:52 UTC #3 lore raw

Quoted 16 lines · Heiko Carstens

Quoted 15 lines · Michal Hocko
Quoted 9 lines · Heiko Carstens
Having the ZONE_MOVABLE default was actually the only point why s390's arch_add_memory() was rather complex compared to other architectures.
We always had this behaviour, since we always wanted to be able to offline memory after it was brought online. Given that back then "online_movable" did not exist, the initial s390 memory hotplug support simply added all additional memory to ZONE_MOVABLE.
Keeping the default the same would be quite important.
Hmm, that is really unfortunate because I would _really_ like to get rid of the previous semantic which was really awkward. The whole point of the rework is to get rid of the nasty zone shifting.

Quoted 7 lines

Quoted 4 lines
Is it an option to use online_movable' rather than online' in your setup? Btw. my long term plan is to remove the zone range constrains altogether so you could online each memblock to the type you want. Would that be sufficient for you in general?
Why is it a problem to change the default for 'online'? As far as I can see that doesn't have too much to do with the order of zones, no?

`online' (aka MMOP_ONLINE_KEEP) should always inherit its current zone. The previous implementation made an exception to allow to shift to another zone if it is on the border of two zones. This is what I wanted to get rid of because it is just too ugly to live.

But now I am not really sure what is the usecase here. I assume you know how to online the memoery. That's why you had to play tricks with the zones previously. All you need now is to use the proper MMOP_ONLINE*

Quoted 11 lines

By the way: we played around a bit with the changes wrt memory hotplug. There are a two odd things:
1) With the new code I can generate overlapping zones for ZONE_DMA and ZONE_NORMAL:
--- new code:
DMA [mem 0x0000000000000000-0x000000007fffffff]
Normal [mem 0x0000000080000000-0x000000017fffffff]

Quoted 9 lines

# cat /sys/devices/system/memory/block_size_bytes
10000000
# cat /sys/devices/system/memory/memory5/valid_zones
DMA
# echo 0 > /sys/devices/system/memory/memory5/online
# cat /sys/devices/system/memory/memory5/valid_zones
Normal
# echo 1 > /sys/devices/system/memory/memory5/online
Normal

OK, interesting. I will double check the code.

Quoted 30 lines

# cat /proc/zoneinfo
Node 0, zone      DMA
spanned  524288        <-----
present  458752
managed  455078
start_pfn:           0 <-----

Node 0, zone Normal

spanned  720896
present  589824
managed  571648

start_pfn: 327680 <-----

So ZONE_DMA ends within ZONE_NORMAL. This shouldn't be possible, unless this restriction is gone?

--- old code:

# echo 0 > /sys/devices/system/memory/memory5/online
# cat /sys/devices/system/memory/memory5/valid_zones
DMA
# echo online_movable > /sys/devices/system/memory/memory5/state
-bash: echo: write error: Invalid argument
# echo online_kernel > /sys/devices/system/memory/memory5/state
-bash: echo: write error: Invalid argument
# echo online > /sys/devices/system/memory/memory5/state
# cat /sys/devices/system/memory/memory5/valid_zones
DMA

Quoted 3 lines

2) Another oddity is that after a memory block was brought online it's association to ZONE_NORMAL or ZONE_MOVABLE seems to be fixed. Even if it is brought offline afterwards:

This is intended behavior because I got rid of the tricky&ugly zone shifting code. Ultimately I would like to allow for overlapping zones so the explicit online_{movable,kernel} will _always_ work.

Signature · 8 lines

Michal Hocko
SUSE Labs

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

Heiko Carstens

author

2017-05-29 10:11 UTC #4 lore raw

Quoted 11 lines · Michal Hocko

Quoted 2 lines
Why is it a problem to change the default for 'online'? As far as I can see that doesn't have too much to do with the order of zones, no?
`online' (aka MMOP_ONLINE_KEEP) should always inherit its current zone. The previous implementation made an exception to allow to shift to another zone if it is on the border of two zones. This is what I wanted to get rid of because it is just too ugly to live.
But now I am not really sure what is the usecase here. I assume you know how to online the memoery. That's why you had to play tricks with the zones previously. All you need now is to use the proper MMOP_ONLINE*

Yes, however that implies that existing user space has to be changed to achieve the same semantics as before. That's the usecase I'm talking about.

On the other hand this change would finally make s390 behave like all other architectures, which is certainly not a bad thing. So, while thinking again I think you convinced me to agree with this change.

Quoted 7 lines

Quoted 3 lines
2) Another oddity is that after a memory block was brought online it's association to ZONE_NORMAL or ZONE_MOVABLE seems to be fixed. Even if it is brought offline afterwards:
This is intended behavior because I got rid of the tricky&ugly zone shifting code. Ultimately I would like to allow for overlapping zones so the explicit online_{movable,kernel} will _always_ work.

Ok, I see. This change (fixed memory block to zone mapping after first online) is a bit surprising. On the other hand I can't think of a sane usecase why one wants to change the zone a memory block belongs to.

Michal Hocko

2017-05-29 10:45 UTC #5 lore raw

Quoted 9 lines · Heiko Carstens

Quoted 8 lines · Michal Hocko
Quoted 2 lines
Why is it a problem to change the default for 'online'? As far as I can see that doesn't have too much to do with the order of zones, no?
`online' (aka MMOP_ONLINE_KEEP) should always inherit its current zone. The previous implementation made an exception to allow to shift to another zone if it is on the border of two zones. This is what I wanted to get rid of because it is just too ugly to live.

Quoted 6 lines

Quoted 3 lines
But now I am not really sure what is the usecase here. I assume you know how to online the memoery. That's why you had to play tricks with the zones previously. All you need now is to use the proper MMOP_ONLINE*
Yes, however that implies that existing user space has to be changed to achieve the same semantics as before. That's the usecase I'm talking about.

Yes that is really unfortunate. It is even more unfortunate how the original behavior got merged without a deeper consideration.

Quoted 3 lines

On the other hand this change would finally make s390 behave like all other architectures, which is certainly not a bad thing. So, while thinking again I think you convinced me to agree with this change.

That is definitely good to hear. Btw. I plan to change the semantic even further. MMOP_ONLINE_KEEP currently ignores movable_node setting and I plan to change that. Hopefully this won't break more userspace...

Quoted 11 lines

Quoted 7 lines
Quoted 3 lines
2) Another oddity is that after a memory block was brought online it's association to ZONE_NORMAL or ZONE_MOVABLE seems to be fixed. Even if it is brought offline afterwards:
This is intended behavior because I got rid of the tricky&ugly zone shifting code. Ultimately I would like to allow for overlapping zones so the explicit online_{movable,kernel} will _always_ work.
Ok, I see. This change (fixed memory block to zone mapping after first online) is a bit surprising. On the other hand I can't think of a sane usecase why one wants to change the zone a memory block belongs to.

Longeterm I would really like to remove any constrains on where to online movable or kernel memory. So even if this will be problem it will be only temporary.

Signature · 8 lines

Michal Hocko
SUSE Labs

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

Michal Hocko

2017-05-30 12:18 UTC #6 lore raw

In reply to Heiko Carstens

On Fri 26-05-17 14:25:09, Heiko Carstens wrote:
[...]

Quoted 25 lines

1) With the new code I can generate overlapping zones for ZONE_DMA and ZONE_NORMAL:
--- new code:
DMA [mem 0x0000000000000000-0x000000007fffffff]
Normal [mem 0x0000000080000000-0x000000017fffffff]
# cat /sys/devices/system/memory/block_size_bytes
10000000
# cat /sys/devices/system/memory/memory5/valid_zones
DMA
# echo 0 > /sys/devices/system/memory/memory5/online
# cat /sys/devices/system/memory/memory5/valid_zones
Normal
# echo 1 > /sys/devices/system/memory/memory5/online
Normal
# cat /proc/zoneinfo
Node 0, zone      DMA
spanned  524288        <-----
present  458752
managed  455078
start_pfn:           0 <-----

Quoted 8 lines

Node 0, zone Normal
spanned  720896
present  589824
managed  571648
start_pfn: 327680 <-----
So ZONE_DMA ends within ZONE_NORMAL. This shouldn't be possible, unless this restriction is gone?

The patch below should help.

Quoted 9 lines

--- old code:

# echo 0 > /sys/devices/system/memory/memory5/online
# cat /sys/devices/system/memory/memory5/valid_zones
DMA
# echo online_movable > /sys/devices/system/memory/memory5/state
-bash: echo: write error: Invalid argument
# echo online_kernel > /sys/devices/system/memory/memory5/state
-bash: echo: write error: Invalid argument

this error doesn't make any sense. Because we we want to online kernel memory and DMA is pretty much the kernel memory

Quoted 3 lines

# echo online > /sys/devices/system/memory/memory5/state
# cat /sys/devices/system/memory/memory5/valid_zones
DMA

---

Heiko Carstens

author

2017-05-30 12:37 UTC #7 lore raw

Quoted 4 lines · Michal Hocko

Quoted 2 lines
So ZONE_DMA ends within ZONE_NORMAL. This shouldn't be possible, unless this restriction is gone?
The patch below should help.

It does fix this specific problem, but introduces a new one:

# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Movable
# echo offline > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
          <--- no output

Memory block 16 is the only one I onlined and offlineto ZONE_MOVABLE.

Michal Hocko

2017-05-30 14:33 UTC #8 lore raw

Quoted 8 lines · Heiko Carstens

Quoted 4 lines · Michal Hocko
Quoted 2 lines
So ZONE_DMA ends within ZONE_NORMAL. This shouldn't be possible, unless this restriction is gone?
The patch below should help.
It does fix this specific problem, but introduces a new one:

Quoted 8 lines

# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Movable
# echo offline > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
          <--- no output

Memory block 16 is the only one I onlined and offlineto ZONE_MOVABLE.

Could you test the this on top please?
---

mm/memory_hotplug.c +6 -1

index 792c098e0e5f..a26f9f8e6365 100644
@@ -937,13 +937,18 @@ void __ref move_pfn_range_to_zone(struct zone *zone,
 	set_zone_contiguous(zone);
 }
 
+/*
+ * Returns a default kernel memory zone for the given pfn range.
+ * If no kernel zone covers this pfn range it will automatically go
+ * to the ZONE_NORMAL.
+ */
 struct zone *default_zone_for_pfn(int nid, unsigned long start_pfn,
 		unsigned long nr_pages)
 {
 	struct pglist_data *pgdat = NODE_DATA(nid);
 	int zid;
 
-	for (zid = 0; zid < MAX_NR_ZONES; zid++) {
+	for (zid = 0; zid <= ZONE_NORMAL; zid++) {
 		struct zone *zone = &pgdat->node_zones[zid];
 
 		if (zone_intersects(zone, start_pfn, nr_pages))

Signature · 8 lines

Michal Hocko
SUSE Labs

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

Heiko Carstens

author

2017-05-30 14:55 UTC #9 lore raw

Quoted 34 lines · Michal Hocko

Quoted 8 lines · Heiko Carstens
Quoted 4 lines · Michal Hocko
Quoted 2 lines
So ZONE_DMA ends within ZONE_NORMAL. This shouldn't be possible, unless this restriction is gone?
The patch below should help.
It does fix this specific problem, but introduces a new one:
Quoted 8 lines
# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Movable
# echo offline > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
          <--- no output
Memory block 16 is the only one I onlined and offlineto ZONE_MOVABLE.
Could you test the this on top please?
---
mm/memory_hotplug.c +5 -0
index 792c098e0e5f..a26f9f8e6365 100644
@@ -937,13 +937,18 @@ void __ref move_pfn_range_to_zone(struct zone *zone,
 	set_zone_contiguous(zone);
 }
 
+/*
+ * Returns a default kernel memory zone for the given pfn range.
+ * If no kernel zone covers this pfn range it will automatically go
+ * to the ZONE_NORMAL.
+ */
 struct zone *default_zone_for_pfn(int nid, unsigned long start_pfn,

mm/memory_hotplug.c void __ref move_pfn_range_to_zone(struct zone *zone, · line 954

Quoted 10 lines

mm/memory_hotplug.c +1 -1

 		unsigned long nr_pages)
 {
 	struct pglist_data *pgdat = NODE_DATA(nid);
 	int zid;
 
-	for (zid = 0; zid < MAX_NR_ZONES; zid++) {
+	for (zid = 0; zid <= ZONE_NORMAL; zid++) {
 		struct zone *zone = &pgdat->node_zones[zid];
 
 		if (zone_intersects(zone, start_pfn, nr_pages))

Still broken, but in different way(s):

# cat /sys/devices/system/memory/memory16/valid_zones
Normal Movable
# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Movable
# cat /sys/devices/system/memory/memory18/valid_zones
Movable
# echo online > /sys/devices/system/memory/memory18/state
# cat /sys/devices/system/memory/memory18/valid_zones
Normal    <--- should be Movable
# cat /sys/devices/system/memory/memory17/valid_zones
          <--- no output

Michal Hocko

2017-05-30 15:04 UTC #10 lore raw

Quoted 48 lines · Heiko Carstens

Quoted 34 lines · Michal Hocko
Quoted 8 lines · Heiko Carstens
Quoted 4 lines · Michal Hocko
Quoted 2 lines
So ZONE_DMA ends within ZONE_NORMAL. This shouldn't be possible, unless this restriction is gone?
The patch below should help.
It does fix this specific problem, but introduces a new one:
Quoted 8 lines
# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Movable
# echo offline > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
          <--- no output
Memory block 16 is the only one I onlined and offlineto ZONE_MOVABLE.
Could you test the this on top please?
---
mm/memory_hotplug.c +5 -0
index 792c098e0e5f..a26f9f8e6365 100644
@@ -937,13 +937,18 @@ void __ref move_pfn_range_to_zone(struct zone *zone,
 	set_zone_contiguous(zone);
 }
 
+/*
+ * Returns a default kernel memory zone for the given pfn range.
+ * If no kernel zone covers this pfn range it will automatically go
+ * to the ZONE_NORMAL.
+ */
 struct zone *default_zone_for_pfn(int nid, unsigned long start_pfn,
mm/memory_hotplug.c void __ref move_pfn_range_to_zone(struct zone *zone, · line 954
Quoted 10 lines
mm/memory_hotplug.c +1 -1
 		unsigned long nr_pages)
 {
 	struct pglist_data *pgdat = NODE_DATA(nid);
 	int zid;
 
-	for (zid = 0; zid < MAX_NR_ZONES; zid++) {
+	for (zid = 0; zid <= ZONE_NORMAL; zid++) {
 		struct zone *zone = &pgdat->node_zones[zid];
 
 		if (zone_intersects(zone, start_pfn, nr_pages))
Still broken, but in different way(s):

Quoted 12 lines

# cat /sys/devices/system/memory/memory16/valid_zones
Normal Movable
# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Movable
# cat /sys/devices/system/memory/memory18/valid_zones
Movable
# echo online > /sys/devices/system/memory/memory18/state
# cat /sys/devices/system/memory/memory18/valid_zones
Normal    <--- should be Movable
# cat /sys/devices/system/memory/memory17/valid_zones
          <--- no output

OK, I will sit on this tomorrow with a clean head without doing 10 things at the same time. Sorry about your wasted time!

Signature · 8 lines

Michal Hocko
SUSE Labs

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

Michal Hocko

2017-05-31 06:24 UTC #11 lore raw

In reply to Heiko Carstens

Quoted 48 lines · Heiko Carstens

Quoted 34 lines · Michal Hocko
Quoted 8 lines · Heiko Carstens
Quoted 4 lines · Michal Hocko
Quoted 2 lines
So ZONE_DMA ends within ZONE_NORMAL. This shouldn't be possible, unless this restriction is gone?
The patch below should help.
It does fix this specific problem, but introduces a new one:
Quoted 8 lines
# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Movable
# echo offline > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
          <--- no output
Memory block 16 is the only one I onlined and offlineto ZONE_MOVABLE.
Could you test the this on top please?
---
mm/memory_hotplug.c +5 -0
index 792c098e0e5f..a26f9f8e6365 100644
@@ -937,13 +937,18 @@ void __ref move_pfn_range_to_zone(struct zone *zone,
 	set_zone_contiguous(zone);
 }
 
+/*
+ * Returns a default kernel memory zone for the given pfn range.
+ * If no kernel zone covers this pfn range it will automatically go
+ * to the ZONE_NORMAL.
+ */
 struct zone *default_zone_for_pfn(int nid, unsigned long start_pfn,
mm/memory_hotplug.c void __ref move_pfn_range_to_zone(struct zone *zone, · line 954
Quoted 10 lines
mm/memory_hotplug.c +1 -1
 		unsigned long nr_pages)
 {
 	struct pglist_data *pgdat = NODE_DATA(nid);
 	int zid;
 
-	for (zid = 0; zid < MAX_NR_ZONES; zid++) {
+	for (zid = 0; zid <= ZONE_NORMAL; zid++) {
 		struct zone *zone = &pgdat->node_zones[zid];
 
 		if (zone_intersects(zone, start_pfn, nr_pages))
Still broken, but in different way(s):

Quoted 12 lines

# cat /sys/devices/system/memory/memory16/valid_zones
Normal Movable
# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Movable
# cat /sys/devices/system/memory/memory18/valid_zones
Movable
# echo online > /sys/devices/system/memory/memory18/state
# cat /sys/devices/system/memory/memory18/valid_zones
Normal    <--- should be Movable
# cat /sys/devices/system/memory/memory17/valid_zones
          <--- no output

OK, so this is an independent problem and an unrelated one to the patch I've posted. We need two patches actually. Damn, I hate MMOP_ONLINE_KEEP. I will send 2 patches as a reply to this email.

Signature · 8 lines

Michal Hocko
SUSE Labs

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

Michal Hocko

2017-05-31 06:25 UTC #12 lore raw

PATCH 1/2 mm, memory_hotplug: fix MMOP_ONLINE_KEEP behavior

From: Michal Hocko <[email protected]>

Heiko Carstens has noticed that the MMOP_ONLINE_KEEP is broken currently

$ grep . memory3?/valid_zones
memory34/valid_zones:Normal Movable
memory35/valid_zones:Normal Movable
memory36/valid_zones:Normal Movable
memory37/valid_zones:Normal Movable

$ echo online_movable > memory34/state
$ grep . memory3?/valid_zones
memory34/valid_zones:Movable
memory35/valid_zones:Movable
memory36/valid_zones:Movable
memory37/valid_zones:Movable

$ echo online > memory36/state
$ grep . memory3?/valid_zones
memory34/valid_zones:Movable
memory36/valid_zones:Normal
memory37/valid_zones:Movable

so we have effectivelly punched a hole into the movable zone. The problem is that move_pfn_range() check for MMOP_ONLINE_KEEP is wrong. It only checks whether the given range is already part of the movable zone which is not the case here as only memory34 is in the zone. Fix this by using allow_online_pfn_range(..., MMOP_ONLINE_KERNEL) if that is false then we can be sure that movable onlining is the right thing to do.

Reported-by Heiko Carstens <[email protected]>

Fixes "mm, memory_hotplug: do not associate hotadded memory to zones until online"

Signed-off-by Michal Hocko <[email protected]>

---
 mm/memory_hotplug.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

mm/memory_hotplug.c +5 -4

index 0a895df2397e..b3895fd609f4 100644
@@ -950,11 +950,12 @@ static struct zone * __meminit move_pfn_range(int online_type, int nid,
 	if (online_type == MMOP_ONLINE_KEEP) {
 		struct zone *movable_zone = &pgdat->node_zones[ZONE_MOVABLE];
 		/*
-		 * MMOP_ONLINE_KEEP inherits the current zone which is
-		 * ZONE_NORMAL by default but we might be within ZONE_MOVABLE
-		 * already.
+		 * MMOP_ONLINE_KEEP defaults to MMOP_ONLINE_KERNEL but use
+		 * movable zone if that is not possible (e.g. we are within
+		 * or past the existing movable zone)
 		 */
-		if (zone_intersects(movable_zone, start_pfn, nr_pages))
+		if (!allow_online_pfn_range(nid, start_pfn, nr_pages,
+					MMOP_ONLINE_KERNEL))
 			zone = movable_zone;
 	} else if (online_type == MMOP_ONLINE_MOVABLE) {
 		zone = &pgdat->node_zones[ZONE_MOVABLE];

Signature · 7 lines

2.11.0

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

Michal Hocko

2017-05-31 06:26 UTC #13 lore raw

PATCH 2/2 mm, memory_hotplug: do not assume ZONE_NORMAL is default kernel zone

In reply to Michal Hocko

From: Michal Hocko <[email protected]>

Heiko Carstens has noticed that he can generate overlapping zones for ZONE_DMA and ZONE_NORMAL:
DMA [mem 0x0000000000000000-0x000000007fffffff]
Normal [mem 0x0000000080000000-0x000000017fffffff]

$ cat /sys/devices/system/memory/block_size_bytes
10000000
$ cat /sys/devices/system/memory/memory5/valid_zones
DMA
$ echo 0 > /sys/devices/system/memory/memory5/online
$ cat /sys/devices/system/memory/memory5/valid_zones
Normal
$ echo 1 > /sys/devices/system/memory/memory5/online
Normal

$ cat /proc/zoneinfo
Node 0, zone      DMA
spanned  524288        <-----
present  458752
managed  455078
start_pfn:           0 <-----

Node 0, zone Normal

spanned  720896
present  589824
managed  571648

start_pfn: 327680 <-----

The reason is that we assume that the default zone for kernel onlining is ZONE_NORMAL. This was a simplification introduced by the memory hotplug rework and it is easily fixable by checking the range overlap in the zone order and considering the first matching zone as the default one. If there is no such zone then assume ZONE_NORMAL as we have been doing so far.

Fixes "mm, memory_hotplug: do not associate hotadded memory to zones until online"

Reported-by Heiko Carstens <[email protected]>

Signed-off-by Michal Hocko <[email protected]>

---
 drivers/base/memory.c          |  2 +-
 include/linux/memory_hotplug.h |  2 ++
 mm/memory_hotplug.c            | 27 ++++++++++++++++++++++++---
 3 files changed, 27 insertions(+), 4 deletions(-)

drivers/base/memory.c +1 -1

index b86fda30ce62..c7c4e0325cdb 100644
@@ -419,7 +419,7 @@ static ssize_t show_valid_zones(struct device *dev,
 
 	nid = pfn_to_nid(start_pfn);
 	if (allow_online_pfn_range(nid, start_pfn, nr_pages, MMOP_ONLINE_KERNEL)) {
-		strcat(buf, NODE_DATA(nid)->node_zones[ZONE_NORMAL].name);
+		strcat(buf, default_zone_for_pfn(nid, start_pfn, nr_pages)->name);
 		append = true;
 	}

include/linux/memory_hotplug.h +2 -0

index 9e0249d0f5e4..ed167541e4fc 100644
@@ -309,4 +309,6 @@ extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
 					  unsigned long pnum);
 extern bool allow_online_pfn_range(int nid, unsigned long pfn, unsigned long nr_pages,
 		int online_type);
+extern struct zone *default_zone_for_pfn(int nid, unsigned long pfn,
+		unsigned long nr_pages);
 #endif /* __LINUX_MEMORY_HOTPLUG_H */

mm/memory_hotplug.c +24 -3

index b3895fd609f4..a0348de3e18c 100644
@@ -858,7 +858,7 @@ bool allow_online_pfn_range(int nid, unsigned long pfn, unsigned long nr_pages,
 {
 	struct pglist_data *pgdat = NODE_DATA(nid);
 	struct zone *movable_zone = &pgdat->node_zones[ZONE_MOVABLE];
-	struct zone *normal_zone =  &pgdat->node_zones[ZONE_NORMAL];
+	struct zone *default_zone = default_zone_for_pfn(nid, pfn, nr_pages);
 
 	/*
 	 * TODO there shouldn't be any inherent reason to have ZONE_NORMAL

@@ -872,7 +872,7 @@ bool allow_online_pfn_range(int nid, unsigned long pfn, unsigned long nr_pages,
 			return true;
 		return movable_zone->zone_start_pfn >= pfn + nr_pages;
 	} else if (online_type == MMOP_ONLINE_MOVABLE) {
-		return zone_end_pfn(normal_zone) <= pfn;
+		return zone_end_pfn(default_zone) <= pfn;
 	}
 
 	/* MMOP_ONLINE_KEEP will always succeed and inherits the current zone */

@@ -938,6 +938,27 @@ void __ref move_pfn_range_to_zone(struct zone *zone,
 }
 
 /*
+ * Returns a default kernel memory zone for the given pfn range.
+ * If no kernel zone covers this pfn range it will automatically go
+ * to the ZONE_NORMAL.
+ */
+struct zone *default_zone_for_pfn(int nid, unsigned long start_pfn,
+		unsigned long nr_pages)
+{
+	struct pglist_data *pgdat = NODE_DATA(nid);
+	int zid;
+
+	for (zid = 0; zid <= ZONE_NORMAL; zid++) {
+		struct zone *zone = &pgdat->node_zones[zid];
+
+		if (zone_intersects(zone, start_pfn, nr_pages))
+			return zone;
+	}
+
+	return &pgdat->node_zones[ZONE_NORMAL];
+}
+
+/*
  * Associates the given pfn range with the given node and the zone appropriate
  * for the given online type.
  */

@@ -945,7 +966,7 @@ static struct zone * __meminit move_pfn_range(int online_type, int nid,
 		unsigned long start_pfn, unsigned long nr_pages)
 {
 	struct pglist_data *pgdat = NODE_DATA(nid);
-	struct zone *zone = &pgdat->node_zones[ZONE_NORMAL];
+	struct zone *zone = default_zone_for_pfn(nid, start_pfn, nr_pages);
 
 	if (online_type == MMOP_ONLINE_KEEP) {
 		struct zone *movable_zone = &pgdat->node_zones[ZONE_MOVABLE];

Signature · 7 lines

2.11.0

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

Heiko Carstens

author Tested-by

2017-06-01 06:50 UTC #14 lore raw

In reply to Michal Hocko

Quoted 13 lines · Michal Hocko

Quoted 4 lines

# cat /sys/devices/system/memory/memory16/valid_zones
Normal Movable
# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones

Quoted 8 lines

Movable
# cat /sys/devices/system/memory/memory18/valid_zones
Movable
# echo online > /sys/devices/system/memory/memory18/state
# cat /sys/devices/system/memory/memory18/valid_zones
Normal    <--- should be Movable
# cat /sys/devices/system/memory/memory17/valid_zones
          <--- no output

Quoted 3 lines

OK, so this is an independent problem and an unrelated one to the patch I've posted. We need two patches actually. Damn, I hate MMOP_ONLINE_KEEP. I will send 2 patches as a reply to this email.

Tested with your patches on top of linux-next as of yesterday, however starting at commit fa812e869a6fe7495a17150bb2639075081ef709 ("mm/zswap.c:
delete an error message for a failed memory allocation in zswap_dstmem_prepare()"), since the "mm: per-lruvec slab stats" patch series breaks everything ;)

Tested-by Heiko Carstens <[email protected]>

Michal Hocko

2017-06-01 07:13 UTC #15 lore raw

Quoted 18 lines · Heiko Carstens

Quoted 13 lines · Michal Hocko
Quoted 4 lines
# cat /sys/devices/system/memory/memory16/valid_zones
Normal Movable
# echo online_movable > /sys/devices/system/memory/memory16/state
# cat /sys/devices/system/memory/memory16/valid_zones
Quoted 8 lines
Movable
# cat /sys/devices/system/memory/memory18/valid_zones
Movable
# echo online > /sys/devices/system/memory/memory18/state
# cat /sys/devices/system/memory/memory18/valid_zones
Normal    <--- should be Movable
# cat /sys/devices/system/memory/memory17/valid_zones
          <--- no output
Quoted 3 lines
OK, so this is an independent problem and an unrelated one to the patch I've posted. We need two patches actually. Damn, I hate MMOP_ONLINE_KEEP. I will send 2 patches as a reply to this email.

Quoted 7 lines

Tested with your patches on top of linux-next as of yesterday, however starting at commit fa812e869a6fe7495a17150bb2639075081ef709 ("mm/zswap.c:
delete an error message for a failed memory allocation in zswap_dstmem_prepare()"), since the "mm: per-lruvec slab stats" patch series breaks everything ;)
Tested-by Heiko Carstens <[email protected]>

Thanks a lot for testing! I will post those patches for wider review later today.

Signature · 8 lines

Michal Hocko
SUSE Labs

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