Groups > linux.kernel > #1612985 > unrolled thread

[PATCH 0/6] mm: make movable onlining suck less

Started by	Michal Hocko <mhocko@kernel.org>
First post	2017-03-30 14:00 +0200
Last post	2017-04-06 17:50 +0200
Articles	20 on this page of 61 — 6 participants

Back to article view | Back to linux.kernel

  [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-03-30 14:00 +0200
    [PATCH 1/6] mm: get rid of zone_is_initialized Michal Hocko <mhocko@kernel.org> - 2017-03-30 14:00 +0200
      Re: [PATCH 1/6] mm: get rid of zone_is_initialized "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2017-03-31 05:50 +0200
        Re: [PATCH 1/6] mm: get rid of zone_is_initialized Michal Hocko <mhocko@kernel.org> - 2017-03-31 08:50 +0200
          Re: [PATCH 1/6] mm: get rid of zone_is_initialized Michal Hocko <mhocko@kernel.org> - 2017-03-31 08:50 +0200
      [PATCH v1 1/6] mm: get rid of zone_is_initialized Michal Hocko <mhocko@kernel.org> - 2017-03-31 09:50 +0200
        Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized Michal Hocko <mhocko@kernel.org> - 2017-04-05 10:20 +0200
          Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized Michal Hocko <mhocko@kernel.org> - 2017-04-05 11:30 +0200
          Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized Igor Mammedov <imammedo@redhat.com> - 2017-04-05 11:50 +0200
    [PATCH 2/6] mm, tile: drop arch_{add,remove}_memory Michal Hocko <mhocko@kernel.org> - 2017-03-30 14:00 +0200
    [PATCH 3/6] mm: remove return value from init_currently_empty_zone Michal Hocko <mhocko@kernel.org> - 2017-03-30 14:00 +0200
      Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2017-03-31 06:00 +0200
        Re: [PATCH 3/6] mm: remove return value from  init_currently_empty_zone Michal Hocko <mhocko@kernel.org> - 2017-03-31 08:50 +0200
          Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2017-03-31 09:10 +0200
            Re: [PATCH 3/6] mm: remove return value from  init_currently_empty_zone Michal Hocko <mhocko@kernel.org> - 2017-03-31 09:20 +0200
      Re: [PATCH 3/6] mm: remove return value from  init_currently_empty_zone Michal Hocko <mhocko@kernel.org> - 2017-03-31 09:50 +0200
      Re: [PATCH 3/6] mm: remove return value from  init_currently_empty_zone Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-03 23:30 +0200
        Re: [PATCH 3/6] mm: remove return value from  init_currently_empty_zone Michal Hocko <mhocko@kernel.org> - 2017-04-04 09:40 +0200
    [PATCH 6/6] mm, memory_hotplug: remove unused cruft after memory hotplug rework Michal Hocko <mhocko@kernel.org> - 2017-03-30 14:00 +0200
      Re: [PATCH 6/6] mm, memory_hotplug: remove unused cruft after memory  hotplug rework Michal Hocko <mhocko@kernel.org> - 2017-03-31 09:50 +0200
    Re: [PATCH 0/6] mm: make movable onlining suck less Heiko Carstens <heiko.carstens@de.ibm.com> - 2017-03-31 21:20 +0200
      Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-03 09:40 +0200
    Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-03 14:00 +0200
      Re: [PATCH 0/6] mm: make movable onlining suck less Igor Mammedov <imammedo@redhat.com> - 2017-04-03 14:30 +0200
      Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-03 22:00 +0200
        Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-03 22:30 +0200
          Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-03 22:50 +0200
            Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-04 09:30 +0200
              Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-04 09:40 +0200
                Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-04 10:30 +0200
                  Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-04 18:00 +0200
                  Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-04 18:10 +0200
                    Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-04 18:50 +0200
                      Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-04 20:40 +0200
                        Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-04 21:50 +0200
                          Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-04 23:50 +0200
                            Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-05 08:50 +0200
                              Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-05 11:30 +0200
                                Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-05 17:00 +0200
                                  Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-05 17:50 +0200
                                    Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-05 19:40 +0200
                                      Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-05 20:20 +0200
                                        Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-05 21:50 +0200
                                        Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-05 23:10 +0200
                                          Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-06 13:10 +0200
                              Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-05 18:00 +0200
                                Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-05 18:40 +0200
                                  Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-05 23:00 +0200
                                  Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-06 11:30 +0200
                            Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-05 16:00 +0200
                              Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-05 17:30 +0200
                          Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-05 08:40 +0200
    Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-06 15:10 +0200
      Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-06 17:30 +0200
        Re: [PATCH 0/6] mm: make movable onlining suck less Reza Arbab <arbab@linux.vnet.ibm.com> - 2017-04-06 17:50 +0200
          Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-06 18:30 +0200
            Re: [PATCH 0/6] mm: make movable onlining suck less Mel Gorman <mgorman@suse.de> - 2017-04-06 18:30 +0200
            Re: [PATCH 0/6] mm: make movable onlining suck less Mel Gorman <mgorman@suse.de> - 2017-04-06 19:00 +0200
              Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-06 19:20 +0200
                Re: [PATCH 0/6] mm: make movable onlining suck less Mel Gorman <mgorman@suse.de> - 2017-04-06 19:50 +0200
        Re: [PATCH 0/6] mm: make movable onlining suck less Michal Hocko <mhocko@kernel.org> - 2017-04-06 17:50 +0200

Page 1 of 4 [1] 2 3 4 Next page →

#1612985 — [PATCH 0/6] mm: make movable onlining suck less

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-30 14:00 +0200
Subject	[PATCH 0/6] mm: make movable onlining suck less
Message-ID	<tqGU9-5LE-3@gated-at.bofh.it>

Hi,
I have posted a crude RFC for this rework [1] and there didn't seem any
objections so I have split up the patch into smaller chunks which will
make the review easier hopefully.

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.

The series consists of 4 cleanup patches which should be ready to be
merged right away (unless I have missed something subtle of course).

Patch 5 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 6.

I would appreciate if s390 folks could take a look at patch 4 and the
arch_add_memory because I am not sure I've grokked what they wanted to
achieve there completely.

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            |  10 +-
 arch/powerpc/mm/mem.c          |  12 +-
 arch/s390/mm/init.c            |  32 +---
 arch/sh/mm/init.c              |  10 +-
 arch/tile/mm/init.c            |  30 ---
 arch/x86/mm/init_32.c          |   7 +-
 arch/x86/mm/init_64.c          |  11 +-
 drivers/base/memory.c          |  52 ++---
 include/linux/memory_hotplug.h |  17 +-
 include/linux/mmzone.h         |  13 +-
 kernel/memremap.c              |   5 +-
 mm/memory_hotplug.c            | 423 ++++++++++++-----------------------------
 mm/page_alloc.c                |  11 +-
 mm/sparse.c                    |   3 +-
 14 files changed, 186 insertions(+), 450 deletions(-)

Shortlog says:
Michal Hocko (6):
      mm: get rid of zone_is_initialized
      mm, tile: drop arch_{add,remove}_memory
      mm: remove return value from init_currently_empty_zone
      mm, memory_hotplug: use node instead of zone in can_online_high_movable
      mm, memory_hotplug: do not associate hotadded memory to zones until online
      mm, memory_hotplug: remove unused cruft after memory hotplug rework

[1] http://lkml.kernel.org/r/20170315091347.GA32626@dhcp22.suse.cz

[toc] | [next] | [standalone]

#1612986 — [PATCH 1/6] mm: get rid of zone_is_initialized

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-30 14:00 +0200
Subject	[PATCH 1/6] mm: get rid of zone_is_initialized
Message-ID	<tqGUa-5LE-15@gated-at.bofh.it>
In reply to	#1612985

From: Michal Hocko <mhocko@suse.com>

There shouldn't be any reason to add initialized when we can tell the
same thing from checking whether there are any pages spanned to the
zone. Remove zone_is_initialized() and replace it by zone_is_empty
which can be used for the same set of tests.

This shouldn't have any visible effect

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/mmzone.h | 11 +++++------
 mm/memory_hotplug.c    |  6 +++---
 mm/page_alloc.c        |  5 +----
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 618499159a7c..dbe3b32fe85d 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -442,8 +442,6 @@ struct zone {
 	seqlock_t		span_seqlock;
 #endif
 
-	int initialized;
-
 	/* Write-intensive fields used from the page allocator */
 	ZONE_PADDING(_pad1_)
 
@@ -520,14 +518,15 @@ static inline bool zone_spans_pfn(const struct zone *zone, unsigned long pfn)
 	return zone->zone_start_pfn <= pfn && pfn < zone_end_pfn(zone);
 }
 
-static inline bool zone_is_initialized(struct zone *zone)
+static inline bool zone_is_empty(struct zone *zone)
 {
-	return zone->initialized;
+	return zone->spanned_pages == 0;
 }
 
-static inline bool zone_is_empty(struct zone *zone)
+static inline bool zone_spans_range(const struct zone *zone, unsigned long start_pfn,
+		unsigned long nr_pages)
 {
-	return zone->spanned_pages == 0;
+	return zone->zone_start_pfn <= start_pfn && start_pfn + nr_pages < zone_end_pfn(zone);
 }
 
 /*
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 6fb6bd2df787..699f5a2a8efd 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -348,7 +348,7 @@ static void fix_zone_id(struct zone *zone, unsigned long start_pfn,
 static int __ref ensure_zone_is_initialized(struct zone *zone,
 			unsigned long start_pfn, unsigned long num_pages)
 {
-	if (!zone_is_initialized(zone))
+	if (zone_is_empty(zone))
 		return init_currently_empty_zone(zone, start_pfn, num_pages);
 
 	return 0;
@@ -1051,7 +1051,7 @@ bool zone_can_shift(unsigned long pfn, unsigned long nr_pages,
 
 		/* no zones in use between current zone and target */
 		for (i = idx + 1; i < target; i++)
-			if (zone_is_initialized(zone - idx + i))
+			if (!zone_is_empty(zone - idx + i))
 				return false;
 	}
 
