memory-hotplug: fix not enough check of valid_zones
Subsystems (4)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 memory-hotplug: fix not enough check of valid_zones
As Yasuaki Ishimatsu described the check here is not enough if memory has hole as follows:
PFN 0x00 0xd0 0xe0 0xf0
+-------------+-------------+-------------+
zone type | Normal | hole | Normal |
+-------------+-------------+-------------+In this case, the check can't guarantee that this is "the last block of memory".
The check of ZONE_MOVABLE has the same problem.
Change the interface name to valid_zones according to most pepole's suggestion.
Sample output of the sysfs files:
memory0/valid_zones: none
memory1/valid_zones: DMA32
memory2/valid_zones: DMA32
memory3/valid_zones: DMA32
memory4/valid_zones: Normal
memory5/valid_zones: Normal
memory6/valid_zones: Normal Movable
memory7/valid_zones: Movable Normal
memory8/valid_zones: Movable
--- Documentation/ABI/testing/sysfs-devices-memory | 8 ++--- Documentation/memory-hotplug.txt | 4 +-- drivers/base/memory.c | 42 ++++++-------------------- 3 files changed, 15 insertions(+), 39 deletions(-)
Documentation/ABI/testing/sysfs-devices-memory +4 -4
@@ -61,13 +61,13 @@ Users: hotplug memory remove tools http://www.ibm.com/developerworks/wikis/display/LinuxP/powerpc-utils -What: /sys/devices/system/memory/memoryX/zones_online_to +What: /sys/devices/system/memory/memoryX/valid_zones Date: July 2014 Contact: Zhang Zhen <[email protected]> Description: - The file /sys/devices/system/memory/memoryX/zones_online_to - is read-only and is designed to show which zone this memory block can - be onlined to. + The file /sys/devices/system/memory/memoryX/valid_zones is + read-only and is designed to show which zone this memory + block can be onlined to. What: /sys/devices/system/memoryX/nodeY Date: October 2009
Documentation/memory-hotplug.txt +2 -2
@@ -155,7 +155,7 @@ Under each memory block, you can see 4 files: /sys/devices/system/memory/memoryXXX/phys_device /sys/devices/system/memory/memoryXXX/state /sys/devices/system/memory/memoryXXX/removable -/sys/devices/system/memory/memoryXXX/zones_online_to +/sys/devices/system/memory/memoryXXX/valid_zones 'phys_index' : read-only and contains memory block id, same as XXX. 'state' : read-write
@@ -171,7 +171,7 @@ Under each memory block, you can see 4 files: block is removable and a value of 0 indicates that it is not removable. A memory block is removable only if every section in the block is removable. -'zones_online_to' : read-only: designed to show which zone this memory block +'valid_zones' : read-only: designed to show which zone this memory block can be onlined to. NOTE:
drivers/base/memory.c +9 -33
70 lines not on this page · raw message
Signature · 4 lines
1.8.1.4 .
Quoted 14 lines · Zhang Zhen
As Yasuaki Ishimatsu described the check here is not enough if memory has hole as follows:
PFN
0x000xd00xe00xf0+-------------+-------------+-------------+ zone type | Normal | hole | Normal | +-------------+-------------+-------------+In this case, the check can't guarantee that this is "the last block of memory".
The check ofZONE_MOVABLEhas the same problem.Change the interface name to
valid_zonesaccording to most pepole's suggestion.
Quoted 10 lines
Sample output of the sysfs files:
memory0/valid_zones: none
memory1/valid_zones: DMA32
memory2/valid_zones: DMA32
memory3/valid_zones: DMA32
memory4/valid_zones: Normal
memory5/valid_zones: Normal
memory6/valid_zones: Normal Movable
memory7/valid_zones: Movable Normal
memory8/valid_zones: Movable
The patch has two changes:
- change sysfs interface name
- change check of ZONE_MOVABLE
So please separate them.
Quoted 101 lines
Signed-off-by Zhang Zhen <[email protected]>--- Documentation/ABI/testing/sysfs-devices-memory | 8 ++--- Documentation/memory-hotplug.txt | 4 +-- drivers/base/memory.c | 42 ++++++-------------------- 3 files changed, 15 insertions(+), 39 deletions(-)Documentation/ABI/testing/sysfs-devices-memory +4
-4@@ -61,13 +61,13 @@ Users: hotplug memory remove tools http://www.ibm.com/developerworks/wikis/display/LinuxP/powerpc-utils -What: /sys/devices/system/memory/memoryX/zones_online_to +What: /sys/devices/system/memory/memoryX/valid_zones Date: July 2014 Contact: Zhang Zhen <[email protected]> Description: - The file /sys/devices/system/memory/memoryX/zones_online_to - is read-only and is designed to show which zone this memory block can - be onlined to. + The file /sys/devices/system/memory/memoryX/valid_zones is + read-only and is designed to show which zone this memory + block can be onlined to. What: /sys/devices/system/memoryX/nodeY Date: October 2009Documentation/memory-hotplug.txt +2
-2@@ -155,7 +155,7 @@ Under each memory block, you can see 4 files: /sys/devices/system/memory/memoryXXX/phys_device /sys/devices/system/memory/memoryXXX/state /sys/devices/system/memory/memoryXXX/removable -/sys/devices/system/memory/memoryXXX/zones_online_to +/sys/devices/system/memory/memoryXXX/valid_zones 'phys_index' : read-only and contains memory block id, same as XXX. 'state' : read-write@@ -171,7 +171,7 @@ Under each memory block, you can see 4 files: block is removable and a value of 0 indicates that it is not removable. A memory block is removable only if every section in the block is removable. -'zones_online_to' : read-only: designed to show which zone this memory block +'valid_zones' : read-only: designed to show which zone this memory block can be onlined to. NOTE:drivers/base/memory.c +4
-26@@ -374,21 +374,7 @@ static ssize_t show_phys_device(struct device *dev, }#ifdef CONFIG_MEMORY_HOTREMOVE -static int __zones_online_to (unsigned long end_pfn, -struct page *first_page,unsigned long nr_pages) -{ -struct zone *zone_next; - -/* The mem block is the last block of memory. */ -if (!pfn_valid (end_pfn +1 )) -return 1 ; - zone_next =page_zone (first_page + nr_pages); -if (zone_idx (zone_next) ==ZONE_MOVABLE ) -return 1 ; -return 0 ; -} - -static ssize_t show_zones_online_to (struct device *dev, +static ssize_t show_valid_zones (struct device *dev,struct device_attribute *attr,char *buf) {struct memory_block *mem =to_memory_block (dev);@@ -407,33 +393,23 @@ static ssize_t show_zones_online_to(struct device *dev, zone =page_zone (first_page); -#ifdef CONFIG_HIGHMEM -if (zone_idx (zone) ==ZONE_HIGHMEM ) { -if (__zones_online_to (end_pfn, first_page, nr_pages)) +if (zone_idx (zone) ==ZONE_MOVABLE -1 ) { +/*The mem block is the last memoryblock of this zone.*/ +if (end_pfn ==zone_end_pfn (zone))return sprintf (buf,"%s %s\n" , zone->name, (zone +1 )->name); } -#else -if (zone_idx (zone) ==ZONE_NORMAL ) { -if (__zones_online_to (end_pfn, first_page, nr_pages)) -return sprintf (buf,"%s %s\n" , - zone->name, (zone +1 )->name); - } -#endif if (zone_idx (zone) ==ZONE_MOVABLE ) { -if (!pfn_valid (start_pfn - nr_pages))
drivers/base/memory.c static ssize_t show_zones_online_to(struct device *dev, · line 408
Quoted 10 lines
drivers/base/memory.c +3
-5-return sprintf (buf,"%s %s\n" , - zone->name, (zone -1 )->name); - zone_prev =page_zone (first_page - nr_pages); -if (zone_idx (zone_prev) !=ZONE_MOVABLE ) +/*The mem block is the first memoryblock of ZONE_MOVABLE.*/ +if (start_pfn == zone->zone_start_pfn)return sprintf (buf,"%s %s\n" , - zone->name, (zone -1 )->name); + zone->name, (zone -1 )->name);
How about swap zone->name and (zone - 1)->name.
If swapping them, sample output of the sysfs files shows as follows:
memory0/valid_zones: none
memory1/valid_zones: DMA32
memory2/valid_zones: DMA32
memory3/valid_zones: DMA32
memory4/valid_zones: Normal
memory5/valid_zones: Normal
memory6/valid_zones: Normal Movable memory7/valid_zones: Normal Movable
~~~~~~~~~~~~~~
memory8/valid_zones: Movable
Thanks,
Yasuaki Ishimatsu
Quoted 19 lines
drivers/base/memory.c +1
-1}return sprintf (buf,"%s\n" , zone->name); } -static DEVICE_ATTR(zones_online_to, 0444, show_zones_online_to, NULL); +static DEVICE_ATTR(valid_zones, 0444, show_valid_zones, NULL);#endif static DEVICE_ATTR(phys_index, 0444, show_mem_start_phys_index, NULL);drivers/base/memory.c +1
-1@@ -587,7 +563,7 @@ static struct attribute *memory_memblk_attrs[] = { &dev_attr_phys_device.attr, &dev_attr_removable.attr, #ifdef CONFIG_MEMORY_HOTREMOVE - &dev_attr_zones_online_to.attr, + &dev_attr_valid_zones.attr, #endif NULL };
Quoted 26 lines · Yasuaki Ishimatsu
Quoted 14 lines · Zhang Zhen
As Yasuaki Ishimatsu described the check here is not enough if memory has hole as follows:
PFN
0x000xd00xe00xf0+-------------+-------------+-------------+ zone type | Normal | hole | Normal | +-------------+-------------+-------------+In this case, the check can't guarantee that this is "the last block of memory".
The check ofZONE_MOVABLEhas the same problem.Change the interface name to
valid_zonesaccording to most pepole's suggestion.Quoted 10 lines
Sample output of the sysfs files:
memory0/valid_zones: none
memory1/valid_zones: DMA32
memory2/valid_zones: DMA32
memory3/valid_zones: DMA32
memory4/valid_zones: Normal
memory5/valid_zones: Normal
memory6/valid_zones: Normal Movable
memory7/valid_zones: Movable Normal
memory8/valid_zones: Movable
Quoted 5 lines
The patch has two changes:
- change sysfs interface name
- change check ofZONE_MOVABLE
So please separate them.
Ok, i will separate them.
Thanks!
Quoted 114 lines
Quoted 101 lines
Signed-off-by Zhang Zhen <[email protected]>--- Documentation/ABI/testing/sysfs-devices-memory | 8 ++--- Documentation/memory-hotplug.txt | 4 +-- drivers/base/memory.c | 42 ++++++-------------------- 3 files changed, 15 insertions(+), 39 deletions(-)Documentation/ABI/testing/sysfs-devices-memory +4
-4@@ -61,13 +61,13 @@ Users: hotplug memory remove tools http://www.ibm.com/developerworks/wikis/display/LinuxP/powerpc-utils -What: /sys/devices/system/memory/memoryX/zones_online_to +What: /sys/devices/system/memory/memoryX/valid_zones Date: July 2014 Contact: Zhang Zhen <[email protected]> Description: - The file /sys/devices/system/memory/memoryX/zones_online_to - is read-only and is designed to show which zone this memory block can - be onlined to. + The file /sys/devices/system/memory/memoryX/valid_zones is + read-only and is designed to show which zone this memory + block can be onlined to. What: /sys/devices/system/memoryX/nodeY Date: October 2009Documentation/memory-hotplug.txt +2
-2@@ -155,7 +155,7 @@ Under each memory block, you can see 4 files: /sys/devices/system/memory/memoryXXX/phys_device /sys/devices/system/memory/memoryXXX/state /sys/devices/system/memory/memoryXXX/removable -/sys/devices/system/memory/memoryXXX/zones_online_to +/sys/devices/system/memory/memoryXXX/valid_zones 'phys_index' : read-only and contains memory block id, same as XXX. 'state' : read-write@@ -171,7 +171,7 @@ Under each memory block, you can see 4 files: block is removable and a value of 0 indicates that it is not removable. A memory block is removable only if every section in the block is removable. -'zones_online_to' : read-only: designed to show which zone this memory block +'valid_zones' : read-only: designed to show which zone this memory block can be onlined to. NOTE:drivers/base/memory.c +4
-26@@ -374,21 +374,7 @@ static ssize_t show_phys_device(struct device *dev, }#ifdef CONFIG_MEMORY_HOTREMOVE -static int __zones_online_to (unsigned long end_pfn, -struct page *first_page,unsigned long nr_pages) -{ -struct zone *zone_next; - -/* The mem block is the last block of memory. */ -if (!pfn_valid (end_pfn +1 )) -return 1 ; - zone_next =page_zone (first_page + nr_pages); -if (zone_idx (zone_next) ==ZONE_MOVABLE ) -return 1 ; -return 0 ; -} - -static ssize_t show_zones_online_to (struct device *dev, +static ssize_t show_valid_zones (struct device *dev,struct device_attribute *attr,char *buf) {struct memory_block *mem =to_memory_block (dev);@@ -407,33 +393,23 @@ static ssize_t show_zones_online_to(struct device *dev, zone =page_zone (first_page); -#ifdef CONFIG_HIGHMEM -if (zone_idx (zone) ==ZONE_HIGHMEM ) { -if (__zones_online_to (end_pfn, first_page, nr_pages)) +if (zone_idx (zone) ==ZONE_MOVABLE -1 ) { +/*The mem block is the last memoryblock of this zone.*/ +if (end_pfn ==zone_end_pfn (zone))return sprintf (buf,"%s %s\n" , zone->name, (zone +1 )->name); } -#else -if (zone_idx (zone) ==ZONE_NORMAL ) { -if (__zones_online_to (end_pfn, first_page, nr_pages)) -return sprintf (buf,"%s %s\n" , - zone->name, (zone +1 )->name); - } -#endif if (zone_idx (zone) ==ZONE_MOVABLE ) { -if (!pfn_valid (start_pfn - nr_pages))drivers/base/memory.c static ssize_t show_zones_online_to(struct device *dev, · line 408
Quoted 10 lines
drivers/base/memory.c +3
-5-return sprintf (buf,"%s %s\n" , - zone->name, (zone -1 )->name); - zone_prev =page_zone (first_page - nr_pages); -if (zone_idx (zone_prev) !=ZONE_MOVABLE ) +/*The mem block is the first memoryblock of ZONE_MOVABLE.*/ +if (start_pfn == zone->zone_start_pfn)return sprintf (buf,"%s %s\n" , - zone->name, (zone -1 )->name); + zone->name, (zone -1 )->name);How about swap
zone->nameand (zone - 1)->name.
Quoted 10 lines
If swapping them, sample output of the sysfs files shows as follows:
memory0/valid_zones: none
memory1/valid_zones: DMA32
memory2/valid_zones: DMA32
memory3/valid_zones: DMA32
memory4/valid_zones: Normal
memory5/valid_zones: Normal
memory6/valid_zones: Normal Movable memory7/valid_zones: Normal Movable
memory6/valid_zones: Normal Movable
memory7/valid_zones: Movable Normal
Here can better show the dividing line between ZONE_MOVABLE and ZONE_NORMAL.
The first column shows it's default zone,
for memory6:
the first column Normal shows that it can be onlined to ZONE_NORMAL by default. echo offline > memory6/state
echo online > memory6/state
the second column Movable shows that it can be onlined to ZONE_MOVABLE by online_movable. echo offline > memory6/state
echo online_movable > memory6/state
for memory7:
the first column Movable shows that it can be onlined to ZONE_MOVABLE by default. echo offline > memory7/state
echo online > memory7/state
the second column Normal shows that it can be onlined to ZONE_NORMAL by online_kernel. echo offline > memory7/state
echo online_kernel > memory7/state
And it is more convenient for script to work.
So i think we should leave it as it is.
Thanks!
~~~~~~~~~~~~~~
Quoted 29 lines
memory8/valid_zones: Movable
Thanks,
Yasuaki IshimatsuQuoted 11 lines
drivers/base/memory.c +1
-1}return sprintf (buf,"%s\n" , zone->name); } -static DEVICE_ATTR(zones_online_to, 0444, show_zones_online_to, NULL); +static DEVICE_ATTR(valid_zones, 0444, show_valid_zones, NULL);#endif static DEVICE_ATTR(phys_index, 0444, show_mem_start_phys_index, NULL);drivers/base/memory.c static struct attribute *memory_memblk_attrs[] = { · line 569
Quoted 8 lines
drivers/base/memory.c +1
-1@@ -587,7 +563,7 @@ static struct attribute *memory_memblk_attrs[] = { &dev_attr_phys_device.attr, &dev_attr_removable.attr, #ifdef CONFIG_MEMORY_HOTREMOVE - &dev_attr_zones_online_to.attr, + &dev_attr_valid_zones.attr, #endif NULL };.
Quoted 159 lines · Zhang Zhen
Quoted 26 lines · Yasuaki Ishimatsu
Quoted 14 lines · Zhang Zhen
As Yasuaki Ishimatsu described the check here is not enough if memory has hole as follows:
PFN
0x000xd00xe00xf0+-------------+-------------+-------------+ zone type | Normal | hole | Normal | +-------------+-------------+-------------+In this case, the check can't guarantee that this is "the last block of memory".
The check ofZONE_MOVABLEhas the same problem.Change the interface name to
valid_zonesaccording to most pepole's suggestion.Quoted 10 lines
Sample output of the sysfs files:
memory0/valid_zones: none
memory1/valid_zones: DMA32
memory2/valid_zones: DMA32
memory3/valid_zones: DMA32
memory4/valid_zones: Normal
memory5/valid_zones: Normal
memory6/valid_zones: Normal Movable
memory7/valid_zones: Movable Normal
memory8/valid_zones: MovableQuoted 5 lines
The patch has two changes:
- change sysfs interface name
- change check ofZONE_MOVABLE
So please separate them.Ok, i will separate them.
Thanks!
Quoted 114 lines
Quoted 101 lines
Signed-off-by Zhang Zhen <[email protected]>--- Documentation/ABI/testing/sysfs-devices-memory | 8 ++--- Documentation/memory-hotplug.txt | 4 +-- drivers/base/memory.c | 42 ++++++-------------------- 3 files changed, 15 insertions(+), 39 deletions(-)Documentation/ABI/testing/sysfs-devices-memory +4
-4@@ -61,13 +61,13 @@ Users: hotplug memory remove tools http://www.ibm.com/developerworks/wikis/display/LinuxP/powerpc-utils -What: /sys/devices/system/memory/memoryX/zones_online_to +What: /sys/devices/system/memory/memoryX/valid_zones Date: July 2014 Contact: Zhang Zhen <[email protected]> Description: - The file /sys/devices/system/memory/memoryX/zones_online_to - is read-only and is designed to show which zone this memory block can - be onlined to. + The file /sys/devices/system/memory/memoryX/valid_zones is + read-only and is designed to show which zone this memory + block can be onlined to. What: /sys/devices/system/memoryX/nodeY Date: October 2009Documentation/memory-hotplug.txt +2
-2@@ -155,7 +155,7 @@ Under each memory block, you can see 4 files: /sys/devices/system/memory/memoryXXX/phys_device /sys/devices/system/memory/memoryXXX/state /sys/devices/system/memory/memoryXXX/removable -/sys/devices/system/memory/memoryXXX/zones_online_to +/sys/devices/system/memory/memoryXXX/valid_zones 'phys_index' : read-only and contains memory block id, same as XXX. 'state' : read-write@@ -171,7 +171,7 @@ Under each memory block, you can see 4 files: block is removable and a value of 0 indicates that it is not removable. A memory block is removable only if every section in the block is removable. -'zones_online_to' : read-only: designed to show which zone this memory block +'valid_zones' : read-only: designed to show which zone this memory block can be onlined to. NOTE:drivers/base/memory.c +4
-26@@ -374,21 +374,7 @@ static ssize_t show_phys_device(struct device *dev, }#ifdef CONFIG_MEMORY_HOTREMOVE -static int __zones_online_to (unsigned long end_pfn, -struct page *first_page,unsigned long nr_pages) -{ -struct zone *zone_next; - -/* The mem block is the last block of memory. */ -if (!pfn_valid (end_pfn +1 )) -return 1 ; - zone_next =page_zone (first_page + nr_pages); -if (zone_idx (zone_next) ==ZONE_MOVABLE ) -return 1 ; -return 0 ; -} - -static ssize_t show_zones_online_to (struct device *dev, +static ssize_t show_valid_zones (struct device *dev,struct device_attribute *attr,char *buf) {struct memory_block *mem =to_memory_block (dev);@@ -407,33 +393,23 @@ static ssize_t show_zones_online_to(struct device *dev, zone =page_zone (first_page); -#ifdef CONFIG_HIGHMEM -if (zone_idx (zone) ==ZONE_HIGHMEM ) { -if (__zones_online_to (end_pfn, first_page, nr_pages)) +if (zone_idx (zone) ==ZONE_MOVABLE -1 ) { +/*The mem block is the last memoryblock of this zone.*/ +if (end_pfn ==zone_end_pfn (zone))return sprintf (buf,"%s %s\n" , zone->name, (zone +1 )->name); } -#else -if (zone_idx (zone) ==ZONE_NORMAL ) { -if (__zones_online_to (end_pfn, first_page, nr_pages)) -return sprintf (buf,"%s %s\n" , - zone->name, (zone +1 )->name); - } -#endif if (zone_idx (zone) ==ZONE_MOVABLE ) { -if (!pfn_valid (start_pfn - nr_pages))drivers/base/memory.c static ssize_t show_zones_online_to(struct device *dev, · line 408
Quoted 10 lines
drivers/base/memory.c +3
-5-return sprintf (buf,"%s %s\n" , - zone->name, (zone -1 )->name); - zone_prev =page_zone (first_page - nr_pages); -if (zone_idx (zone_prev) !=ZONE_MOVABLE ) +/*The mem block is the first memoryblock of ZONE_MOVABLE.*/ +if (start_pfn == zone->zone_start_pfn)return sprintf (buf,"%s %s\n" , - zone->name, (zone -1 )->name); + zone->name, (zone -1 )->name);How about swap
zone->nameand (zone - 1)->name.Quoted 10 lines
If swapping them, sample output of the sysfs files shows as follows:
memory0/valid_zones: none
memory1/valid_zones: DMA32
memory2/valid_zones: DMA32
memory3/valid_zones: DMA32
memory4/valid_zones: Normal
memory5/valid_zones: Normal
memory6/valid_zones: Normal Movable memory7/valid_zones: Normal Movable
Quoted 6 lines
memory6/valid_zones: Normal Movable
memory7/valid_zones: Movable Normal
Here can better show the dividing line betweenZONE_MOVABLEandZONE_NORMAL.The first column shows it's default zone,
If so, please write the information in 'valid_zones' term of memory-hotplug.txt. Nobody know it.
Thanks,
Yasuaki Ishimatasu
Quoted 51 lines
for memory6:
the first column Normal shows that it can be onlined toZONE_NORMALby default. echo offline > memory6/state
echo online > memory6/state
the second column Movable shows that it can be onlined toZONE_MOVABLEbyonline_movable. echo offline > memory6/state
echoonline_movable> memory6/state
for memory7:
the first column Movable shows that it can be onlined toZONE_MOVABLEby default. echo offline > memory7/state
echo online > memory7/state
the second column Normal shows that it can be onlined toZONE_NORMALbyonline_kernel. echo offline > memory7/state
echoonline_kernel> memory7/stateAnd it is more convenient for script to work.
So i think we should leave it as it is.Thanks!
~~~~~~~~~~~~~~Quoted 29 lines
memory8/valid_zones: Movable
Thanks,
Yasuaki IshimatsuQuoted 11 lines
drivers/base/memory.c +1
-1}return sprintf (buf,"%s\n" , zone->name); } -static DEVICE_ATTR(zones_online_to, 0444, show_zones_online_to, NULL); +static DEVICE_ATTR(valid_zones, 0444, show_valid_zones, NULL);#endif static DEVICE_ATTR(phys_index, 0444, show_mem_start_phys_index, NULL);drivers/base/memory.c static struct attribute *memory_memblk_attrs[] = { · line 569
Quoted 8 lines
drivers/base/memory.c +1
-1@@ -587,7 +563,7 @@ static struct attribute *memory_memblk_attrs[] = { &dev_attr_phys_device.attr, &dev_attr_removable.attr, #ifdef CONFIG_MEMORY_HOTREMOVE - &dev_attr_zones_online_to.attr, + &dev_attr_valid_zones.attr, #endif NULL };.