Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1619814 > unrolled thread
| Started by | Michal Hocko <mhocko@kernel.org> |
|---|---|
| First post | 2017-04-10 13:10 +0200 |
| Last post | 2017-04-24 10:00 +0200 |
| Articles | 20 on this page of 63 — 9 participants |
Back to article view | Back to linux.kernel
[PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-10 13:10 +0200
[PATCH 3/9] mm: drop page_initialized check from get_nid_for_pfn Michal Hocko <mhocko@kernel.org> - 2017-04-10 13:10 +0200
Re: [PATCH 3/9] mm: drop page_initialized check from get_nid_for_pfn Vlastimil Babka <vbabka@suse.cz> - 2017-04-13 15:00 +0200
[PATCH 9/9] mm, memory_hotplug: remove unused cruft after memory hotplug rework Michal Hocko <mhocko@kernel.org> - 2017-04-10 13:10 +0200
Re: [PATCH 9/9] mm, memory_hotplug: remove unused cruft after memory hotplug rework Vlastimil Babka <vbabka@suse.cz> - 2017-04-20 10:40 +0200
[PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section Michal Hocko <mhocko@kernel.org> - 2017-04-10 13:10 +0200
Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section Jerome Glisse <jglisse@redhat.com> - 2017-04-10 18:30 +0200
Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section Michal Hocko <mhocko@kernel.org> - 2017-04-10 18:40 +0200
Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section Vlastimil Babka <vbabka@suse.cz> - 2017-04-13 15:10 +0200
Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section Jerome Glisse <jglisse@redhat.com> - 2017-04-17 22:20 +0200
Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section Michal Hocko <mhocko@kernel.org> - 2017-04-18 09:20 +0200
[PATCH 5/9] mm, memory_hotplug: split up register_one_node Michal Hocko <mhocko@kernel.org> - 2017-04-10 13:10 +0200
Re: [PATCH 5/9] mm, memory_hotplug: split up register_one_node Vlastimil Babka <vbabka@suse.cz> - 2017-04-13 16:10 +0200
Re: [PATCH 5/9] mm, memory_hotplug: split up register_one_node Michal Hocko <mhocko@kernel.org> - 2017-04-13 16:20 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Igor Mammedov <imammedo@redhat.com> - 2017-04-10 16:30 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-10 17:00 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-10 17:30 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-10 17:40 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Igor Mammedov <imammedo@redhat.com> - 2017-04-11 10:10 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-11 10:50 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Igor Mammedov <imammedo@redhat.com> - 2017-04-11 12:00 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-11 12:50 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-10 18:10 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Vlastimil Babka <vbabka@suse.cz> - 2017-04-18 10:30 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-10 18:10 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Igor Mammedov <imammedo@redhat.com> - 2017-04-11 08:40 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-11 11:30 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Igor Mammedov <imammedo@redhat.com> - 2017-04-11 12:00 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-11 13:10 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-11 13:40 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-11 14:40 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-10 17:50 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-11 11:00 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Jerome Glisse <jglisse@redhat.com> - 2017-04-10 18:40 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-10 20:00 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Balbir Singh <bsingharora@gmail.com> - 2017-04-11 05:00 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-11 19:10 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Dan Williams <dan.j.williams@gmail.com> - 2017-04-18 00:00 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-18 09:20 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Dan Williams <dan.j.williams@gmail.com> - 2017-04-18 18:50 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-18 22:00 +0200
Re: [PATCH -v2 0/9] mm: make movable onlining suck less Dan Williams <dan.j.williams@intel.com> - 2017-04-20 05:40 +0200
[PATCH 3/3] mm: __first_valid_page skip over offline pages Michal Hocko <mhocko@kernel.org> - 2017-04-15 14:20 +0200
[PATCH 1/3] mm: consider zone which is not fully populated to have holes Michal Hocko <mhocko@kernel.org> - 2017-04-15 14:20 +0200
Re: [PATCH 1/3] mm: consider zone which is not fully populated to have holes Vlastimil Babka <vbabka@suse.cz> - 2017-04-18 10:50 +0200
Re: [PATCH 1/3] mm: consider zone which is not fully populated to have holes Michal Hocko <mhocko@kernel.org> - 2017-04-18 11:30 +0200
Re: [PATCH 1/3] mm: consider zone which is not fully populated to have holes Vlastimil Babka <vbabka@suse.cz> - 2017-04-19 14:00 +0200
Re: [PATCH 1/3] mm: consider zone which is not fully populated to have holes Michal Hocko <mhocko@kernel.org> - 2017-04-19 14:20 +0200
Re: [PATCH 1/3] mm: consider zone which is not fully populated to have holes Vlastimil Babka <vbabka@suse.cz> - 2017-04-19 14:40 +0200
Re: [PATCH 1/3] mm: consider zone which is not fully populated to have holes Michal Hocko <mhocko@kernel.org> - 2017-04-19 15:00 +0200
(none) Michal Hocko <mhocko@kernel.org> - 2017-04-15 14:20 +0200
[PATCH 2/3] mm, compaction: skip over holes in __reset_isolation_suitable Michal Hocko <mhocko@kernel.org> - 2017-04-15 14:20 +0200
Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-17 07:50 +0200
Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-17 10:20 +0200
Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-20 03:30 +0200
Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-20 09:30 +0200
Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-20 10:50 +0200
Re: your mail Vlastimil Babka <vbabka@suse.cz> - 2017-04-20 14:00 +0200
Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-20 14:20 +0200
Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-21 06:40 +0200
Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-21 09:20 +0200
Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-24 03:50 +0200
Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-24 10:00 +0200
Page 1 of 4 [1] 2 3 4 Next page →
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-10 13:10 +0200 |
| Subject | [PATCH -v2 0/9] mm: make movable onlining suck less |
| Message-ID | <tuFmO-8iD-7@gated-at.bofh.it> |
Hi,
The last version of this series has been posted here [1]. It has seen
some more serious testing (thanks to Reza Arbab) and fixes for the found
issues. I have also decided to drop patch 1 [2] because it turned out to
be more complicated than I initially thought [3]. Few more patches were
added to deal with expectation on zone/node initialization.
I have rebased on top of the current mmotm-2017-04-07-15-53. It
conflicts with HMM because it touches memory hotplug as
well. We have discussed [4] with Jérôme and he agreed to
rebase on top of this rework [5] so I have reverted his series
before applyig mine. I will help him to resolve the resulting
conflicts. You can find the whole series including the HMM revers in
git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git branch
attempts/rewrite-mem_hotplug
Motivation:
Movable onlining is a real hack with many downsides - mainly
reintroduction of lowmem/highmem issues we used to have on 32b systems -
but it is the only way to make the memory hotremove more reliable which
is something that people are asking for.
The current semantic of memory movable onlinening is really cumbersome,
however. The main reason for this is that the udev driven approach is
basically unusable because udev races with the memory probing while only
the last memory block or the one adjacent to the existing zone_movable
are allowed to be onlined movable. In short the criterion for the
successful online_movable changes under udev's feet. A reliable udev
approach would require a 2 phase approach where the first successful
movable online would have to check all the previous blocks and online
them in descending order. This is hard to be considered sane.
This patchset aims at making the onlining semantic more usable. First of
all it allows to online memory movable as long as it doesn't clash with
the existing ZONE_NORMAL. That means that ZONE_NORMAL and ZONE_MOVABLE
cannot overlap. Currently I preserve the original ordering semantic so
the zone always precedes the movable zone but I have plans to remove this
restriction in future because it is not really necessary.
First 3 patches are cleanups which should be ready to be merged right
away (unless I have missed something subtle of course).
Patch 4 deals with ZONE_DEVICE dependencies down the __add_pages path.
Patch 5 deals with implicit assumptions of register_one_node on pgdat
initialization.
Patch 6 is the core of the change. In order to make it easier to review
I have tried it to be as minimalistic as possible and the large code
removal is moved to patch 9.
Patch 7 is a trivial follow up cleanup. Patch 8 fixes sparse warnings
and finally patch 9 removes the unused code.
I have tested the patches in kvm:
# qemu-system-x86_64 -enable-kvm -monitor pty -m 2G,slots=4,maxmem=4G -numa node,mem=1G -numa node,mem=1G ...
and then probed the additional memory by
(qemu) object_add memory-backend-ram,id=mem1,size=1G
(qemu) device_add pc-dimm,id=dimm1,memdev=mem1
Then I have used this simple script to probe the memory block by hand
# cat probe_memblock.sh
#!/bin/sh
BLOCK_NR=$1
# echo $((0x100000000+$BLOCK_NR*(128<<20))) > /sys/devices/system/memory/probe
# for i in $(seq 10); do sh probe_memblock.sh $i; done
# grep . /sys/devices/system/memory/memory3?/valid_zones 2>/dev/null
/sys/devices/system/memory/memory33/valid_zones:Normal Movable
/sys/devices/system/memory/memory34/valid_zones:Normal Movable
/sys/devices/system/memory/memory35/valid_zones:Normal Movable
/sys/devices/system/memory/memory36/valid_zones:Normal Movable
/sys/devices/system/memory/memory37/valid_zones:Normal Movable
/sys/devices/system/memory/memory38/valid_zones:Normal Movable
/sys/devices/system/memory/memory39/valid_zones:Normal Movable
The main difference to the original implementation is that all new
memblocks can be both online_kernel and online_movable initially
because there is no clash obviously. For the comparison the original
implementation would have
/sys/devices/system/memory/memory33/valid_zones:Normal
/sys/devices/system/memory/memory34/valid_zones:Normal
/sys/devices/system/memory/memory35/valid_zones:Normal
/sys/devices/system/memory/memory36/valid_zones:Normal
/sys/devices/system/memory/memory37/valid_zones:Normal
/sys/devices/system/memory/memory38/valid_zones:Normal
/sys/devices/system/memory/memory39/valid_zones:Normal Movable
Now
# echo online_movable > /sys/devices/system/memory/memory34/state
# grep . /sys/devices/system/memory/memory3?/valid_zones 2>/dev/null
/sys/devices/system/memory/memory33/valid_zones:Normal Movable
/sys/devices/system/memory/memory34/valid_zones:Movable
/sys/devices/system/memory/memory35/valid_zones:Movable
/sys/devices/system/memory/memory36/valid_zones:Movable
/sys/devices/system/memory/memory37/valid_zones:Movable
/sys/devices/system/memory/memory38/valid_zones:Movable
/sys/devices/system/memory/memory39/valid_zones:Movable
Block 33 can still be online both kernel and movable while all
the remaining can be only movable.
/proc/zonelist says
Node 0, zone Normal
pages free 0
min 0
low 0
high 0
spanned 0
present 0
--
Node 0, zone Movable
pages free 32753
min 85
low 117
high 149
spanned 32768
present 32768
A new memblock at a lower address will result in a new memblock (32)
which will still allow both Normal and Movable.
# sh probe_memblock.sh 0
# grep . /sys/devices/system/memory/memory3[2-5]/valid_zones 2>/dev/null
/sys/devices/system/memory/memory32/valid_zones:Normal Movable
/sys/devices/system/memory/memory33/valid_zones:Normal Movable
/sys/devices/system/memory/memory34/valid_zones:Movable
/sys/devices/system/memory/memory35/valid_zones:Movable
and online_kernel will convert it to the zone normal properly
while 33 can be still onlined both ways.
# echo online_kernel > /sys/devices/system/memory/memory32/state
# grep . /sys/devices/system/memory/memory3[2-5]/valid_zones 2>/dev/null
/sys/devices/system/memory/memory32/valid_zones:Normal
/sys/devices/system/memory/memory33/valid_zones:Normal Movable
/sys/devices/system/memory/memory34/valid_zones:Movable
/sys/devices/system/memory/memory35/valid_zones:Movable
/proc/zoneinfo will now tell
Node 0, zone Normal
pages free 65441
min 165
low 230
high 295
spanned 65536
present 65536
--
Node 0, zone Movable
pages free 32740
min 82
low 114
high 146
spanned 32768
present 32768
so both zones have one memblock spanned and present.
Onlining 39 should associate this block to the movable zone
# echo online > /sys/devices/system/memory/memory39/state
/proc/zoneinfo will now tell
Node 0, zone Normal
pages free 32765
min 80
low 112
high 144
spanned 32768
present 32768
--
Node 0, zone Movable
pages free 65501
min 160
low 225
high 290
spanned 196608
present 65536
so we will have a movable zone which spans 6 memblocks, 2 present and 4
representing a hole.
Offlining both movable blocks will lead to the zone with no present
pages which is the expected behavior I believe.
# echo offline > /sys/devices/system/memory/memory39/state
# echo offline > /sys/devices/system/memory/memory34/state
# grep -A6 "Movable\|Normal" /proc/zoneinfo
Node 0, zone Normal
pages free 32735
min 90
low 122
high 154
spanned 32768
present 32768
--
Node 0, zone Movable
pages free 0
min 0
low 0
high 0
spanned 196608
present 0
Any thoughts, complains, suggestions?
As a bonus we will get a nice cleanup in the memory hotplug codebase
arch/ia64/mm/init.c | 11 +-
arch/powerpc/mm/mem.c | 12 +-
arch/s390/mm/init.c | 32 +--
arch/sh/mm/init.c | 10 +-
arch/x86/mm/init_32.c | 7 +-
arch/x86/mm/init_64.c | 11 +-
drivers/base/memory.c | 74 ++++---
drivers/base/node.c | 58 ++----
include/linux/memory_hotplug.h | 19 +-
include/linux/mmzone.h | 16 +-
include/linux/node.h | 35 +++-
kernel/memremap.c | 6 +-
mm/memory_hotplug.c | 451 ++++++++++++++---------------------------
mm/page_alloc.c | 8 +-
mm/sparse.c | 3 +-
15 files changed, 284 insertions(+), 469 deletions(-)
Shortlog says:
Michal Hocko (9):
mm: remove return value from init_currently_empty_zone
mm, memory_hotplug: use node instead of zone in can_online_high_movable
mm: drop page_initialized check from get_nid_for_pfn
mm, memory_hotplug: get rid of is_zone_device_section
mm, memory_hotplug: split up register_one_node
mm, memory_hotplug: do not associate hotadded memory to zones until online
mm, memory_hotplug: replace for_device by want_memblock in arch_add_memory
mm, memory_hotplug: fix the section mismatch warning
mm, memory_hotplug: remove unused cruft after memory hotplug rework
[1] http://lkml.kernel.org/r/20170330115454.32154-1-mhocko@kernel.org
[2] http://lkml.kernel.org/r/20170331073954.GF27098@dhcp22.suse.cz
[3] http://lkml.kernel.org/r/20170405081400.GE6035@dhcp22.suse.cz
[4] http://lkml.kernel.org/r/20170407121349.GB16392@dhcp22.suse.cz
[5] http://lkml.kernel.org/r/20170407182752.GA17852@redhat.com
[toc] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-10 13:10 +0200 |
| Subject | [PATCH 3/9] mm: drop page_initialized check from get_nid_for_pfn |
| Message-ID | <tuFmP-8iD-23@gated-at.bofh.it> |
| In reply to | #1619814 |
From: Michal Hocko <mhocko@suse.com>
c04fc586c1a4 ("mm: show node to memory section relationship with
symlinks in sysfs") has added means to export memblock<->node
association into the sysfs. It has also introduced get_nid_for_pfn
which is a rather confusing counterpart of pfn_to_nid which checks also
whether the pfn page is already initialized (page_initialized). This
is done by checking page::lru != NULL which doesn't make any sense at
all. Nothing in this path really relies on the lru list being used or
initialized. Just remove it because this will become a problem with
later patches.
Thanks to Reza Arbab for testing which revealed this to be a problem
(http://lkml.kernel.org/r/20170403202337.GA12482@dhcp22.suse.cz)
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
drivers/base/node.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index 5548f9686016..06294d69779b 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -368,21 +368,14 @@ int unregister_cpu_under_node(unsigned int cpu, unsigned int nid)
}
#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
-#define page_initialized(page) (page->lru.next)
-
static int __ref get_nid_for_pfn(unsigned long pfn)
{
- struct page *page;
-
if (!pfn_valid_within(pfn))
return -1;
#ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
if (system_state == SYSTEM_BOOTING)
return early_pfn_to_nid(pfn);
#endif
- page = pfn_to_page(pfn);
- if (!page_initialized(page))
- return -1;
return pfn_to_nid(pfn);
}
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2017-04-13 15:00 +0200 |
| Subject | Re: [PATCH 3/9] mm: drop page_initialized check from get_nid_for_pfn |
| Message-ID | <tvMvU-388-9@gated-at.bofh.it> |
| In reply to | #1619815 |
On 04/10/2017 01:03 PM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> c04fc586c1a4 ("mm: show node to memory section relationship with
> symlinks in sysfs") has added means to export memblock<->node
> association into the sysfs. It has also introduced get_nid_for_pfn
> which is a rather confusing counterpart of pfn_to_nid which checks also
> whether the pfn page is already initialized (page_initialized). This
> is done by checking page::lru != NULL which doesn't make any sense at
> all. Nothing in this path really relies on the lru list being used or
> initialized. Just remove it because this will become a problem with
> later patches.
>
> Thanks to Reza Arbab for testing which revealed this to be a problem
> (http://lkml.kernel.org/r/20170403202337.GA12482@dhcp22.suse.cz)
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-10 13:10 +0200 |
| Subject | [PATCH 9/9] mm, memory_hotplug: remove unused cruft after memory hotplug rework |
| Message-ID | <tuFmP-8iD-25@gated-at.bofh.it> |
| In reply to | #1619814 |
From: Michal Hocko <mhocko@suse.com>
zone_for_memory doesn't have any user anymore as well as the whole zone
shifting infrastructure so drop them all.
This shouldn't introduce any functional changes.
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
include/linux/memory_hotplug.h | 2 -
mm/memory_hotplug.c | 207 -----------------------------------------
2 files changed, 209 deletions(-)
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index c28d0aba7525..a9985f6c460a 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -274,8 +274,6 @@ extern int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
void *arg, int (*func)(struct memory_block *, void *));
extern int add_memory(int nid, u64 start, u64 size);
extern int add_memory_resource(int nid, struct resource *resource, bool online);
-extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
- bool for_device);
extern int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock);
extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
unsigned long nr_pages);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index be8be844d340..94e96ca790f6 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -299,180 +299,6 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
}
#endif /* CONFIG_HAVE_BOOTMEM_INFO_NODE */
-static void __meminit grow_zone_span(struct zone *zone, unsigned long start_pfn,
- unsigned long end_pfn)
-{
- unsigned long old_zone_end_pfn;
-
- zone_span_writelock(zone);
-
- old_zone_end_pfn = zone_end_pfn(zone);
- if (zone_is_empty(zone) || start_pfn < zone->zone_start_pfn)
- zone->zone_start_pfn = start_pfn;
-
- zone->spanned_pages = max(old_zone_end_pfn, end_pfn) -
- zone->zone_start_pfn;
-
- zone_span_writeunlock(zone);
-}
-
-static void resize_zone(struct zone *zone, unsigned long start_pfn,
- unsigned long end_pfn)
-{
- zone_span_writelock(zone);
-
- if (end_pfn - start_pfn) {
- zone->zone_start_pfn = start_pfn;
- zone->spanned_pages = end_pfn - start_pfn;
- } else {
- /*
- * make it consist as free_area_init_core(),
- * if spanned_pages = 0, then keep start_pfn = 0
- */
- zone->zone_start_pfn = 0;
- zone->spanned_pages = 0;
- }
-
- zone_span_writeunlock(zone);
-}
-
-static void fix_zone_id(struct zone *zone, unsigned long start_pfn,
- unsigned long end_pfn)
-{
- enum zone_type zid = zone_idx(zone);
- int nid = zone->zone_pgdat->node_id;
- unsigned long pfn;
-
- for (pfn = start_pfn; pfn < end_pfn; pfn++)
- set_page_links(pfn_to_page(pfn), zid, nid, pfn);
-}
-
-static void __ref ensure_zone_is_initialized(struct zone *zone,
- unsigned long start_pfn, unsigned long num_pages)
-{
- if (!zone_is_initialized(zone))
- init_currently_empty_zone(zone, start_pfn, num_pages);
-}
-
-static int __meminit move_pfn_range_left(struct zone *z1, struct zone *z2,
- unsigned long start_pfn, unsigned long end_pfn)
-{
- unsigned long flags;
- unsigned long z1_start_pfn;
-
- ensure_zone_is_initialized(z1, start_pfn, end_pfn - start_pfn);
-
- pgdat_resize_lock(z1->zone_pgdat, &flags);
-
- /* can't move pfns which are higher than @z2 */
- if (end_pfn > zone_end_pfn(z2))
- goto out_fail;
- /* the move out part must be at the left most of @z2 */
- if (start_pfn > z2->zone_start_pfn)
- goto out_fail;
- /* must included/overlap */
- if (end_pfn <= z2->zone_start_pfn)
- goto out_fail;
-
- /* use start_pfn for z1's start_pfn if z1 is empty */
- if (!zone_is_empty(z1))
- z1_start_pfn = z1->zone_start_pfn;
- else
- z1_start_pfn = start_pfn;
-
- resize_zone(z1, z1_start_pfn, end_pfn);
- resize_zone(z2, end_pfn, zone_end_pfn(z2));
-
- pgdat_resize_unlock(z1->zone_pgdat, &flags);
-
- fix_zone_id(z1, start_pfn, end_pfn);
-
- return 0;
-out_fail:
- pgdat_resize_unlock(z1->zone_pgdat, &flags);
- return -1;
-}
-
-static int __meminit move_pfn_range_right(struct zone *z1, struct zone *z2,
- unsigned long start_pfn, unsigned long end_pfn)
-{
- unsigned long flags;
- unsigned long z2_end_pfn;
-
- ensure_zone_is_initialized(z2, start_pfn, end_pfn - start_pfn);
-
- pgdat_resize_lock(z1->zone_pgdat, &flags);
-
- /* can't move pfns which are lower than @z1 */
- if (z1->zone_start_pfn > start_pfn)
- goto out_fail;
- /* the move out part mast at the right most of @z1 */
- if (zone_end_pfn(z1) > end_pfn)
- goto out_fail;
- /* must included/overlap */
- if (start_pfn >= zone_end_pfn(z1))
- goto out_fail;
-
- /* use end_pfn for z2's end_pfn if z2 is empty */
- if (!zone_is_empty(z2))
- z2_end_pfn = zone_end_pfn(z2);
- else
- z2_end_pfn = end_pfn;
-
- resize_zone(z1, z1->zone_start_pfn, start_pfn);
- resize_zone(z2, start_pfn, z2_end_pfn);
-
- pgdat_resize_unlock(z1->zone_pgdat, &flags);
-
- fix_zone_id(z2, start_pfn, end_pfn);
-
- return 0;
-out_fail:
- pgdat_resize_unlock(z1->zone_pgdat, &flags);
- return -1;
-}
-
-static void __meminit grow_pgdat_span(struct pglist_data *pgdat, unsigned long start_pfn,
- unsigned long end_pfn)
-{
- unsigned long old_pgdat_end_pfn = pgdat_end_pfn(pgdat);
-
- if (!pgdat->node_spanned_pages || start_pfn < pgdat->node_start_pfn)
- pgdat->node_start_pfn = start_pfn;
-
- pgdat->node_spanned_pages = max(old_pgdat_end_pfn, end_pfn) -
- pgdat->node_start_pfn;
-}
-
-static int __meminit __add_zone(struct zone *zone, unsigned long phys_start_pfn)
-{
- struct pglist_data *pgdat = zone->zone_pgdat;
- int nr_pages = PAGES_PER_SECTION;
- int nid = pgdat->node_id;
- int zone_type;
- unsigned long flags, pfn;
-
- zone_type = zone - pgdat->node_zones;
- ensure_zone_is_initialized(zone, phys_start_pfn, nr_pages);
-
- pgdat_resize_lock(zone->zone_pgdat, &flags);
- grow_zone_span(zone, phys_start_pfn, phys_start_pfn + nr_pages);
- grow_pgdat_span(zone->zone_pgdat, phys_start_pfn,
- phys_start_pfn + nr_pages);
- pgdat_resize_unlock(zone->zone_pgdat, &flags);
- memmap_init_zone(nr_pages, nid, zone_type,
- phys_start_pfn, MEMMAP_HOTPLUG);
-
- /* online_page_range is called later and expects pages reserved */
- for (pfn = phys_start_pfn; pfn < phys_start_pfn + nr_pages; pfn++) {
- if (!pfn_valid(pfn))
- continue;
-
- SetPageReserved(pfn_to_page(pfn));
- }
- return 0;
-}
-
static int __meminit __add_section(int nid, unsigned long phys_start_pfn, bool want_memblock)
{
int ret;
@@ -1349,39 +1175,6 @@ static int check_hotplug_memory_range(u64 start, u64 size)
return 0;
}
-/*
- * If movable zone has already been setup, newly added memory should be check.
- * If its address is higher than movable zone, it should be added as movable.
- * Without this check, movable zone may overlap with other zone.
- */
-static int should_add_memory_movable(int nid, u64 start, u64 size)
-{
- unsigned long start_pfn = start >> PAGE_SHIFT;
- pg_data_t *pgdat = NODE_DATA(nid);
- struct zone *movable_zone = pgdat->node_zones + ZONE_MOVABLE;
-
- if (zone_is_empty(movable_zone))
- return 0;
-
- if (movable_zone->zone_start_pfn <= start_pfn)
- return 1;
-
- return 0;
-}
-
-int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
- bool for_device)
-{
-#ifdef CONFIG_ZONE_DEVICE
- if (for_device)
- return ZONE_DEVICE;
-#endif
- if (should_add_memory_movable(nid, start, size))
- return ZONE_MOVABLE;
-
- return zone_default;
-}
-
static int online_memory_block(struct memory_block *mem, void *arg)
{
return device_online(&mem->dev);
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2017-04-20 10:40 +0200 |
| Subject | Re: [PATCH 9/9] mm, memory_hotplug: remove unused cruft after memory hotplug rework |
| Message-ID | <tyfN9-6Do-29@gated-at.bofh.it> |
| In reply to | #1619816 |
On 04/10/2017 01:03 PM, Michal Hocko wrote: > From: Michal Hocko <mhocko@suse.com> > > zone_for_memory doesn't have any user anymore as well as the whole zone > shifting infrastructure so drop them all. > > This shouldn't introduce any functional changes. > > Signed-off-by: Michal Hocko <mhocko@suse.com> Acked-by: Vlastimil Babka <vbabka@suse.cz>
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-10 13:10 +0200 |
| Subject | [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section |
| Message-ID | <tuFmP-8iD-29@gated-at.bofh.it> |
| In reply to | #1619814 |
From: Michal Hocko <mhocko@suse.com>
device memory hotplug hooks into regular memory hotplug only half way.
It needs memory sections to track struct pages but there is no
need/desire to associate those sections with memory blocks and export
them to the userspace via sysfs because they cannot be onlined anyway.
This is currently expressed by for_device argument to arch_add_memory
which then makes sure to associate the given memory range with
ZONE_DEVICE. register_new_memory then relies on is_zone_device_section
to distinguish special memory hotplug from the regular one. While this
works now, later patches in this series want to move __add_zone outside
of arch_add_memory path so we have to come up with something else.
Add want_memblock down the __add_pages path and use it to control
whether the section->memblock association should be done. arch_add_memory
then just trivially want memblock for everything but for_device hotplug.
remove_memory_section doesn't need is_zone_device_section either. We can
simply skip all the memblock specific cleanup if there is no memblock
for the given section.
This shouldn't introduce any functional change.
Cc: Dan Williams <dan.j.williams@gmail.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
arch/ia64/mm/init.c | 2 +-
arch/powerpc/mm/mem.c | 2 +-
arch/s390/mm/init.c | 2 +-
arch/sh/mm/init.c | 2 +-
arch/x86/mm/init_32.c | 2 +-
arch/x86/mm/init_64.c | 2 +-
drivers/base/memory.c | 22 ++++++++--------------
include/linux/memory_hotplug.h | 2 +-
mm/memory_hotplug.c | 11 +++++++----
9 files changed, 22 insertions(+), 25 deletions(-)
diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 06cdaef54b2e..62085fd902e6 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -657,7 +657,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
zone = pgdat->node_zones +
zone_for_memory(nid, start, size, ZONE_NORMAL, for_device);
- ret = __add_pages(nid, zone, start_pfn, nr_pages);
+ ret = __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
if (ret)
printk("%s: Problem encountered in __add_pages() as ret=%d\n",
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 5f844337de21..ea3e09a62f38 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -149,7 +149,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
zone = pgdata->node_zones +
zone_for_memory(nid, start, size, 0, for_device);
- return __add_pages(nid, zone, start_pfn, nr_pages);
+ return __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
}
#ifdef CONFIG_MEMORY_HOTREMOVE
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index bf5b8a0c4ff7..5c84346e5211 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -182,7 +182,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
continue;
nr_pages = (start_pfn + size_pages > zone_end_pfn) ?
zone_end_pfn - start_pfn : size_pages;
- rc = __add_pages(nid, zone, start_pfn, nr_pages);
+ rc = __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
if (rc)
break;
start_pfn += nr_pages;
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index 75491862d900..a9d57f75ae8c 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -498,7 +498,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
ret = __add_pages(nid, pgdat->node_zones +
zone_for_memory(nid, start, size, ZONE_NORMAL,
for_device),
- start_pfn, nr_pages);
+ start_pfn, nr_pages, !for_device);
if (unlikely(ret))
printk("%s: Failed, __add_pages() == %d\n", __func__, ret);
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index c68078fd06fd..4b0f05328af0 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -834,7 +834,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- return __add_pages(nid, zone, start_pfn, nr_pages);
+ return __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
}
#ifdef CONFIG_MEMORY_HOTREMOVE
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 7eef17239378..39cfaee93975 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -652,7 +652,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
init_memory_mapping(start, start + size);
- ret = __add_pages(nid, zone, start_pfn, nr_pages);
+ ret = __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
WARN_ON_ONCE(ret);
/* update max_pfn, max_low_pfn and high_memory */
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index cc4f1d0cbffe..89c15e942852 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -685,14 +685,6 @@ static int add_memory_block(int base_section_nr)
return 0;
}
-static bool is_zone_device_section(struct mem_section *ms)
-{
- struct page *page;
-
- page = sparse_decode_mem_map(ms->section_mem_map, __section_nr(ms));
- return is_zone_device_page(page);
-}
-
/*
* need an interface for the VM to add new memory regions,
* but without onlining it.
@@ -702,9 +694,6 @@ int register_new_memory(int nid, struct mem_section *section)
int ret = 0;
struct memory_block *mem;
- if (is_zone_device_section(section))
- return 0;
-
mutex_lock(&mem_sysfs_mutex);
mem = find_memory_block(section);
@@ -741,11 +730,16 @@ static int remove_memory_section(unsigned long node_id,
{
struct memory_block *mem;
- if (is_zone_device_section(section))
- return 0;
-
mutex_lock(&mem_sysfs_mutex);
+
+ /*
+ * Some users of the memory hotplug do not want/need memblock to
+ * track all sections. Skip over those.
+ */
mem = find_memory_block(section);
+ if (!mem)
+ return 0;
+
unregister_mem_sect_under_nodes(mem, __section_nr(section));
mem->section_count--;
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 134a2f69c21a..3c8cf86201c3 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -111,7 +111,7 @@ extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
/* reasonably generic interface to expand the physical pages in a zone */
extern int __add_pages(int nid, struct zone *zone, unsigned long start_pfn,
- unsigned long nr_pages);
+ unsigned long nr_pages, bool want_memblock);
#ifdef CONFIG_NUMA
extern int memory_add_physaddr_to_nid(u64 start);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 342332f29364..1570b3eea493 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -493,7 +493,7 @@ static int __meminit __add_zone(struct zone *zone, unsigned long phys_start_pfn)
}
static int __meminit __add_section(int nid, struct zone *zone,
- unsigned long phys_start_pfn)
+ unsigned long phys_start_pfn, bool want_memblock)
{
int ret;
@@ -510,7 +510,10 @@ static int __meminit __add_section(int nid, struct zone *zone,
if (ret < 0)
return ret;
- return register_new_memory(nid, __pfn_to_section(phys_start_pfn));
+ if (want_memblock)
+ ret = register_new_memory(nid, __pfn_to_section(phys_start_pfn));
+
+ return ret;
}
/*
@@ -520,7 +523,7 @@ static int __meminit __add_section(int nid, struct zone *zone,
* add the new pages.
*/
int __ref __add_pages(int nid, struct zone *zone, unsigned long phys_start_pfn,
- unsigned long nr_pages)
+ unsigned long nr_pages, bool want_memblock)
{
unsigned long i;
int err = 0;
@@ -548,7 +551,7 @@ int __ref __add_pages(int nid, struct zone *zone, unsigned long phys_start_pfn,
}
for (i = start_sec; i <= end_sec; i++) {
- err = __add_section(nid, zone, section_nr_to_pfn(i));
+ err = __add_section(nid, zone, section_nr_to_pfn(i), want_memblock);
/*
* EEXIST is finally dealt with by ioresource collision
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Jerome Glisse <jglisse@redhat.com> |
|---|---|
| Date | 2017-04-10 18:30 +0200 |
| Subject | Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section |
| Message-ID | <tuKmu-341-7@gated-at.bofh.it> |
| In reply to | #1619818 |
On Mon, Apr 10, 2017 at 01:03:46PM +0200, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> device memory hotplug hooks into regular memory hotplug only half way.
> It needs memory sections to track struct pages but there is no
> need/desire to associate those sections with memory blocks and export
> them to the userspace via sysfs because they cannot be onlined anyway.
>
> This is currently expressed by for_device argument to arch_add_memory
> which then makes sure to associate the given memory range with
> ZONE_DEVICE. register_new_memory then relies on is_zone_device_section
> to distinguish special memory hotplug from the regular one. While this
> works now, later patches in this series want to move __add_zone outside
> of arch_add_memory path so we have to come up with something else.
>
> Add want_memblock down the __add_pages path and use it to control
> whether the section->memblock association should be done. arch_add_memory
> then just trivially want memblock for everything but for_device hotplug.
>
> remove_memory_section doesn't need is_zone_device_section either. We can
> simply skip all the memblock specific cleanup if there is no memblock
> for the given section.
>
> This shouldn't introduce any functional change.
>
> Cc: Dan Williams <dan.j.williams@gmail.com>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
[...]
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 342332f29364..1570b3eea493 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -493,7 +493,7 @@ static int __meminit __add_zone(struct zone *zone, unsigned long phys_start_pfn)
> }
>
> static int __meminit __add_section(int nid, struct zone *zone,
> - unsigned long phys_start_pfn)
> + unsigned long phys_start_pfn, bool want_memblock)
> {
> int ret;
>
> @@ -510,7 +510,10 @@ static int __meminit __add_section(int nid, struct zone *zone,
> if (ret < 0)
> return ret;
>
> - return register_new_memory(nid, __pfn_to_section(phys_start_pfn));
> + if (want_memblock)
> + ret = register_new_memory(nid, __pfn_to_section(phys_start_pfn));
> +
> + return ret;
> }
The above is wrong for ZONE_DEVICE sparse_add_one_section() will return a
positive value (on success) thus ret > 0 and other function in the hotplug
path will interpret positive value as an error.
I suggest something like:
if (!want_memblock)
return 0;
return register_new_memory(nid, __pfn_to_section(phys_start_pfn));
}
instead (also avoid a > 80 columns warning message).
Cheers,
Jérôme
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-10 18:40 +0200 |
| Subject | Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section |
| Message-ID | <tuKwa-38R-23@gated-at.bofh.it> |
| In reply to | #1620217 |
On Mon 10-04-17 12:20:02, Jerome Glisse wrote:
> On Mon, Apr 10, 2017 at 01:03:46PM +0200, Michal Hocko wrote:
[...]
> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index 342332f29364..1570b3eea493 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -493,7 +493,7 @@ static int __meminit __add_zone(struct zone *zone, unsigned long phys_start_pfn)
> > }
> >
> > static int __meminit __add_section(int nid, struct zone *zone,
> > - unsigned long phys_start_pfn)
> > + unsigned long phys_start_pfn, bool want_memblock)
> > {
> > int ret;
> >
> > @@ -510,7 +510,10 @@ static int __meminit __add_section(int nid, struct zone *zone,
> > if (ret < 0)
> > return ret;
> >
> > - return register_new_memory(nid, __pfn_to_section(phys_start_pfn));
> > + if (want_memblock)
> > + ret = register_new_memory(nid, __pfn_to_section(phys_start_pfn));
> > +
> > + return ret;
> > }
>
> The above is wrong for ZONE_DEVICE sparse_add_one_section() will return a
> positive value (on success) thus ret > 0 and other function in the hotplug
> path will interpret positive value as an error.
>
> I suggest something like:
> if (!want_memblock)
> return 0;
>
> return register_new_memory(nid, __pfn_to_section(phys_start_pfn));
> }
You are right! I will fold the following. Thanks!
---
From cc44b4a465b889910e74b3ccc2d12f4dd1c79065 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Mon, 10 Apr 2017 18:29:11 +0200
Subject: [PATCH] fold me "mm, memory_hotplug: get rid of
is_zone_device_section"
- return 0 want_memblock == 0 from __add_section as per Jerome Glisse
---
mm/memory_hotplug.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 035165ceefef..9942d8937d0a 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -299,7 +299,8 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
}
#endif /* CONFIG_HAVE_BOOTMEM_INFO_NODE */
-static int __meminit __add_section(int nid, unsigned long phys_start_pfn, bool want_memblock)
+static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
+ bool want_memblock)
{
int ret;
int i;
@@ -328,10 +329,10 @@ static int __meminit __add_section(int nid, unsigned long phys_start_pfn, bool w
SetPageReserved(page);
}
- if (want_memblock)
- ret = register_new_memory(nid, __pfn_to_section(phys_start_pfn));
+ if (!want_memblock)
+ return 0
- return ret;
+ return register_new_memory(nid, __pfn_to_section(phys_start_pfn));
}
/*
--
2.11.0
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2017-04-13 15:10 +0200 |
| Subject | Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section |
| Message-ID | <tvMFz-3qV-13@gated-at.bofh.it> |
| In reply to | #1619818 |
On 04/10/2017 01:03 PM, Michal Hocko wrote: > From: Michal Hocko <mhocko@suse.com> > > device memory hotplug hooks into regular memory hotplug only half way. > It needs memory sections to track struct pages but there is no > need/desire to associate those sections with memory blocks and export > them to the userspace via sysfs because they cannot be onlined anyway. > > This is currently expressed by for_device argument to arch_add_memory > which then makes sure to associate the given memory range with > ZONE_DEVICE. register_new_memory then relies on is_zone_device_section > to distinguish special memory hotplug from the regular one. While this > works now, later patches in this series want to move __add_zone outside > of arch_add_memory path so we have to come up with something else. > > Add want_memblock down the __add_pages path and use it to control > whether the section->memblock association should be done. arch_add_memory > then just trivially want memblock for everything but for_device hotplug. > > remove_memory_section doesn't need is_zone_device_section either. We can > simply skip all the memblock specific cleanup if there is no memblock > for the given section. > > This shouldn't introduce any functional change. > > Cc: Dan Williams <dan.j.williams@gmail.com> > Signed-off-by: Michal Hocko <mhocko@suse.com> For the fixed version: Acked-by: Vlastimil Babka <vbabka@suse.cz>
[toc] | [prev] | [next] | [standalone]
| From | Jerome Glisse <jglisse@redhat.com> |
|---|---|
| Date | 2017-04-17 22:20 +0200 |
| Subject | Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section |
| Message-ID | <txlhU-59s-13@gated-at.bofh.it> |
| In reply to | #1619818 |
On Mon, Apr 10, 2017 at 01:03:46PM +0200, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> device memory hotplug hooks into regular memory hotplug only half way.
> It needs memory sections to track struct pages but there is no
> need/desire to associate those sections with memory blocks and export
> them to the userspace via sysfs because they cannot be onlined anyway.
>
> This is currently expressed by for_device argument to arch_add_memory
> which then makes sure to associate the given memory range with
> ZONE_DEVICE. register_new_memory then relies on is_zone_device_section
> to distinguish special memory hotplug from the regular one. While this
> works now, later patches in this series want to move __add_zone outside
> of arch_add_memory path so we have to come up with something else.
>
> Add want_memblock down the __add_pages path and use it to control
> whether the section->memblock association should be done. arch_add_memory
> then just trivially want memblock for everything but for_device hotplug.
>
> remove_memory_section doesn't need is_zone_device_section either. We can
> simply skip all the memblock specific cleanup if there is no memblock
> for the given section.
>
> This shouldn't introduce any functional change.
>
> Cc: Dan Williams <dan.j.williams@gmail.com>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
> arch/ia64/mm/init.c | 2 +-
> arch/powerpc/mm/mem.c | 2 +-
> arch/s390/mm/init.c | 2 +-
> arch/sh/mm/init.c | 2 +-
> arch/x86/mm/init_32.c | 2 +-
> arch/x86/mm/init_64.c | 2 +-
> drivers/base/memory.c | 22 ++++++++--------------
> include/linux/memory_hotplug.h | 2 +-
> mm/memory_hotplug.c | 11 +++++++----
> 9 files changed, 22 insertions(+), 25 deletions(-)
>
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 06cdaef54b2e..62085fd902e6 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -657,7 +657,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>
> zone = pgdat->node_zones +
> zone_for_memory(nid, start, size, ZONE_NORMAL, for_device);
> - ret = __add_pages(nid, zone, start_pfn, nr_pages);
> + ret = __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
>
> if (ret)
> printk("%s: Problem encountered in __add_pages() as ret=%d\n",
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 5f844337de21..ea3e09a62f38 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -149,7 +149,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> zone = pgdata->node_zones +
> zone_for_memory(nid, start, size, 0, for_device);
>
> - return __add_pages(nid, zone, start_pfn, nr_pages);
> + return __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
> }
>
> #ifdef CONFIG_MEMORY_HOTREMOVE
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index bf5b8a0c4ff7..5c84346e5211 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -182,7 +182,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> continue;
> nr_pages = (start_pfn + size_pages > zone_end_pfn) ?
> zone_end_pfn - start_pfn : size_pages;
> - rc = __add_pages(nid, zone, start_pfn, nr_pages);
> + rc = __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
> if (rc)
> break;
> start_pfn += nr_pages;
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index 75491862d900..a9d57f75ae8c 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -498,7 +498,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> ret = __add_pages(nid, pgdat->node_zones +
> zone_for_memory(nid, start, size, ZONE_NORMAL,
> for_device),
> - start_pfn, nr_pages);
> + start_pfn, nr_pages, !for_device);
> if (unlikely(ret))
> printk("%s: Failed, __add_pages() == %d\n", __func__, ret);
>
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index c68078fd06fd..4b0f05328af0 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -834,7 +834,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
>
> - return __add_pages(nid, zone, start_pfn, nr_pages);
> + return __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
> }
>
> #ifdef CONFIG_MEMORY_HOTREMOVE
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 7eef17239378..39cfaee93975 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -652,7 +652,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>
> init_memory_mapping(start, start + size);
>
> - ret = __add_pages(nid, zone, start_pfn, nr_pages);
> + ret = __add_pages(nid, zone, start_pfn, nr_pages, !for_device);
> WARN_ON_ONCE(ret);
>
> /* update max_pfn, max_low_pfn and high_memory */
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index cc4f1d0cbffe..89c15e942852 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -685,14 +685,6 @@ static int add_memory_block(int base_section_nr)
> return 0;
> }
>
> -static bool is_zone_device_section(struct mem_section *ms)
> -{
> - struct page *page;
> -
> - page = sparse_decode_mem_map(ms->section_mem_map, __section_nr(ms));
> - return is_zone_device_page(page);
> -}
> -
> /*
> * need an interface for the VM to add new memory regions,
> * but without onlining it.
> @@ -702,9 +694,6 @@ int register_new_memory(int nid, struct mem_section *section)
> int ret = 0;
> struct memory_block *mem;
>
> - if (is_zone_device_section(section))
> - return 0;
> -
> mutex_lock(&mem_sysfs_mutex);
>
> mem = find_memory_block(section);
> @@ -741,11 +730,16 @@ static int remove_memory_section(unsigned long node_id,
> {
> struct memory_block *mem;
>
> - if (is_zone_device_section(section))
> - return 0;
> -
> mutex_lock(&mem_sysfs_mutex);
> +
> + /*
> + * Some users of the memory hotplug do not want/need memblock to
> + * track all sections. Skip over those.
> + */
> mem = find_memory_block(section);
> + if (!mem)
> + return 0;
> +
Another bug above spoted by Evgeny Baskakov from NVidia, mutex unlock
is missing ie something like:
if (!mem) {
mutex_unlock(&mem_sysfs_mutex);
return 0;
}
Between when are you planning on reposting ? I was hoping sometime soon
so i can repost HMM on top. I know with springtime celebration eveyrone
is out collecting chocolate eggs :)
Cheers,
Jérôme
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-18 09:20 +0200 |
| Subject | Re: [PATCH 4/9] mm, memory_hotplug: get rid of is_zone_device_section |
| Message-ID | <txvAC-3i3-17@gated-at.bofh.it> |
| In reply to | #1624863 |
On Mon 17-04-17 16:12:35, Jerome Glisse wrote:
[...]
> > @@ -741,11 +730,16 @@ static int remove_memory_section(unsigned long node_id,
> > {
> > struct memory_block *mem;
> >
> > - if (is_zone_device_section(section))
> > - return 0;
> > -
> > mutex_lock(&mem_sysfs_mutex);
> > +
> > + /*
> > + * Some users of the memory hotplug do not want/need memblock to
> > + * track all sections. Skip over those.
> > + */
> > mem = find_memory_block(section);
> > + if (!mem)
> > + return 0;
> > +
>
> Another bug above spoted by Evgeny Baskakov from NVidia, mutex unlock
> is missing ie something like:
>
> if (!mem) {
> mutex_unlock(&mem_sysfs_mutex);
> return 0;
> }
Thanks for spotting this. I went with the following fixup
---
From 38efdaf68b5c79df953385e9385581f75d46e651 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Tue, 18 Apr 2017 09:17:31 +0200
Subject: [PATCH] fold me "mm, memory_hotplug: get rid of
is_zone_device_section"
- fix remove_memory_section unlock on find_memory_block failure
as per Jerome - spotted by Evgeny Baskakov
---
drivers/base/memory.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 45c25e2e3da4..5ae81617f11d 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -746,7 +746,7 @@ static int remove_memory_section(unsigned long node_id,
*/
mem = find_memory_block(section);
if (!mem)
- return 0;
+ goto out_unlock;
unregister_mem_sect_under_nodes(mem, __section_nr(section));
@@ -756,6 +756,7 @@ static int remove_memory_section(unsigned long node_id,
else
put_device(&mem->dev);
+out_unlock:
mutex_unlock(&mem_sysfs_mutex);
return 0;
}
--
2.11.0
> Between when are you planning on reposting ?
this weak, the sooner the better.
Thanks!
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-10 13:10 +0200 |
| Subject | [PATCH 5/9] mm, memory_hotplug: split up register_one_node |
| Message-ID | <tuFmP-8iD-31@gated-at.bofh.it> |
| In reply to | #1619814 |
From: Michal Hocko <mhocko@suse.com>
Memory hotplug (add_memory_resource) has to reinitialize node
infrastructure if the node is offline (one which went through the
complete add_memory(); remove_memory() cycle). That involves node
registration to the kobj infrastructure (register_node), the proper
association with cpus (register_cpu_under_node) and finally creation of
node<->memblock symlinks (link_mem_sections).
The last part requires to know node_start_pfn and node_spanned_pages
which we currently have but a leter patch will postpone this
initialization to the onlining phase which happens later. In fact we do
not need to rely on the early pgdat initialization even now because the
currently hot added pfn range is currently known.
Split register_one_node into core which does all the common work for
the boot time NUMA initialization and the hotplug (__register_one_node).
register_one_node keeps the full initialization while hotplug calls
__register_one_node and manually calls link_mem_sections for the proper
range.
This shouldn't introduce any functional change.
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
drivers/base/node.c | 51 ++++++++++++++++++++-------------------------------
include/linux/node.h | 35 ++++++++++++++++++++++++++++++++++-
mm/memory_hotplug.c | 17 ++++++++++++++++-
3 files changed, 70 insertions(+), 33 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index 06294d69779b..dff5b53f7905 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -461,10 +461,9 @@ int unregister_mem_sect_under_nodes(struct memory_block *mem_blk,
return 0;
}
-static int link_mem_sections(int nid)
+int link_mem_sections(int nid, unsigned long start_pfn, unsigned long nr_pages)
{
- unsigned long start_pfn = NODE_DATA(nid)->node_start_pfn;
- unsigned long end_pfn = start_pfn + NODE_DATA(nid)->node_spanned_pages;
+ unsigned long end_pfn = start_pfn + nr_pages;
unsigned long pfn;
struct memory_block *mem_blk = NULL;
int err = 0;
@@ -552,10 +551,7 @@ static int node_memory_callback(struct notifier_block *self,
return NOTIFY_OK;
}
#endif /* CONFIG_HUGETLBFS */
-#else /* !CONFIG_MEMORY_HOTPLUG_SPARSE */
-
-static int link_mem_sections(int nid) { return 0; }
-#endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */
+#endif /* CONFIG_MEMORY_HOTPLUG_SPARSE */
#if !defined(CONFIG_MEMORY_HOTPLUG_SPARSE) || \
!defined(CONFIG_HUGETLBFS)
@@ -569,39 +565,32 @@ static void init_node_hugetlb_work(int nid) { }
#endif
-int register_one_node(int nid)
+int __register_one_node(int nid)
{
- int error = 0;
+ int p_node = parent_node(nid);
+ struct node *parent = NULL;
+ int error;
int cpu;
- if (node_online(nid)) {
- int p_node = parent_node(nid);
- struct node *parent = NULL;
-
- if (p_node != nid)
- parent = node_devices[p_node];
-
- node_devices[nid] = kzalloc(sizeof(struct node), GFP_KERNEL);
- if (!node_devices[nid])
- return -ENOMEM;
-
- error = register_node(node_devices[nid], nid, parent);
+ if (p_node != nid)
+ parent = node_devices[p_node];
- /* link cpu under this node */
- for_each_present_cpu(cpu) {
- if (cpu_to_node(cpu) == nid)
- register_cpu_under_node(cpu, nid);
- }
+ node_devices[nid] = kzalloc(sizeof(struct node), GFP_KERNEL);
+ if (!node_devices[nid])
+ return -ENOMEM;
- /* link memory sections under this node */
- error = link_mem_sections(nid);
+ error = register_node(node_devices[nid], nid, parent);
- /* initialize work queue for memory hot plug */
- init_node_hugetlb_work(nid);
+ /* link cpu under this node */
+ for_each_present_cpu(cpu) {
+ if (cpu_to_node(cpu) == nid)
+ register_cpu_under_node(cpu, nid);
}
- return error;
+ /* initialize work queue for memory hot plug */
+ init_node_hugetlb_work(nid);
+ return error;
}
void unregister_one_node(int nid)
diff --git a/include/linux/node.h b/include/linux/node.h
index 2115ad5d6f19..d1751beb462c 100644
--- a/include/linux/node.h
+++ b/include/linux/node.h
@@ -30,9 +30,38 @@ struct memory_block;
extern struct node *node_devices[];
typedef void (*node_registration_func_t)(struct node *);
+#if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_NUMA)
+extern int link_mem_sections(int nid, unsigned long start_pfn, unsigned long nr_pages);
+#else
+static inline int link_mem_sections(int nid, unsigned long start_pfn, unsigned long nr_pages)
+{
+ return 0;
+}
+#endif
+
extern void unregister_node(struct node *node);
#ifdef CONFIG_NUMA
-extern int register_one_node(int nid);
+/* Core of the node registration - only memory hotplug should use this */
+extern int __register_one_node(int nid);
+
+/* Registers an online node */
+static inline int register_one_node(int nid)
+{
+ int error = 0;
+
+ if (node_online(nid)) {
+ struct pglist_data *pgdat = NODE_DATA(nid);
+
+ error = __register_one_node(nid);
+ if (error)
+ return error;
+ /* link memory sections under this node */
+ error = link_mem_sections(nid, pgdat->node_start_pfn, pgdat->node_spanned_pages);
+ }
+
+ return error;
+}
+
extern void unregister_one_node(int nid);
extern int register_cpu_under_node(unsigned int cpu, unsigned int nid);
extern int unregister_cpu_under_node(unsigned int cpu, unsigned int nid);
@@ -46,6 +75,10 @@ extern void register_hugetlbfs_with_node(node_registration_func_t doregister,
node_registration_func_t unregister);
#endif
#else
+static inline int __register_one_node(int nid)
+{
+ return 0;
+}
static inline int register_one_node(int nid)
{
return 0;
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 1570b3eea493..f5df0fe15ddf 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1387,7 +1387,22 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
node_set_online(nid);
if (new_node) {
- ret = register_one_node(nid);
+ unsigned long start_pfn = start >> PAGE_SHIFT;
+ unsigned long nr_pages = size >> PAGE_SHIFT;
+
+ ret = __register_one_node(nid);
+ if (ret)
+ goto register_fail;
+
+ /*
+ * link memory sections under this node. This is already
+ * done when creatig memory section in register_new_memory
+ * but that depends to have the node registered so offline
+ * nodes have to go through register_node.
+ * TODO clean up this mess.
+ */
+ ret = link_mem_sections(nid, start_pfn, nr_pages);
+register_fail:
/*
* If sysfs file of new node can't create, cpu on the node
* can't be hot-added. There is no rollback way now.
--
2.11.0
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2017-04-13 16:10 +0200 |
| Subject | Re: [PATCH 5/9] mm, memory_hotplug: split up register_one_node |
| Message-ID | <tvNBE-43N-23@gated-at.bofh.it> |
| In reply to | #1619819 |
On 04/10/2017 01:03 PM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> Memory hotplug (add_memory_resource) has to reinitialize node
> infrastructure if the node is offline (one which went through the
> complete add_memory(); remove_memory() cycle). That involves node
> registration to the kobj infrastructure (register_node), the proper
> association with cpus (register_cpu_under_node) and finally creation of
> node<->memblock symlinks (link_mem_sections).
>
> The last part requires to know node_start_pfn and node_spanned_pages
> which we currently have but a leter patch will postpone this
> initialization to the onlining phase which happens later. In fact we do
> not need to rely on the early pgdat initialization even now because the
> currently hot added pfn range is currently known.
>
> Split register_one_node into core which does all the common work for
> the boot time NUMA initialization and the hotplug (__register_one_node).
> register_one_node keeps the full initialization while hotplug calls
> __register_one_node and manually calls link_mem_sections for the proper
> range.
>
> This shouldn't introduce any functional change.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
nit:
> @@ -1387,7 +1387,22 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
> node_set_online(nid);
>
> if (new_node) {
> - ret = register_one_node(nid);
> + unsigned long start_pfn = start >> PAGE_SHIFT;
> + unsigned long nr_pages = size >> PAGE_SHIFT;
> +
> + ret = __register_one_node(nid);
> + if (ret)
> + goto register_fail;
> +
> + /*
> + * link memory sections under this node. This is already
> + * done when creatig memory section in register_new_memory
> + * but that depends to have the node registered so offline
> + * nodes have to go through register_node.
> + * TODO clean up this mess.
Is this a work-in-progress or final TODO? :)
> + */
> + ret = link_mem_sections(nid, start_pfn, nr_pages);
> +register_fail:
> /*
> * If sysfs file of new node can't create, cpu on the node
> * can't be hot-added. There is no rollback way now.
>
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-13 16:20 +0200 |
| Subject | Re: [PATCH 5/9] mm, memory_hotplug: split up register_one_node |
| Message-ID | <tvNLk-47F-11@gated-at.bofh.it> |
| In reply to | #1623064 |
On Thu 13-04-17 16:05:17, Vlastimil Babka wrote:
> On 04/10/2017 01:03 PM, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@suse.com>
> >
> > Memory hotplug (add_memory_resource) has to reinitialize node
> > infrastructure if the node is offline (one which went through the
> > complete add_memory(); remove_memory() cycle). That involves node
> > registration to the kobj infrastructure (register_node), the proper
> > association with cpus (register_cpu_under_node) and finally creation of
> > node<->memblock symlinks (link_mem_sections).
> >
> > The last part requires to know node_start_pfn and node_spanned_pages
> > which we currently have but a leter patch will postpone this
> > initialization to the onlining phase which happens later. In fact we do
> > not need to rely on the early pgdat initialization even now because the
> > currently hot added pfn range is currently known.
> >
> > Split register_one_node into core which does all the common work for
> > the boot time NUMA initialization and the hotplug (__register_one_node).
> > register_one_node keeps the full initialization while hotplug calls
> > __register_one_node and manually calls link_mem_sections for the proper
> > range.
> >
> > This shouldn't introduce any functional change.
> >
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
Thanks!
> nit:
> > @@ -1387,7 +1387,22 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
> > node_set_online(nid);
> >
> > if (new_node) {
> > - ret = register_one_node(nid);
> > + unsigned long start_pfn = start >> PAGE_SHIFT;
> > + unsigned long nr_pages = size >> PAGE_SHIFT;
> > +
> > + ret = __register_one_node(nid);
> > + if (ret)
> > + goto register_fail;
> > +
> > + /*
> > + * link memory sections under this node. This is already
> > + * done when creatig memory section in register_new_memory
> > + * but that depends to have the node registered so offline
> > + * nodes have to go through register_node.
> > + * TODO clean up this mess.
>
> Is this a work-in-progress or final TODO? :)
I do not plan to address it in this series, but I will revisit it later.
There are more like this in other patches.
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Igor Mammedov <imammedo@redhat.com> |
|---|---|
| Date | 2017-04-10 16:30 +0200 |
| Message-ID | <tuIum-1Rg-23@gated-at.bofh.it> |
| In reply to | #1619814 |
On Mon, 10 Apr 2017 13:03:42 +0200 Michal Hocko <mhocko@kernel.org> wrote: > Hi, > The last version of this series has been posted here [1]. It has seen > some more serious testing (thanks to Reza Arbab) and fixes for the found > issues. I have also decided to drop patch 1 [2] because it turned out to > be more complicated than I initially thought [3]. Few more patches were > added to deal with expectation on zone/node initialization. > > I have rebased on top of the current mmotm-2017-04-07-15-53. It > conflicts with HMM because it touches memory hotplug as > well. We have discussed [4] with Jérôme and he agreed to > rebase on top of this rework [5] so I have reverted his series > before applyig mine. I will help him to resolve the resulting > conflicts. You can find the whole series including the HMM revers in > git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git branch > attempts/rewrite-mem_hotplug > > Motivation: > Movable onlining is a real hack with many downsides - mainly > reintroduction of lowmem/highmem issues we used to have on 32b systems - > but it is the only way to make the memory hotremove more reliable which > is something that people are asking for. > > The current semantic of memory movable onlinening is really cumbersome, > however. The main reason for this is that the udev driven approach is > basically unusable because udev races with the memory probing while only > the last memory block or the one adjacent to the existing zone_movable > are allowed to be onlined movable. In short the criterion for the > successful online_movable changes under udev's feet. A reliable udev > approach would require a 2 phase approach where the first successful > movable online would have to check all the previous blocks and online > them in descending order. This is hard to be considered sane. > > This patchset aims at making the onlining semantic more usable. First of > all it allows to online memory movable as long as it doesn't clash with > the existing ZONE_NORMAL. That means that ZONE_NORMAL and ZONE_MOVABLE > cannot overlap. Currently I preserve the original ordering semantic so > the zone always precedes the movable zone but I have plans to remove this > restriction in future because it is not really necessary. > > First 3 patches are cleanups which should be ready to be merged right > away (unless I have missed something subtle of course). > > Patch 4 deals with ZONE_DEVICE dependencies down the __add_pages path. > > Patch 5 deals with implicit assumptions of register_one_node on pgdat > initialization. > > Patch 6 is the core of the change. In order to make it easier to review > I have tried it to be as minimalistic as possible and the large code > removal is moved to patch 9. > > Patch 7 is a trivial follow up cleanup. Patch 8 fixes sparse warnings > and finally patch 9 removes the unused code. > > I have tested the patches in kvm: > # qemu-system-x86_64 -enable-kvm -monitor pty -m 2G,slots=4,maxmem=4G -numa node,mem=1G -numa node,mem=1G ... > > and then probed the additional memory by > (qemu) object_add memory-backend-ram,id=mem1,size=1G > (qemu) device_add pc-dimm,id=dimm1,memdev=mem1 Hi Michal, I've given series some dumb testing, see below for unexpected changes I've noticed. Using the same CLI as above plus hotpluggable dimms present at startup (it still uses hotplug path as dimms aren't reported in e820) -object memory-backend-ram,id=mem1,size=256M -object memory-backend-ram,id=mem0,size=256M \ -device pc-dimm,id=dimm1,memdev=mem1,slot=1,node=0 -device pc-dimm,id=dimm0,memdev=mem0,slot=0,node=0 so dimm1 => memory3[23] and dimm0 => memory3[45] #issue1: unable to online memblock as NORMAL adjacent to onlined MOVABLE 1: after boot memory32:offline removable: 0 zones: Normal Movable memory33:offline removable: 0 zones: Normal Movable memory34:offline removable: 0 zones: Normal Movable memory35:offline removable: 0 zones: Normal Movable 2: online as movable 1st dimm #echo online_movable > memory32/state #echo online_movable > memory33/state everything is as expected: memory32:online removable: 1 zones: Movable memory33:online removable: 1 zones: Movable memory34:offline removable: 0 zones: Movable memory35:offline removable: 0 zones: Movable 3: try to offline memory32 and online as NORMAL #echo offline > memory32/state memory32:offline removable: 1 zones: Normal Movable memory33:online removable: 1 zones: Movable memory34:offline removable: 0 zones: Movable memory35:offline removable: 0 zones: Movable #echo online_kernel > memory32/state write error: Invalid argument // that's not what's expected memory32:offline removable: 1 zones: Normal Movable memory33:online removable: 1 zones: Movable memory34:offline removable: 0 zones: Movable memory35:offline removable: 0 zones: Movable ====== #issue2: dimm1 assigned to node 1 on qemu CLI memblock is onlined as movable by default // after boot memory32:offline removable: 1 zones: Normal memory33:offline removable: 1 zones: Normal Movable memory34:offline removable: 1 zones: Normal memory35:offline removable: 1 zones: Normal Movable // not related to this issue but notice not all blocks are // "Normal Movable" when compared when both dimms on node 0 /#issue1/ #echo online_movable > memory33/state #echo online > memory32/state memory32:online removable: 1 zones: Movable memory33:online removable: 1 zones: Movable before series memory32 goes to zone NORMAL as expected memory32:online removable: 0 zones: Normal Movable memory33:online removable: 1 zones: Movable Normal ====== #issue3: removable flag flipped to non-removable state // before series at commit ef0b577b6: memory32:offline removable: 0 zones: Normal Movable memory33:offline removable: 0 zones: Normal Movable memory34:offline removable: 0 zones: Normal Movable memory35:offline removable: 0 zones: Normal Movable // after series at commit 6a010434 memory32:offline removable: 1 zones: Normal memory33:offline removable: 1 zones: Normal memory34:offline removable: 1 zones: Normal memory35:offline removable: 1 zones: Normal Movable also looking at #issue1 removable flag state doesn't seem to be consistent between state changes but maybe that's been broken before
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-10 17:00 +0200 |
| Message-ID | <tuIXo-21C-29@gated-at.bofh.it> |
| In reply to | #1619980 |
On Mon 10-04-17 16:27:49, Igor Mammedov wrote: [...] > Hi Michal, > > I've given series some dumb testing, see below for unexpected changes I've noticed. > > Using the same CLI as above plus hotpluggable dimms present at startup > (it still uses hotplug path as dimms aren't reported in e820) > > -object memory-backend-ram,id=mem1,size=256M -object memory-backend-ram,id=mem0,size=256M \ > -device pc-dimm,id=dimm1,memdev=mem1,slot=1,node=0 -device pc-dimm,id=dimm0,memdev=mem0,slot=0,node=0 > > so dimm1 => memory3[23] and dimm0 => memory3[45] > > #issue1: > unable to online memblock as NORMAL adjacent to onlined MOVABLE > > 1: after boot > memory32:offline removable: 0 zones: Normal Movable > memory33:offline removable: 0 zones: Normal Movable > memory34:offline removable: 0 zones: Normal Movable > memory35:offline removable: 0 zones: Normal Movable > > 2: online as movable 1st dimm > > #echo online_movable > memory32/state > #echo online_movable > memory33/state > > everything is as expected: > memory32:online removable: 1 zones: Movable > memory33:online removable: 1 zones: Movable > memory34:offline removable: 0 zones: Movable > memory35:offline removable: 0 zones: Movable > > 3: try to offline memory32 and online as NORMAL > > #echo offline > memory32/state > memory32:offline removable: 1 zones: Normal Movable > memory33:online removable: 1 zones: Movable > memory34:offline removable: 0 zones: Movable > memory35:offline removable: 0 zones: Movable OK, this is not expected. We are not shifting zones anymore so the range which was online_movable will not become available to the zone Normal. So this must be something broken down the show_valid_zones path. I will investigate. > > #echo online_kernel > memory32/state > write error: Invalid argument > // that's not what's expected this is proper behavior with the current implementation. Does anything depend on the zone reusing? > memory32:offline removable: 1 zones: Normal Movable > memory33:online removable: 1 zones: Movable > memory34:offline removable: 0 zones: Movable > memory35:offline removable: 0 zones: Movable > > > ====== > #issue2: dimm1 assigned to node 1 on qemu CLI > memblock is onlined as movable by default > > // after boot > memory32:offline removable: 1 zones: Normal > memory33:offline removable: 1 zones: Normal Movable > memory34:offline removable: 1 zones: Normal > memory35:offline removable: 1 zones: Normal Movable > // not related to this issue but notice not all blocks are > // "Normal Movable" when compared when both dimms on node 0 /#issue1/ yes they should be > #echo online_movable > memory33/state > #echo online > memory32/state > > memory32:online removable: 1 zones: Movable > memory33:online removable: 1 zones: Movable > > before series memory32 goes to zone NORMAL as expected > memory32:online removable: 0 zones: Normal Movable > memory33:online removable: 1 zones: Movable Normal OK, I will double check. > ====== > #issue3: > removable flag flipped to non-removable state > > // before series at commit ef0b577b6: > memory32:offline removable: 0 zones: Normal Movable > memory33:offline removable: 0 zones: Normal Movable > memory34:offline removable: 0 zones: Normal Movable > memory35:offline removable: 0 zones: Normal Movable > > // after series at commit 6a010434 > memory32:offline removable: 1 zones: Normal > memory33:offline removable: 1 zones: Normal > memory34:offline removable: 1 zones: Normal > memory35:offline removable: 1 zones: Normal Movable > > also looking at #issue1 removable flag state doesn't > seem to be consistent between state changes but maybe that's > been broken before OK, will have a look. Thanks for your testing! -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-10 17:30 +0200 |
| Message-ID | <tuJqq-2rm-27@gated-at.bofh.it> |
| In reply to | #1620004 |
[dropping Lai Jiangshan whose email bounces] On Mon 10-04-17 16:56:39, Michal Hocko wrote: > On Mon 10-04-17 16:27:49, Igor Mammedov wrote: > [...] > > Hi Michal, > > > > I've given series some dumb testing, see below for unexpected changes I've noticed. > > > > Using the same CLI as above plus hotpluggable dimms present at startup > > (it still uses hotplug path as dimms aren't reported in e820) > > > > -object memory-backend-ram,id=mem1,size=256M -object memory-backend-ram,id=mem0,size=256M \ > > -device pc-dimm,id=dimm1,memdev=mem1,slot=1,node=0 -device pc-dimm,id=dimm0,memdev=mem0,slot=0,node=0 > > > > so dimm1 => memory3[23] and dimm0 => memory3[45] > > > > #issue1: > > unable to online memblock as NORMAL adjacent to onlined MOVABLE > > > > 1: after boot > > memory32:offline removable: 0 zones: Normal Movable > > memory33:offline removable: 0 zones: Normal Movable > > memory34:offline removable: 0 zones: Normal Movable > > memory35:offline removable: 0 zones: Normal Movable > > > > 2: online as movable 1st dimm > > > > #echo online_movable > memory32/state > > #echo online_movable > memory33/state > > > > everything is as expected: > > memory32:online removable: 1 zones: Movable > > memory33:online removable: 1 zones: Movable > > memory34:offline removable: 0 zones: Movable > > memory35:offline removable: 0 zones: Movable > > > > 3: try to offline memory32 and online as NORMAL > > > > #echo offline > memory32/state > > memory32:offline removable: 1 zones: Normal Movable > > memory33:online removable: 1 zones: Movable > > memory34:offline removable: 0 zones: Movable > > memory35:offline removable: 0 zones: Movable > > OK, this is not expected. We are not shifting zones anymore so the range > which was online_movable will not become available to the zone Normal. > So this must be something broken down the show_valid_zones path. I will > investigate. Heh, this one is embarrassing diff --git a/drivers/base/memory.c b/drivers/base/memory.c index 1c6fdacbccd3..9677b6b711b0 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -402,7 +402,7 @@ static ssize_t show_valid_zones(struct device *dev, return sprintf(buf, "none\n"); start_pfn = valid_start_pfn; - nr_pages = valid_end_pfn - valid_end_pfn; + nr_pages = valid_end_pfn - start_pfn; /* * Check the existing zone. Make sure that we do that only on the -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-10 17:40 +0200 |
| Message-ID | <tuJA6-2uY-17@gated-at.bofh.it> |
| In reply to | #1620033 |
On Mon 10-04-17 17:22:28, Michal Hocko wrote:
[...]
> Heh, this one is embarrassing
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 1c6fdacbccd3..9677b6b711b0 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -402,7 +402,7 @@ static ssize_t show_valid_zones(struct device *dev,
> return sprintf(buf, "none\n");
>
> start_pfn = valid_start_pfn;
> - nr_pages = valid_end_pfn - valid_end_pfn;
> + nr_pages = valid_end_pfn - start_pfn;
>
> /*
> * Check the existing zone. Make sure that we do that only on the
Btw. while starting into the code I think that allow_online_pfn_range is
also wrong and we need the following
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 94e96ca790f6..035165ceefef 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -858,7 +858,7 @@ bool allow_online_pfn_range(int nid, unsigned long pfn, unsigned long nr_pages,
* TODO make sure we do not overlap with ZONE_DEVICE
*/
if (online_type == MMOP_ONLINE_KERNEL) {
- if (!populated_zone(movable_zone))
+ if (!movable_zone->spanned_pages)
return true;
return movable_zone->zone_start_pfn >= pfn + nr_pages;
} else if (online_type == MMOP_ONLINE_MOVABLE) {
because we would allow ZONE_NORMAL after the full movable zone has been
offlined.
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Igor Mammedov <imammedo@redhat.com> |
|---|---|
| Date | 2017-04-11 10:10 +0200 |
| Message-ID | <tuZ2b-4oA-35@gated-at.bofh.it> |
| In reply to | #1620004 |
On Mon, 10 Apr 2017 16:56:39 +0200 Michal Hocko <mhocko@kernel.org> wrote: > On Mon 10-04-17 16:27:49, Igor Mammedov wrote: > [...] > > Hi Michal, > > > > I've given series some dumb testing, see below for unexpected changes I've noticed. > > > > Using the same CLI as above plus hotpluggable dimms present at startup > > (it still uses hotplug path as dimms aren't reported in e820) > > > > -object memory-backend-ram,id=mem1,size=256M -object memory-backend-ram,id=mem0,size=256M \ > > -device pc-dimm,id=dimm1,memdev=mem1,slot=1,node=0 -device pc-dimm,id=dimm0,memdev=mem0,slot=0,node=0 > > > > so dimm1 => memory3[23] and dimm0 => memory3[45] > > > > #issue1: > > unable to online memblock as NORMAL adjacent to onlined MOVABLE > > > > 1: after boot > > memory32:offline removable: 0 zones: Normal Movable > > memory33:offline removable: 0 zones: Normal Movable > > memory34:offline removable: 0 zones: Normal Movable > > memory35:offline removable: 0 zones: Normal Movable > > > > 2: online as movable 1st dimm > > > > #echo online_movable > memory32/state > > #echo online_movable > memory33/state > > > > everything is as expected: > > memory32:online removable: 1 zones: Movable > > memory33:online removable: 1 zones: Movable > > memory34:offline removable: 0 zones: Movable > > memory35:offline removable: 0 zones: Movable > > > > 3: try to offline memory32 and online as NORMAL > > > > #echo offline > memory32/state > > memory32:offline removable: 1 zones: Normal Movable > > memory33:online removable: 1 zones: Movable > > memory34:offline removable: 0 zones: Movable > > memory35:offline removable: 0 zones: Movable > > OK, this is not expected. We are not shifting zones anymore so the range > which was online_movable will not become available to the zone Normal. > So this must be something broken down the show_valid_zones path. I will > investigate. > > > > > #echo online_kernel > memory32/state > > write error: Invalid argument > > // that's not what's expected > > this is proper behavior with the current implementation. Does anything > depend on the zone reusing? if we didn't have zone imbalance issue in design, the it wouldn't matter but as it stands it's not minore issue. Consider following, one hotplugs some memory and onlines it as movable, then one needs to hotplug some more but to do so one one needs more memory from zone NORMAL and to keep zone balance some memory in MOVABLE should be reonlined as NORMAL
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-04-11 10:50 +0200 |
| Message-ID | <tuZES-4BD-15@gated-at.bofh.it> |
| In reply to | #1621009 |
On Tue 11-04-17 10:01:52, Igor Mammedov wrote: > On Mon, 10 Apr 2017 16:56:39 +0200 > Michal Hocko <mhocko@kernel.org> wrote: [...] > > > #echo online_kernel > memory32/state > > > write error: Invalid argument > > > // that's not what's expected > > > > this is proper behavior with the current implementation. Does anything > > depend on the zone reusing? > if we didn't have zone imbalance issue in design, > the it wouldn't matter but as it stands it's not > minore issue. > > Consider following, > one hotplugs some memory and onlines it as movable, > then one needs to hotplug some more but to do so > one one needs more memory from zone NORMAL and to keep > zone balance some memory in MOVABLE should be reonlined > as NORMAL Is this something that we absolutely have to have right _now_? Or are you OK if I address this in follow up series? Because it will make the current code slightly more complex and to be honest I would rather like to see this "core" merge and build more on top. -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
Page 1 of 4 [1] 2 3 4 Next page →
Back to top | Article view | linux.kernel
csiph-web