@@ -1062,7 +1062,7 @@ bool zone_can_shift(unsigned long pfn, unsigned long nr_pages,
 
 		/* no zones in use between current zone and target */
 		for (i = target + 1; i < idx; i++)
-			if (zone_is_initialized(zone - idx + i))
+			if (!zone_is_empty(zone - idx + i))
 				return false;
 	}
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5ee8a26fa383..af58b51c5897 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -795,7 +795,7 @@ static inline void __free_one_page(struct page *page,
 
 	max_order = min_t(unsigned int, MAX_ORDER, pageblock_order + 1);
 
-	VM_BUG_ON(!zone_is_initialized(zone));
+	VM_BUG_ON(zone_is_empty(zone));
 	VM_BUG_ON_PAGE(page->flags & PAGE_FLAGS_CHECK_AT_PREP, page);
 
 	VM_BUG_ON(migratetype == -1);
@@ -5535,9 +5535,6 @@ int __meminit init_currently_empty_zone(struct zone *zone,
 			zone_start_pfn, (zone_start_pfn + size));
 
 	zone_init_free_lists(zone);
-	zone->initialized = 1;
-
-	return 0;
 }
 
 #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1613611 — Re: [PATCH 1/6] mm: get rid of zone_is_initialized

From	"Hillf Danton" <hillf.zj@alibaba-inc.com>
Date	2017-03-31 05:50 +0200
Subject	Re: [PATCH 1/6] mm: get rid of zone_is_initialized
Message-ID	<tqVJv-7zL-15@gated-at.bofh.it>
In reply to	#1612986

On March 30, 2017 7:55 PM Michal Hocko wrote:
> 
> @@ -5535,9 +5535,6 @@ int __meminit init_currently_empty_zone(struct zone *zone,
>  			zone_start_pfn, (zone_start_pfn + size));
> 
>  	zone_init_free_lists(zone);
> -	zone->initialized = 1;
> -
> -	return 0;
>  }
Nit: Add changes more than correct.

Hillf

[toc] | [prev] | [next] | [standalone]

#1613664 — Re: [PATCH 1/6] mm: get rid of zone_is_initialized

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-31 08:50 +0200
Subject	Re: [PATCH 1/6] mm: get rid of zone_is_initialized
Message-ID	<tqYxH-10x-5@gated-at.bofh.it>
In reply to	#1613611

On Fri 31-03-17 11:39:07, Hillf Danton wrote:
> 
> On March 30, 2017 7:55 PM Michal Hocko wrote:
> > 
> > @@ -5535,9 +5535,6 @@ int __meminit init_currently_empty_zone(struct zone *zone,
> >  			zone_start_pfn, (zone_start_pfn + size));
> > 
> >  	zone_init_free_lists(zone);
> > -	zone->initialized = 1;
> > -
> > -	return 0;
> >  }
> Nit: Add changes more than correct.

