Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1554793 > unrolled thread
| Started by | Andrew Morton <akpm@linux-foundation.org> |
|---|---|
| First post | 2017-01-10 00:30 +0100 |
| Last post | 2017-01-11 17:50 +0100 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v2] memory_hotplug: zone_can_shift() returns boolean value Andrew Morton <akpm@linux-foundation.org> - 2017-01-10 00:30 +0100
[PATCH v3] memory_hotplug: zone_can_shift() returns boolean value Yasuaki Ishimatsu <yasu.isimatu@gmail.com> - 2017-01-11 17:50 +0100
Re: [PATCH v2] memory_hotplug: zone_can_shift() returns boolean value Yasuaki Ishimatsu <yasu.isimatu@gmail.com> - 2017-01-11 17:50 +0100
| From | Andrew Morton <akpm@linux-foundation.org> |
|---|---|
| Date | 2017-01-10 00:30 +0100 |
| Subject | Re: [PATCH v2] memory_hotplug: zone_can_shift() returns boolean value |
| Message-ID | <sXRy2-203-21@gated-at.bofh.it> |
On Tue, 13 Dec 2016 15:29:49 -0500 Yasuaki Ishimatsu <yasu.isimatu@gmail.com> wrote:
> online_{kernel|movable} is used to change the memory zone to
> ZONE_{NORMAL|MOVABLE} and online the memory.
>
> To check that memory zone can be changed, zone_can_shift() is used.
> Currently the function returns minus integer value, plus integer
> value and 0. When the function returns minus or plus integer value,
> it means that the memory zone can be changed to ZONE_{NORNAL|MOVABLE}.
>
> But when the function returns 0, there is 2 meanings.
>
> One of the meanings is that the memory zone does not need to be changed.
> For example, when memory is in ZONE_NORMAL and onlined by online_kernel
> the memory zone does not need to be changed.
>
> Another meaning is that the memory zone cannot be changed. When memory
> is in ZONE_NORMAL and onlined by online_movable, the memory zone may
> not be changed to ZONE_MOVALBE due to memory online limitation(see
> Documentation/memory-hotplug.txt). In this case, memory must not be
> onlined.
>
> The patch changes the return type of zone_can_shift() so that memory
> is not onlined when memory zone cannot be changed.
What are the user-visible runtime effects of this fix?
Please always include this info when fixing bugs - it is required so
that others can decide which kernel version(s) need the fix.
Thanks.
[toc] | [next] | [standalone]
| From | Yasuaki Ishimatsu <yasu.isimatu@gmail.com> |
|---|---|
| Date | 2017-01-11 17:50 +0100 |
| Subject | [PATCH v3] memory_hotplug: zone_can_shift() returns boolean value |
| Message-ID | <sYug1-OR-3@gated-at.bofh.it> |
| In reply to | #1554793 |
online_{kernel|movable} is used to change the memory zone to
ZONE_{NORMAL|MOVABLE} and online the memory.
To check that memory zone can be changed, zone_can_shift() is used.
Currently the function returns minus integer value, plus integer
value and 0. When the function returns minus or plus integer value,
it means that the memory zone can be changed to ZONE_{NORNAL|MOVABLE}.
But when the function returns 0, there is 2 meanings.
One of the meanings is that the memory zone does not need to be changed.
For example, when memory is in ZONE_NORMAL and onlined by online_kernel
the memory zone does not need to be changed.
Another meaning is that the memory zone cannot be changed. When memory
is in ZONE_NORMAL and onlined by online_movable, the memory zone may
not be changed to ZONE_MOVALBE due to memory online limitation(see
Documentation/memory-hotplug.txt). In this case, memory must not be
onlined.
The patch changes the return type of zone_can_shift() so that memory
online operation fails when memory zone cannot be changed as follows:
Before applying patch:
# grep -A 35 "Node 2" /proc/zoneinfo
Node 2, zone Normal
<snip>
node_scanned 0
spanned 8388608
present 7864320
managed 7864320
# echo online_movable > memory4097/state
# grep -A 35 "Node 2" /proc/zoneinfo
Node 2, zone Normal
<snip>
node_scanned 0
spanned 8388608
present 8388608
managed 8388608
online_movable operation succeeded. But memory is onlined as
ZONE_NORMAL, not ZONE_MOVABLE.
After applying patch:
# grep -A 35 "Node 2" /proc/zoneinfo
Node 2, zone Normal
<snip>
node_scanned 0
spanned 8388608
present 7864320
managed 7864320
# echo online_movable > memory4097/state
bash: echo: write error: Invalid argument
# grep -A 35 "Node 2" /proc/zoneinfo
Node 2, zone Normal
<snip>
node_scanned 0
spanned 8388608
present 7864320
managed 7864320
online_movable operation failed because of failure of changing
the memory zone from ZONE_NORMAL to ZONE_MOVABLE
Fixes: df429ac03936 ("memory-hotplug: more general validation of zone during online")
Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Reviewed-by: Reza Arbab <arbab@linux.vnet.ibm.com>
---
from v2:
- Add the user-visible runtime effects of this fix
from v1:
- Initialize zone_shift argument in zone_can_shift() to 0
- Fix duplicate output of valid_zones
drivers/base/memory.c | 4 ++--
include/linux/memory_hotplug.h | 4 ++--
mm/memory_hotplug.c | 28 +++++++++++++++++-----------
3 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index bb69e58..3f47d94 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -408,14 +408,14 @@ static ssize_t show_valid_zones(struct device *dev,
sprintf(buf, "%s", zone->name);
/* MMOP_ONLINE_KERNEL */
- zone_shift = zone_can_shift(start_pfn, nr_pages, ZONE_NORMAL);
+ zone_can_shift(start_pfn, nr_pages, ZONE_NORMAL, &zone_shift);
if (zone_shift) {
strcat(buf, " ");
strcat(buf, (zone + zone_shift)->name);
}
/* MMOP_ONLINE_MOVABLE */
- zone_shift = zone_can_shift(start_pfn, nr_pages, ZONE_MOVABLE);
+ zone_can_shift(start_pfn, nr_pages, ZONE_MOVABLE, &zone_shift);
if (zone_shift) {
strcat(buf, " ");
strcat(buf, (zone + zone_shift)->name);
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 01033fa..c1784c0 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -284,7 +284,7 @@ extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms,
unsigned long map_offset);
extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
unsigned long pnum);
-extern int zone_can_shift(unsigned long pfn, unsigned long nr_pages,
- enum zone_type target);
+extern bool zone_can_shift(unsigned long pfn, unsigned long nr_pages,
+ enum zone_type target, int *zone_shift);
#endif /* __LINUX_MEMORY_HOTPLUG_H */
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 9847e4a..d6dd65c 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1034,36 +1034,39 @@ static void node_states_set_node(int node, struct memory_notify *arg)
node_set_state(node, N_MEMORY);
}
-int zone_can_shift(unsigned long pfn, unsigned long nr_pages,
- enum zone_type target)
+bool zone_can_shift(unsigned long pfn, unsigned long nr_pages,
+ enum zone_type target, int *zone_shift)
{
struct zone *zone = page_zone(pfn_to_page(pfn));
enum zone_type idx = zone_idx(zone);
int i;
+ *zone_shift = 0;
+
if (idx < target) {
/* pages must be at end of current zone */
if (pfn + nr_pages != zone_end_pfn(zone))
- return 0;
+ return false;
/* no zones in use between current zone and target */
for (i = idx + 1; i < target; i++)
if (zone_is_initialized(zone - idx + i))
- return 0;
+ return false;
}
if (target < idx) {
/* pages must be at beginning of current zone */
if (pfn != zone->zone_start_pfn)
- return 0;
+ return false;
/* no zones in use between current zone and target */
for (i = target + 1; i < idx; i++)
if (zone_is_initialized(zone - idx + i))
- return 0;
+ return false;
}
- return target - idx;
+ *zone_shift = target - idx;
+ return true;
}
/* Must be protected by mem_hotplug_begin() */
@@ -1090,10 +1093,13 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ
!can_online_high_movable(zone))
return -EINVAL;
- if (online_type == MMOP_ONLINE_KERNEL)
- zone_shift = zone_can_shift(pfn, nr_pages, ZONE_NORMAL);
- else if (online_type == MMOP_ONLINE_MOVABLE)
- zone_shift = zone_can_shift(pfn, nr_pages, ZONE_MOVABLE);
+ if (online_type == MMOP_ONLINE_KERNEL) {
+ if (!zone_can_shift(pfn, nr_pages, ZONE_NORMAL, &zone_shift))
+ return -EINVAL;
+ } else if (online_type == MMOP_ONLINE_MOVABLE) {
+ if (!zone_can_shift(pfn, nr_pages, ZONE_MOVABLE, &zone_shift))
+ return -EINVAL;
+ }
zone = move_pfn_range(zone_shift, pfn, pfn + nr_pages);
if (!zone)
--
1.8.3.1
[toc] | [prev] | [next] | [standalone]
| From | Yasuaki Ishimatsu <yasu.isimatu@gmail.com> |
|---|---|
| Date | 2017-01-11 17:50 +0100 |
| Subject | Re: [PATCH v2] memory_hotplug: zone_can_shift() returns boolean value |
| Message-ID | <sYug1-OR-5@gated-at.bofh.it> |
| In reply to | #1554793 |
Hi Andrew
On 01/09/2017 06:27 PM, Andrew Morton wrote:
> On Tue, 13 Dec 2016 15:29:49 -0500 Yasuaki Ishimatsu <yasu.isimatu@gmail.com> wrote:
>
>> online_{kernel|movable} is used to change the memory zone to
>> ZONE_{NORMAL|MOVABLE} and online the memory.
>>
>> To check that memory zone can be changed, zone_can_shift() is used.
>> Currently the function returns minus integer value, plus integer
>> value and 0. When the function returns minus or plus integer value,
>> it means that the memory zone can be changed to ZONE_{NORNAL|MOVABLE}.
>>
>> But when the function returns 0, there is 2 meanings.
>>
>> One of the meanings is that the memory zone does not need to be changed.
>> For example, when memory is in ZONE_NORMAL and onlined by online_kernel
>> the memory zone does not need to be changed.
>>
>> Another meaning is that the memory zone cannot be changed. When memory
>> is in ZONE_NORMAL and onlined by online_movable, the memory zone may
>> not be changed to ZONE_MOVALBE due to memory online limitation(see
>> Documentation/memory-hotplug.txt). In this case, memory must not be
>> onlined.
>>
>> The patch changes the return type of zone_can_shift() so that memory
>> is not onlined when memory zone cannot be changed.
>
> What are the user-visible runtime effects of this fix?
The user-visible runtime effects of the fix are here:
Before applying patch:
# grep -A 35 "Node 2" /proc/zoneinfo
Node 2, zone Normal
<snip>
node_scanned 0
spanned 8388608
present 7864320
managed 7864320
# echo online_movable > memory4097/state
# grep -A 35 "Node 2" /proc/zoneinfo
Node 2, zone Normal
<snip>
node_scanned 0
spanned 8388608
present 8388608
managed 8388608
online_movable operation succeeded. But memory is onlined as
ZONE_NORMAL, not ZONE_MOVABLE.
After applying patch:
# grep -A 35 "Node 2" /proc/zoneinfo
Node 2, zone Normal
<snip>
node_scanned 0
spanned 8388608
present 7864320
managed 7864320
# echo online_movable > memory4097/state
bash: echo: write error: Invalid argument
# grep -A 35 "Node 2" /proc/zoneinfo
Node 2, zone Normal
<snip>
node_scanned 0
spanned 8388608
present 7864320
managed 7864320
online_movable operation failed because of failure of changing
the memory zone from ZONE_NORMAL to ZONE_MOVABLE
> Please always include this info when fixing bugs - it is required so
> that others can decide which kernel version(s) need the fix.
I'll add the above information and resend the patch as v3.
Thanks,
Yasuaki Ishimatsu
> Thanks.
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web