I am sorry, I do not follow?
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1613668 — Re: [PATCH 1/6] mm: get rid of zone_is_initialized

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-31 08:50 +0200
Subject	Re: [PATCH 1/6] mm: get rid of zone_is_initialized
Message-ID	<tqYxI-10x-9@gated-at.bofh.it>
In reply to	#1613664

On Fri 31-03-17 08:43:01, Michal Hocko wrote:
> On Fri 31-03-17 11:39:07, Hillf Danton wrote:
> > 
> > On March 30, 2017 7:55 PM Michal Hocko wrote:
> > > 
> > > @@ -5535,9 +5535,6 @@ int __meminit init_currently_empty_zone(struct zone *zone,
> > >  			zone_start_pfn, (zone_start_pfn + size));
> > > 
> > >  	zone_init_free_lists(zone);
> > > -	zone->initialized = 1;
> > > -
> > > -	return 0;
> > >  }
> > Nit: Add changes more than correct.
> 
> I am sorry, I do not follow?

OK, got it. init_currently_empty_zone should be changed to void. Screw
up during the split of the initial patch. Will fix that up. Also
zone_spans_range should go to patch 5 because it doesn't have any user
until then.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1613708 — [PATCH v1 1/6] mm: get rid of zone_is_initialized

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-31 09:50 +0200
Subject	[PATCH v1 1/6] mm: get rid of zone_is_initialized
Message-ID	<tqZtM-1Du-17@gated-at.bofh.it>
In reply to	#1612986

Fixed screw ups during the initial patch split up as per Hillf
---
From 8be6c5e47de66210e47710c80e72e8abd899017b Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Wed, 29 Mar 2017 15:11:30 +0200
Subject: [PATCH] mm: get rid of zone_is_initialized

There shouldn't be any reason to add initialized when we can tell the
same thing from checking whether there are any pages spanned to the
zone. Remove zone_is_initialized() and replace it by zone_is_empty
which can be used for the same set of tests.

This shouldn't have any visible effect

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/mmzone.h | 7 -------
 mm/memory_hotplug.c    | 6 +++---
 mm/page_alloc.c        | 3 +--
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 618499159a7c..3bac3ed71c7a 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -442,8 +442,6 @@ struct zone {
 	seqlock_t		span_seqlock;
 #endif
 
-	int initialized;
-
 	/* Write-intensive fields used from the page allocator */
 	ZONE_PADDING(_pad1_)
 
@@ -520,11 +518,6 @@ static inline bool zone_spans_pfn(const struct zone *zone, unsigned long pfn)
 	return zone->zone_start_pfn <= pfn && pfn < zone_end_pfn(zone);
 }
 
-static inline bool zone_is_initialized(struct zone *zone)
-{
-	return zone->initialized;
-}
-
 static inline bool zone_is_empty(struct zone *zone)
 {
 	return zone->spanned_pages == 0;
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 6fb6bd2df787..699f5a2a8efd 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -348,7 +348,7 @@ static void fix_zone_id(struct zone *zone, unsigned long start_pfn,
 static int __ref ensure_zone_is_initialized(struct zone *zone,
 			unsigned long start_pfn, unsigned long num_pages)
 {
-	if (!zone_is_initialized(zone))
+	if (zone_is_empty(zone))
 		return init_currently_empty_zone(zone, start_pfn, num_pages);
 
 	return 0;
@@ -1051,7 +1051,7 @@ bool zone_can_shift(unsigned long pfn, unsigned long nr_pages,
 
 		/* no zones in use between current zone and target */
 		for (i = idx + 1; i < target; i++)
-			if (zone_is_initialized(zone - idx + i))
+			if (!zone_is_empty(zone - idx + i))
 				return false;
 	}
 
@@ -1062,7 +1062,7 @@ bool zone_can_shift(unsigned long pfn, unsigned long nr_pages,
 
 		/* no zones in use between current zone and target */
 		for (i = target + 1; i < idx; i++)
-			if (zone_is_initialized(zone - idx + i))
+			if (!zone_is_empty(zone - idx + i))
 				return false;
 	}
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5ee8a26fa383..756353d1e293 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -795,7 +795,7 @@ static inline void __free_one_page(struct page *page,
 
 	max_order = min_t(unsigned int, MAX_ORDER, pageblock_order + 1);
 
-	VM_BUG_ON(!zone_is_initialized(zone));
+	VM_BUG_ON(zone_is_empty(zone));
 	VM_BUG_ON_PAGE(page->flags & PAGE_FLAGS_CHECK_AT_PREP, page);
 
 	VM_BUG_ON(migratetype == -1);
@@ -5535,7 +5535,6 @@ int __meminit init_currently_empty_zone(struct zone *zone,
 			zone_start_pfn, (zone_start_pfn + size));
 
 	zone_init_free_lists(zone);
-	zone->initialized = 1;
 
 	return 0;
 }
-- 
2.11.0

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1616682 — Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized

From	Michal Hocko <mhocko@kernel.org>
Date	2017-04-05 10:20 +0200
Subject	Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized
Message-ID	<tsOky-8y-29@gated-at.bofh.it>
In reply to	#1613708

On Fri 31-03-17 09:39:54, Michal Hocko wrote:
> Fixed screw ups during the initial patch split up as per Hillf
> ---
> From 8be6c5e47de66210e47710c80e72e8abd899017b Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Wed, 29 Mar 2017 15:11:30 +0200
> Subject: [PATCH] mm: get rid of zone_is_initialized
> 
> There shouldn't be any reason to add initialized when we can tell the
> same thing from checking whether there are any pages spanned to the
> zone. Remove zone_is_initialized() and replace it by zone_is_empty
> which can be used for the same set of tests.
> 
> This shouldn't have any visible effect

I've decided to drop this patch. My main motivation was to simplify
the hotplug workflow/ The situation is more hairy than I expected,
though. On one hand all zones should be initialized early during the
hotplug in add_memory_resource but direct users of arch_add_memory will
need this to be called I suspect. Let's just keep the current status quo
and clean up it later. It is not really needed for this series.

> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/linux/mmzone.h | 7 -------
>  mm/memory_hotplug.c    | 6 +++---
>  mm/page_alloc.c        | 3 +--
>  3 files changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 618499159a7c..3bac3ed71c7a 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -442,8 +442,6 @@ struct zone {
>  	seqlock_t		span_seqlock;
>  #endif
>  
> -	int initialized;
> -
>  	/* Write-intensive fields used from the page allocator */
>  	ZONE_PADDING(_pad1_)
>  
> @@ -520,11 +518,6 @@ static inline bool zone_spans_pfn(const struct zone *zone, unsigned long pfn)
>  	return zone->zone_start_pfn <= pfn && pfn < zone_end_pfn(zone);
>  }
>  
> -static inline bool zone_is_initialized(struct zone *zone)
> -{
> -	return zone->initialized;
> -}
> -
>  static inline bool zone_is_empty(struct zone *zone)
>  {
>  	return zone->spanned_pages == 0;
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 6fb6bd2df787..699f5a2a8efd 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -348,7 +348,7 @@ static void fix_zone_id(struct zone *zone, unsigned long start_pfn,
>  static int __ref ensure_zone_is_initialized(struct zone *zone,
>  			unsigned long start_pfn, unsigned long num_pages)
>  {
> -	if (!zone_is_initialized(zone))
> +	if (zone_is_empty(zone))
>  		return init_currently_empty_zone(zone, start_pfn, num_pages);
>  
>  	return 0;
> @@ -1051,7 +1051,7 @@ bool zone_can_shift(unsigned long pfn, unsigned long nr_pages,
>  
>  		/* no zones in use between current zone and target */
>  		for (i = idx + 1; i < target; i++)
> -			if (zone_is_initialized(zone - idx + i))
> +			if (!zone_is_empty(zone - idx + i))
>  				return false;
>  	}
>  
> @@ -1062,7 +1062,7 @@ bool zone_can_shift(unsigned long pfn, unsigned long nr_pages,
>  
>  		/* no zones in use between current zone and target */
>  		for (i = target + 1; i < idx; i++)
> -			if (zone_is_initialized(zone - idx + i))
> +			if (!zone_is_empty(zone - idx + i))
>  				return false;
>  	}
>  
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5ee8a26fa383..756353d1e293 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -795,7 +795,7 @@ static inline void __free_one_page(struct page *page,
>  
>  	max_order = min_t(unsigned int, MAX_ORDER, pageblock_order + 1);
>  
> -	VM_BUG_ON(!zone_is_initialized(zone));
> +	VM_BUG_ON(zone_is_empty(zone));
>  	VM_BUG_ON_PAGE(page->flags & PAGE_FLAGS_CHECK_AT_PREP, page);
>  
>  	VM_BUG_ON(migratetype == -1);
> @@ -5535,7 +5535,6 @@ int __meminit init_currently_empty_zone(struct zone *zone,
>  			zone_start_pfn, (zone_start_pfn + size));
>  
>  	zone_init_free_lists(zone);
> -	zone->initialized = 1;
>  
>  	return 0;
>  }
> -- 
> 2.11.0
> 
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1616755 — Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized

From	Michal Hocko <mhocko@kernel.org>
Date	2017-04-05 11:30 +0200
Subject	Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized
Message-ID	<tsPqh-Mk-9@gated-at.bofh.it>
In reply to	#1616682

On Wed 05-04-17 11:06:54, Igor Mammedov wrote:
> On Wed, 5 Apr 2017 10:14:00 +0200
> Michal Hocko <mhocko@kernel.org> wrote:
> 
> > On Fri 31-03-17 09:39:54, Michal Hocko wrote:
> > > Fixed screw ups during the initial patch split up as per Hillf
> > > ---
> > > From 8be6c5e47de66210e47710c80e72e8abd899017b Mon Sep 17 00:00:00 2001
> > > From: Michal Hocko <mhocko@suse.com>
> > > Date: Wed, 29 Mar 2017 15:11:30 +0200
> > > Subject: [PATCH] mm: get rid of zone_is_initialized
> > > 
> > > There shouldn't be any reason to add initialized when we can tell the
> > > same thing from checking whether there are any pages spanned to the
> > > zone. Remove zone_is_initialized() and replace it by zone_is_empty
> > > which can be used for the same set of tests.
> > > 
> > > This shouldn't have any visible effect  
> > 
> > I've decided to drop this patch. My main motivation was to simplify
> > the hotplug workflow/ The situation is more hairy than I expected,
> > though. On one hand all zones should be initialized early during the
> > hotplug in add_memory_resource but direct users of arch_add_memory will
> > need this to be called I suspect. Let's just keep the current status quo
> > and clean up it later. It is not really needed for this series.
> Would you post v2 with all fixups you've done so far?

yes, but I would like to sort what Reza Arbab is seeing first. I have
pushed my current changes to [1] for others to have something to play
with

[1] git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git attempts/rewrite-mem_hotplug-WIP
branch. Please note I might rebase it later

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1616769 — Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized

From	Igor Mammedov <imammedo@redhat.com>
Date	2017-04-05 11:50 +0200
Subject	Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized
Message-ID	<tsPqi-Mk-11@gated-at.bofh.it>
In reply to	#1616682

On Wed, 5 Apr 2017 10:14:00 +0200
Michal Hocko <mhocko@kernel.org> wrote:

> On Fri 31-03-17 09:39:54, Michal Hocko wrote:
> > Fixed screw ups during the initial patch split up as per Hillf
> > ---
> > From 8be6c5e47de66210e47710c80e72e8abd899017b Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.com>
> > Date: Wed, 29 Mar 2017 15:11:30 +0200
> > Subject: [PATCH] mm: get rid of zone_is_initialized
> > 
> > There shouldn't be any reason to add initialized when we can tell the
> > same thing from checking whether there are any pages spanned to the
> > zone. Remove zone_is_initialized() and replace it by zone_is_empty
> > which can be used for the same set of tests.
> > 
> > This shouldn't have any visible effect  
> 
> I've decided to drop this patch. My main motivation was to simplify
> the hotplug workflow/ The situation is more hairy than I expected,
> though. On one hand all zones should be initialized early during the
> hotplug in add_memory_resource but direct users of arch_add_memory will
> need this to be called I suspect. Let's just keep the current status quo
> and clean up it later. It is not really needed for this series.
Would you post v2 with all fixups you've done so far?

[toc] | [prev] | [next] | [standalone]

#1612987 — [PATCH 2/6] mm, tile: drop arch_{add,remove}_memory

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-30 14:00 +0200
Subject	[PATCH 2/6] mm, tile: drop arch_{add,remove}_memory
Message-ID	<tqGUa-5LE-17@gated-at.bofh.it>
In reply to	#1612985

From: Michal Hocko <mhocko@suse.com>

these functions are unreachable because tile doesn't support memory
hotplug becasuse it doesn't select ARCH_ENABLE_MEMORY_HOTPLUG nor
it supports SPARSEMEM.

This code hasn't been compiled for a while obviously because nobody has
noticed that __add_pages has a different signature since 2009.

Cc: Chris Metcalf <cmetcalf@mellanox.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 arch/tile/mm/init.c | 30 ------------------------------
 1 file changed, 30 deletions(-)

diff --git a/arch/tile/mm/init.c b/arch/tile/mm/init.c
index 3a97e4d7205c..5f757e04bcd2 100644
--- a/arch/tile/mm/init.c
+++ b/arch/tile/mm/init.c
@@ -857,36 +857,6 @@ void __init mem_init(void)
 #endif
 }
 
-/*
- * this is for the non-NUMA, single node SMP system case.
- * Specifically, in the case of x86, we will always add
- * memory to the highmem for now.
- */
-#ifndef CONFIG_NEED_MULTIPLE_NODES
-int arch_add_memory(u64 start, u64 size, bool for_device)
-{
-	struct pglist_data *pgdata = &contig_page_data;
-	struct zone *zone = pgdata->node_zones + MAX_NR_ZONES-1;
-	unsigned long start_pfn = start >> PAGE_SHIFT;
-	unsigned long nr_pages = size >> PAGE_SHIFT;
-
-	return __add_pages(zone, start_pfn, nr_pages);
-}
-
-int remove_memory(u64 start, u64 size)
-{
-	return -EINVAL;
-}
-
-#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
-{
-	/* TODO */
-	return -EBUSY;
-}
-#endif
-#endif
-
 struct kmem_cache *pgd_cache;
 
 void __init pgtable_cache_init(void)
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1612993 — [PATCH 3/6] mm: remove return value from init_currently_empty_zone

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-30 14:00 +0200
Subject	[PATCH 3/6] mm: remove return value from init_currently_empty_zone
Message-ID	<tqGUa-5LE-25@gated-at.bofh.it>
In reply to	#1612985

From: Michal Hocko <mhocko@suse.com>

init_currently_empty_zone doesn't have any error to return yet it is
still an int and callers try to be defensive and try to handle potential
error. Remove this nonsense and simplify all callers.

This patch shouldn't have any visible effect

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/mmzone.h |  2 +-
 mm/memory_hotplug.c    | 25 ++++++-------------------
 mm/page_alloc.c        |  6 ++----
 3 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index dbe3b32fe85d..c86c78617d17 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -768,7 +768,7 @@ enum memmap_context {
 	MEMMAP_EARLY,
 	MEMMAP_HOTPLUG,
 };
-extern int init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
+extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
 				     unsigned long size);
 
 extern void lruvec_init(struct lruvec *lruvec);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 699f5a2a8efd..056dbbe6d20e 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -343,27 +343,20 @@ static void fix_zone_id(struct zone *zone, unsigned long start_pfn,
 		set_page_links(pfn_to_page(pfn), zid, nid, pfn);
 }
 
-/* Can fail with -ENOMEM from allocating a wait table with vmalloc() or
- * alloc_bootmem_node_nopanic()/memblock_virt_alloc_node_nopanic() */
-static int __ref ensure_zone_is_initialized(struct zone *zone,
+static void __ref ensure_zone_is_initialized(struct zone *zone,
 			unsigned long start_pfn, unsigned long num_pages)
 {
-	if (zone_is_empty(zone))
-		return init_currently_empty_zone(zone, start_pfn, num_pages);
-
-	return 0;
+	if (!zone_is_empty(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)
 {
-	int ret;
 	unsigned long flags;
 	unsigned long z1_start_pfn;
 
-	ret = ensure_zone_is_initialized(z1, start_pfn, end_pfn - start_pfn);
-	if (ret)
-		return ret;
+	ensure_zone_is_initialized(z1, start_pfn, end_pfn - start_pfn);
 
 	pgdat_resize_lock(z1->zone_pgdat, &flags);
 
@@ -399,13 +392,10 @@ static int __meminit move_pfn_range_left(struct zone *z1, struct zone *z2,
 static int __meminit move_pfn_range_right(struct zone *z1, struct zone *z2,
 		unsigned long start_pfn, unsigned long end_pfn)
 {
-	int ret;
 	unsigned long flags;
 	unsigned long z2_end_pfn;
 
-	ret = ensure_zone_is_initialized(z2, start_pfn, end_pfn - start_pfn);
-	if (ret)
-		return ret;
+	ensure_zone_is_initialized(z2, start_pfn, end_pfn - start_pfn);
 
 	pgdat_resize_lock(z1->zone_pgdat, &flags);
 
@@ -476,12 +466,9 @@ static int __meminit __add_zone(struct zone *zone, unsigned long phys_start_pfn)
 	int nid = pgdat->node_id;
 	int zone_type;
 	unsigned long flags, pfn;
-	int ret;
 
 	zone_type = zone - pgdat->node_zones;
-	ret = ensure_zone_is_initialized(zone, phys_start_pfn, nr_pages);
-	if (ret)
-		return ret;
+	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);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index af58b51c5897..c6127f1a62e9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5518,7 +5518,7 @@ static __meminit void zone_pcp_init(struct zone *zone)
 					 zone_batchsize(zone));
 }
 
-int __meminit init_currently_empty_zone(struct zone *zone,
+void __meminit init_currently_empty_zone(struct zone *zone,
 					unsigned long zone_start_pfn,
 					unsigned long size)
 {
@@ -5997,7 +5997,6 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
 {
 	enum zone_type j;
 	int nid = pgdat->node_id;
-	int ret;
 
 	pgdat_resize_init(pgdat);
 #ifdef CONFIG_NUMA_BALANCING
@@ -6079,8 +6078,7 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
 
 		set_pageblock_order();
 		setup_usemap(pgdat, zone, zone_start_pfn, size);
-		ret = init_currently_empty_zone(zone, zone_start_pfn, size);
-		BUG_ON(ret);
+		init_currently_empty_zone(zone, zone_start_pfn, size);
 		memmap_init(size, nid, j, zone_start_pfn);
 	}
 }
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1613612 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

From	"Hillf Danton" <hillf.zj@alibaba-inc.com>
Date	2017-03-31 06:00 +0200
Subject	Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone
Message-ID	<tqVTb-7Dg-1@gated-at.bofh.it>
In reply to	#1612993

On March 30, 2017 7:55 PM Michal Hocko wrote: 
> 
> From: Michal Hocko <mhocko@suse.com>
> 
> init_currently_empty_zone doesn't have any error to return yet it is
> still an int and callers try to be defensive and try to handle potential
> error. Remove this nonsense and simplify all callers.
> 
It is already cut off in 1/6 in this series?

<snip>

> -/* Can fail with -ENOMEM from allocating a wait table with vmalloc() or
> - * alloc_bootmem_node_nopanic()/memblock_virt_alloc_node_nopanic() */
> -static int __ref ensure_zone_is_initialized(struct zone *zone,
> +static void __ref ensure_zone_is_initialized(struct zone *zone,
>  			unsigned long start_pfn, unsigned long num_pages)
>  {
> -	if (zone_is_empty(zone))
> -		return init_currently_empty_zone(zone, start_pfn, num_pages);
> -
> -	return 0;
> +	if (!zone_is_empty(zone))
> +		init_currently_empty_zone(zone, start_pfn, num_pages);
>  }
Semantic change added?

Hillf

[toc] | [prev] | [next] | [standalone]

#1613667 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-31 08:50 +0200
Subject	Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone
Message-ID	<tqYxI-10x-11@gated-at.bofh.it>
In reply to	#1613612

On Fri 31-03-17 11:49:49, Hillf Danton wrote:
[...]
> > -/* Can fail with -ENOMEM from allocating a wait table with vmalloc() or
> > - * alloc_bootmem_node_nopanic()/memblock_virt_alloc_node_nopanic() */
> > -static int __ref ensure_zone_is_initialized(struct zone *zone,
> > +static void __ref ensure_zone_is_initialized(struct zone *zone,
> >  			unsigned long start_pfn, unsigned long num_pages)
> >  {
> > -	if (zone_is_empty(zone))
> > -		return init_currently_empty_zone(zone, start_pfn, num_pages);
> > -
> > -	return 0;
> > +	if (!zone_is_empty(zone))
> > +		init_currently_empty_zone(zone, start_pfn, num_pages);
> >  }
> Semantic change added?

could you be more specific?
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1613676 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

From	"Hillf Danton" <hillf.zj@alibaba-inc.com>
Date	2017-03-31 09:10 +0200
Subject	Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone
Message-ID	<tqYR4-1na-5@gated-at.bofh.it>
In reply to	#1613667

On March 31, 2017 2:49 PM Michal Hocko wrote: 
> On Fri 31-03-17 11:49:49, Hillf Danton wrote:
> [...]
> > > -/* Can fail with -ENOMEM from allocating a wait table with vmalloc() or
> > > - * alloc_bootmem_node_nopanic()/memblock_virt_alloc_node_nopanic() */
> > > -static int __ref ensure_zone_is_initialized(struct zone *zone,
> > > +static void __ref ensure_zone_is_initialized(struct zone *zone,
> > >  			unsigned long start_pfn, unsigned long num_pages)
> > >  {
> > > -	if (zone_is_empty(zone))
> > > -		return init_currently_empty_zone(zone, start_pfn, num_pages);
> > > -
> > > -	return 0;
> > > +	if (!zone_is_empty(zone))
> > > +		init_currently_empty_zone(zone, start_pfn, num_pages);
> > >  }
> > Semantic change added?
> 
> could you be more specific?

Well, I'm wondering why you are trying to initiate a nonempty zone.

Hillf

[toc] | [prev] | [next] | [standalone]

#1613678 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-31 09:20 +0200
Subject	Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone
Message-ID	<tqZ0J-1qH-3@gated-at.bofh.it>
In reply to	#1613676

On Fri 31-03-17 15:06:41, Hillf Danton wrote:
> On March 31, 2017 2:49 PM Michal Hocko wrote: 
> > On Fri 31-03-17 11:49:49, Hillf Danton wrote:
> > [...]
> > > > -/* Can fail with -ENOMEM from allocating a wait table with vmalloc() or
> > > > - * alloc_bootmem_node_nopanic()/memblock_virt_alloc_node_nopanic() */
> > > > -static int __ref ensure_zone_is_initialized(struct zone *zone,
> > > > +static void __ref ensure_zone_is_initialized(struct zone *zone,
> > > >  			unsigned long start_pfn, unsigned long num_pages)
> > > >  {
> > > > -	if (zone_is_empty(zone))
> > > > -		return init_currently_empty_zone(zone, start_pfn, num_pages);
> > > > -
> > > > -	return 0;
> > > > +	if (!zone_is_empty(zone))
> > > > +		init_currently_empty_zone(zone, start_pfn, num_pages);
> > > >  }
> > > Semantic change added?
> > 
> > could you be more specific?
> 
> Well, I'm wondering why you are trying to initiate a nonempty zone.

Ups, another fuck up during the initial patch split up. Thanks for
catching that but it would be more helpful to be more specific during
the feedback. I am getting blind to the code I am staring for quite some
time so it was not obvious what you mean here.

Thanks
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1613707 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-31 09:50 +0200
Subject	Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone
Message-ID	<tqZtM-1Du-23@gated-at.bofh.it>
In reply to	#1612993

Fixed screw ups during the initial patch split up as per Hillf
---
From a53c465b8f8046c6e6886dc3c17945238e63ee7c Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Wed, 29 Mar 2017 15:17:48 +0200
Subject: [PATCH] mm: remove return value from init_currently_empty_zone

init_currently_empty_zone doesn't have any error to return yet it is
still an int and callers try to be defensive and try to handle potential
error. Remove this nonsense and simplify all callers.

This patch shouldn't have any visible effect

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/mmzone.h |  2 +-
 mm/memory_hotplug.c    | 23 +++++------------------
 mm/page_alloc.c        |  8 ++------
 3 files changed, 8 insertions(+), 25 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 3bac3ed71c7a..6973a3e6ad73 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -762,7 +762,7 @@ enum memmap_context {
 	MEMMAP_EARLY,
 	MEMMAP_HOTPLUG,
 };
-extern int init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
+extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
 				     unsigned long size);
 
 extern void lruvec_init(struct lruvec *lruvec);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 699f5a2a8efd..1d55e5ec943f 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -343,27 +343,20 @@ static void fix_zone_id(struct zone *zone, unsigned long start_pfn,
 		set_page_links(pfn_to_page(pfn), zid, nid, pfn);
 }
 
-/* Can fail with -ENOMEM from allocating a wait table with vmalloc() or
- * alloc_bootmem_node_nopanic()/memblock_virt_alloc_node_nopanic() */
-static int __ref ensure_zone_is_initialized(struct zone *zone,
+static void __ref ensure_zone_is_initialized(struct zone *zone,
 			unsigned long start_pfn, unsigned long num_pages)
 {
 	if (zone_is_empty(zone))
-		return init_currently_empty_zone(zone, start_pfn, num_pages);
-
-	return 0;
+		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)
 {
-	int ret;
 	unsigned long flags;
 	unsigned long z1_start_pfn;
 
-	ret = ensure_zone_is_initialized(z1, start_pfn, end_pfn - start_pfn);
-	if (ret)
-		return ret;
+	ensure_zone_is_initialized(z1, start_pfn, end_pfn - start_pfn);
 
 	pgdat_resize_lock(z1->zone_pgdat, &flags);
 
@@ -399,13 +392,10 @@ static int __meminit move_pfn_range_left(struct zone *z1, struct zone *z2,
 static int __meminit move_pfn_range_right(struct zone *z1, struct zone *z2,
 		unsigned long start_pfn, unsigned long end_pfn)
 {
-	int ret;
 	unsigned long flags;
 	unsigned long z2_end_pfn;
 
-	ret = ensure_zone_is_initialized(z2, start_pfn, end_pfn - start_pfn);
-	if (ret)
-		return ret;
+	ensure_zone_is_initialized(z2, start_pfn, end_pfn - start_pfn);
 
 	pgdat_resize_lock(z1->zone_pgdat, &flags);
 
@@ -476,12 +466,9 @@ static int __meminit __add_zone(struct zone *zone, unsigned long phys_start_pfn)
 	int nid = pgdat->node_id;
 	int zone_type;
 	unsigned long flags, pfn;
-	int ret;
 
 	zone_type = zone - pgdat->node_zones;
-	ret = ensure_zone_is_initialized(zone, phys_start_pfn, nr_pages);
-	if (ret)
-		return ret;
+	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);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 756353d1e293..c6127f1a62e9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5518,7 +5518,7 @@ static __meminit void zone_pcp_init(struct zone *zone)
 					 zone_batchsize(zone));
 }
 
-int __meminit init_currently_empty_zone(struct zone *zone,
+void __meminit init_currently_empty_zone(struct zone *zone,
 					unsigned long zone_start_pfn,
 					unsigned long size)
 {
@@ -5535,8 +5535,6 @@ int __meminit init_currently_empty_zone(struct zone *zone,
 			zone_start_pfn, (zone_start_pfn + size));
 
 	zone_init_free_lists(zone);
-
-	return 0;
 }
 
 #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
@@ -5999,7 +5997,6 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
 {
 	enum zone_type j;
 	int nid = pgdat->node_id;
-	int ret;
 
 	pgdat_resize_init(pgdat);
 #ifdef CONFIG_NUMA_BALANCING
@@ -6081,8 +6078,7 @@ static void __paginginit free_area_init_core(struct pglist_data *pgdat)
 
 		set_pageblock_order();
 		setup_usemap(pgdat, zone, zone_start_pfn, size);
-		ret = init_currently_empty_zone(zone, zone_start_pfn, size);
-		BUG_ON(ret);
+		init_currently_empty_zone(zone, zone_start_pfn, size);
 		memmap_init(size, nid, j, zone_start_pfn);
 	}
 }
-- 
2.11.0

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1615564 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

From	Reza Arbab <arbab@linux.vnet.ibm.com>
Date	2017-04-03 23:30 +0200
Subject	Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone
Message-ID	<tshHY-3Tw-3@gated-at.bofh.it>
In reply to	#1612993

On Thu, Mar 30, 2017 at 01:54:51PM +0200, Michal Hocko wrote:
>init_currently_empty_zone doesn't have any error to return yet it is
>still an int and callers try to be defensive and try to handle potential
>error. Remove this nonsense and simplify all callers.

Semi-related; arch_remove_memory() returns int, but callers ignore it.

Is that worth cleaning up? If so, should the implementations be 
simplified, or should we maybe do a pr_error() or something with it?

-- 
Reza Arbab

[toc] | [prev] | [next] | [standalone]

#1615753 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

From	Michal Hocko <mhocko@kernel.org>
Date	2017-04-04 09:40 +0200
Subject	Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone
Message-ID	<tsrei-1N1-15@gated-at.bofh.it>
In reply to	#1615564

On Mon 03-04-17 16:22:32, Reza Arbab wrote:
> On Thu, Mar 30, 2017 at 01:54:51PM +0200, Michal Hocko wrote:
> >init_currently_empty_zone doesn't have any error to return yet it is
> >still an int and callers try to be defensive and try to handle potential
> >error. Remove this nonsense and simplify all callers.
> 
> Semi-related; arch_remove_memory() returns int, but callers ignore it.
> 
> Is that worth cleaning up? If so, should the implementations be simplified,
> or should we maybe do a pr_error() or something with it?

No, pr_error is not really helpful. Either that path can fail and we
should handle it properly - which will be hard because remove_memory
cannot handle that or we should just make arch_remove_memory
non-failing. I have a suspicion that this path doesn't really fail
in fact. This requires a deeper inspection though. I've put that on my
todo list.

Thanks!
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1612996 — [PATCH 6/6] mm, memory_hotplug: remove unused cruft after memory hotplug rework

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-30 14:00 +0200
Subject	[PATCH 6/6] mm, memory_hotplug: remove unused cruft after memory hotplug rework
Message-ID	<tqGUb-5LE-41@gated-at.bofh.it>
In reply to	#1612985

From: Michal Hocko <mhocko@suse.com>

arch_add_memory doesn't need for_device parameter anymore because
devm_memremap_pages already does all what it needs to.

zone_for_memory doesn't have any user anymore as well as the whole zone
shifting infrastructure so drop them as well.

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              |   3 +-
 arch/x86/mm/init_32.c          |   2 +-
 arch/x86/mm/init_64.c          |   2 +-
 include/linux/memory_hotplug.h |   4 +-
 kernel/memremap.c              |   2 +-
 mm/memory_hotplug.c            | 209 +----------------------------------------
 9 files changed, 9 insertions(+), 219 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 0fb7f3946785..6ebb570f1eae 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -645,7 +645,7 @@ mem_init (void)
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index db1369a7f69f..707a4146938b 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -126,7 +126,7 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
 	return -ENODEV;
 }
 
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 9e8c515ee29f..7d7591b63b57 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -153,7 +153,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
 #endif
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = PFN_DOWN(start);
 	unsigned long size_pages = PFN_DOWN(size);
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index 95261b66bcf3..4e4afa0ab3c3 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -485,13 +485,12 @@ void free_initrd_mem(unsigned long start, unsigned long end)
 #endif
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = PFN_DOWN(start);
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	int ret;
 
-
 	/* We only have ZONE_NORMAL, so this is easy.. */
 	ret = __add_pages(nid, start_pfn, nr_pages);
 	if (unlikely(ret))
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 5de79aa7d6ce..b389f2ec75f9 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -816,7 +816,7 @@ void __init mem_init(void)
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index bc53f24e6703..20f575333b08 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -637,7 +637,7 @@ static void  update_end_of_memory_vars(u64 start, u64 size)
 	}
 }
 
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 63577ce57028..d5adb7c468d1 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -274,9 +274,7 @@ 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 for_device);
+extern int arch_add_memory(int nid, u64 start, u64 size);
 extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 		unsigned long nr_pages);
 extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 1b137649cb82..808f7974918a 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -366,7 +366,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
 
 	lock_device_hotplug();
 	mem_hotplug_begin();
-	error = arch_add_memory(nid, align_start, align_size, true);
+	error = arch_add_memory(nid, align_start, align_size);
 	if (!error)
 		move_pfn_range_to_zone(&NODE_DATA(nid)->node_zones[ZONE_DEVICE],
 				align_start, align_size);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index d7efa0191bb7..b9dc1c4e26c3 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -295,180 +295,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_empty(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)
 {
 	int ret;
@@ -1332,39 +1158,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);
@@ -1410,7 +1203,7 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
 	}
 
 	/* call arch's memory hotadd */
-	ret = arch_add_memory(nid, start, size, false);
+	ret = arch_add_memory(nid, start, size);
 
 	if (ret < 0)
 		goto error;
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1613706 — Re: [PATCH 6/6] mm, memory_hotplug: remove unused cruft after memory hotplug rework

From	Michal Hocko <mhocko@kernel.org>
Date	2017-03-31 09:50 +0200
Subject	Re: [PATCH 6/6] mm, memory_hotplug: remove unused cruft after memory hotplug rework
Message-ID	<tqZtL-1Du-1@gated-at.bofh.it>
In reply to	#1612996

rebase on top of previous changes
---
From dfb7cab2783d2175ad8355ce65943a28e43d8700 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Wed, 29 Mar 2017 18:08:51 +0200
Subject: [PATCH] mm, memory_hotplug: remove unused cruft after memory hotplug
 rework

arch_add_memory doesn't need for_device parameter anymore because
devm_memremap_pages already does all what it needs to.

zone_for_memory doesn't have any user anymore as well as the whole zone
shifting infrastructure so drop them as well.

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              |   3 +-
 arch/x86/mm/init_32.c          |   2 +-
 arch/x86/mm/init_64.c          |   2 +-
 include/linux/memory_hotplug.h |   4 +-
 kernel/memremap.c              |   2 +-
 mm/memory_hotplug.c            | 209 +----------------------------------------
 9 files changed, 9 insertions(+), 219 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 0fb7f3946785..6ebb570f1eae 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -645,7 +645,7 @@ mem_init (void)
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index db1369a7f69f..707a4146938b 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -126,7 +126,7 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
 	return -ENODEV;
 }
 
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 9e8c515ee29f..7d7591b63b57 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -153,7 +153,7 @@ void __init free_initrd_mem(unsigned long start, unsigned long end)
 #endif
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = PFN_DOWN(start);
 	unsigned long size_pages = PFN_DOWN(size);
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index 95261b66bcf3..4e4afa0ab3c3 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -485,13 +485,12 @@ void free_initrd_mem(unsigned long start, unsigned long end)
 #endif
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = PFN_DOWN(start);
 	unsigned long nr_pages = size >> PAGE_SHIFT;
 	int ret;
 
-
 	/* We only have ZONE_NORMAL, so this is easy.. */
 	ret = __add_pages(nid, start_pfn, nr_pages);
 	if (unlikely(ret))
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 5de79aa7d6ce..b389f2ec75f9 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -816,7 +816,7 @@ void __init mem_init(void)
 }
 
 #ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index bc53f24e6703..20f575333b08 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -637,7 +637,7 @@ static void  update_end_of_memory_vars(u64 start, u64 size)
 	}
 }
 
-int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
+int arch_add_memory(int nid, u64 start, u64 size)
 {
 	unsigned long start_pfn = start >> PAGE_SHIFT;
 	unsigned long nr_pages = size >> PAGE_SHIFT;
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 63577ce57028..d5adb7c468d1 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -274,9 +274,7 @@ 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 for_device);
+extern int arch_add_memory(int nid, u64 start, u64 size);
 extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 		unsigned long nr_pages);
 extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 1b137649cb82..808f7974918a 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -366,7 +366,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
 
 	lock_device_hotplug();
 	mem_hotplug_begin();
-	error = arch_add_memory(nid, align_start, align_size, true);
+	error = arch_add_memory(nid, align_start, align_size);
 	if (!error)
 		move_pfn_range_to_zone(&NODE_DATA(nid)->node_zones[ZONE_DEVICE],
 				align_start, align_size);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 3a8b24e47a2b..b9dc1c4e26c3 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -295,180 +295,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_empty(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)
 {
 	int ret;
@@ -1332,39 +1158,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);
@@ -1410,7 +1203,7 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
 	}
 
 	/* call arch's memory hotadd */
-	ret = arch_add_memory(nid, start, size, false);
+	ret = arch_add_memory(nid, start, size);
 
 	if (ret < 0)
 		goto error;
-- 
2.11.0

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

Page 1 of 4 [1] 2 3 4 Next page →

csiph-web

[PATCH 0/6] mm: make movable onlining suck less

Contents

#1612985 — [PATCH 0/6] mm: make movable onlining suck less

#1612986 — [PATCH 1/6] mm: get rid of zone_is_initialized

#1613611 — Re: [PATCH 1/6] mm: get rid of zone_is_initialized

#1613664 — Re: [PATCH 1/6] mm: get rid of zone_is_initialized

#1613668 — Re: [PATCH 1/6] mm: get rid of zone_is_initialized

#1613708 — [PATCH v1 1/6] mm: get rid of zone_is_initialized

#1616682 — Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized

#1616755 — Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized

#1616769 — Re: [PATCH v1 1/6] mm: get rid of zone_is_initialized

#1612987 — [PATCH 2/6] mm, tile: drop arch_{add,remove}_memory

#1612993 — [PATCH 3/6] mm: remove return value from init_currently_empty_zone

#1613612 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

#1613667 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

#1613676 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

#1613678 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

#1613707 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

#1615564 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

#1615753 — Re: [PATCH 3/6] mm: remove return value from init_currently_empty_zone

#1612996 — [PATCH 6/6] mm, memory_hotplug: remove unused cruft after memory hotplug rework

#1613706 — Re: [PATCH 6/6] mm, memory_hotplug: remove unused cruft after memory hotplug rework