[PATCH v2 0/2] mm: remove the memory isolate notifier

[PATCH v2 0/2] mm: remove the memory isolate notifier

[David Hildenbrand]

This is the MM part of
	https://lkml.org/lkml/2019/10/31/487

"We can get rid of the memory isolate notifier by switching to balloon
compaction in powerpc's CMM (Collaborative Memory Management). The memory
isolate notifier was only necessary to allow to offline memory blocks that
contain inflated/"loaned" pages - which also possible when the inflated
pages are movable (via balloon compaction). [...]"

Michael queued the POWERPC bits that remove the single user, but I am
missing ACKs for the MM bits. I think it makes sense to let these two
patches also go via Michael's tree, to avoid collissions. Thoughts?

v1 -> v2: (MM bits)
- "mm: remove the memory isolate notifier"
-- Remove another stale comment
-- Minor code cleanup

David Hildenbrand (2):
  mm: remove the memory isolate notifier
  mm: remove "count" parameter from has_unmovable_pages()

 drivers/base/memory.c          | 19 -----------------
 include/linux/memory.h         | 27 ------------------------
 include/linux/page-isolation.h |  4 ++--
 mm/memory_hotplug.c            |  2 +-
 mm/page_alloc.c                | 21 +++++++------------
 mm/page_isolation.c            | 38 ++++------------------------------
 6 files changed, 14 insertions(+), 97 deletions(-)

-- 
2.21.0

[PATCH v2 1/2] mm: remove the memory isolate notifier

[David Hildenbrand]

Luckily, we have no users left, so we can get rid of it. Cleanup
set_migratetype_isolate() a little bit.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Qian Cai <cai@lca.pw>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Pingfan Liu <kernelfans@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/base/memory.c  | 19 -------------------
 include/linux/memory.h | 27 ---------------------------
 mm/page_isolation.c    | 38 ++++----------------------------------
 3 files changed, 4 insertions(+), 80 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index a757d9ed88a7..03c18c97c2bf 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -73,20 +73,6 @@ void unregister_memory_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL(unregister_memory_notifier);
 
-static ATOMIC_NOTIFIER_HEAD(memory_isolate_chain);
-
-int register_memory_isolate_notifier(struct notifier_block *nb)
-{
-	return atomic_notifier_chain_register(&memory_isolate_chain, nb);
-}
-EXPORT_SYMBOL(register_memory_isolate_notifier);
-
-void unregister_memory_isolate_notifier(struct notifier_block *nb)
-{
-	atomic_notifier_chain_unregister(&memory_isolate_chain, nb);
-}
-EXPORT_SYMBOL(unregister_memory_isolate_notifier);
-
 static void memory_block_release(struct device *dev)
 {
 	struct memory_block *mem = to_memory_block(dev);
@@ -178,11 +164,6 @@ int memory_notify(unsigned long val, void *v)
 	return blocking_notifier_call_chain(&memory_chain, val, v);
 }
 
-int memory_isolate_notify(unsigned long val, void *v)
-{
-	return atomic_notifier_call_chain(&memory_isolate_chain, val, v);
-}
-
 /*
  * The probe routines leave the pages uninitialized, just as the bootmem code
  * does. Make sure we do not access them, but instead use only information from
diff --git a/include/linux/memory.h b/include/linux/memory.h
index 0ebb105eb261..d3fde2d0d94b 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -55,19 +55,6 @@ struct memory_notify {
 	int status_change_nid;
 };
 
-/*
- * During pageblock isolation, count the number of pages within the
- * range [start_pfn, start_pfn + nr_pages) which are owned by code
- * in the notifier chain.
- */
-#define MEM_ISOLATE_COUNT	(1<<0)
-
-struct memory_isolate_notify {
-	unsigned long start_pfn;	/* Start of range to check */
-	unsigned int nr_pages;		/* # pages in range to check */
-	unsigned int pages_found;	/* # pages owned found by callbacks */
-};
-
 struct notifier_block;
 struct mem_section;
 
@@ -94,27 +81,13 @@ static inline int memory_notify(unsigned long val, void *v)
 {
 	return 0;
 }
-static inline int register_memory_isolate_notifier(struct notifier_block *nb)
-{
-	return 0;
-}
-static inline void unregister_memory_isolate_notifier(struct notifier_block *nb)
-{
-}
-static inline int memory_isolate_notify(unsigned long val, void *v)
-{
-	return 0;
-}
 #else
 extern int register_memory_notifier(struct notifier_block *nb);
 extern void unregister_memory_notifier(struct notifier_block *nb);
-extern int register_memory_isolate_notifier(struct notifier_block *nb);
-extern void unregister_memory_isolate_notifier(struct notifier_block *nb);
 int create_memory_block_devices(unsigned long start, unsigned long size);
 void remove_memory_block_devices(unsigned long start, unsigned long size);
 extern void memory_dev_init(void);
 extern int memory_notify(unsigned long val, void *v);
-extern int memory_isolate_notify(unsigned long val, void *v);
 extern struct memory_block *find_memory_block(struct mem_section *);
 typedef int (*walk_memory_blocks_func_t)(struct memory_block *, void *);
 extern int walk_memory_blocks(unsigned long start, unsigned long size,
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 04ee1663cdbe..21af88b718aa 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -18,9 +18,7 @@
 static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags)
 {
 	struct zone *zone;
-	unsigned long flags, pfn;
-	struct memory_isolate_notify arg;
-	int notifier_ret;
+	unsigned long flags;
 	int ret = -EBUSY;
 
 	zone = page_zone(page);
@@ -35,41 +33,11 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	if (is_migrate_isolate_page(page))
 		goto out;
 
-	pfn = page_to_pfn(page);
-	arg.start_pfn = pfn;
-	arg.nr_pages = pageblock_nr_pages;
-	arg.pages_found = 0;
-
-	/*
-	 * It may be possible to isolate a pageblock even if the
-	 * migratetype is not MIGRATE_MOVABLE. The memory isolation
-	 * notifier chain is used by balloon drivers to return the
-	 * number of pages in a range that are held by the balloon
-	 * driver to shrink memory. If all the pages are accounted for
-	 * by balloons, are free, or on the LRU, isolation can continue.
-	 * Later, for example, when memory hotplug notifier runs, these
-	 * pages reported as "can be isolated" should be isolated(freed)
-	 * by the balloon driver through the memory notifier chain.
-	 */
-	notifier_ret = memory_isolate_notify(MEM_ISOLATE_COUNT, &arg);
-	notifier_ret = notifier_to_errno(notifier_ret);
-	if (notifier_ret)
-		goto out;
 	/*
 	 * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself.
 	 * We just check MOVABLE pages.
 	 */
-	if (!has_unmovable_pages(zone, page, arg.pages_found, migratetype,
-				 isol_flags))
-		ret = 0;
-
-	/*
-	 * immobile means "not-on-lru" pages. If immobile is larger than
-	 * removable-by-driver pages reported by notifier, we'll fail.
-	 */
-
-out:
-	if (!ret) {
+	if (!has_unmovable_pages(zone, page, 0, migratetype, isol_flags)) {
 		unsigned long nr_pages;
 		int mt = get_pageblock_migratetype(page);
 
@@ -79,8 +47,10 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 									NULL);
 
 		__mod_zone_freepage_state(zone, -nr_pages, mt);
+		ret = 0;
 	}
 
+out:
 	spin_unlock_irqrestore(&zone->lock, flags);
 	if (!ret)
 		drain_all_pages(zone);
-- 
2.21.0

Re: [PATCH v2 1/2] mm: remove the memory isolate notifier

[Michal Hocko]

On Thu 14-11-19 14:19:10, David Hildenbrand wrote:
> Luckily, we have no users left, so we can get rid of it. Cleanup
> set_migratetype_isolate() a little bit.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Pingfan Liu <kernelfans@gmail.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: David Hildenbrand <david@redhat.com>

\o/
Acked-by: Michal Hocko <mhocko@suse.com>

There is some potential on a further cleanups but this is definitely a
great step

> ---
>  drivers/base/memory.c  | 19 -------------------
>  include/linux/memory.h | 27 ---------------------------
>  mm/page_isolation.c    | 38 ++++----------------------------------
>  3 files changed, 4 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index a757d9ed88a7..03c18c97c2bf 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -73,20 +73,6 @@ void unregister_memory_notifier(struct notifier_block *nb)
>  }
>  EXPORT_SYMBOL(unregister_memory_notifier);
>  
> -static ATOMIC_NOTIFIER_HEAD(memory_isolate_chain);
> -
> -int register_memory_isolate_notifier(struct notifier_block *nb)
> -{
> -	return atomic_notifier_chain_register(&memory_isolate_chain, nb);
> -}
> -EXPORT_SYMBOL(register_memory_isolate_notifier);
> -
> -void unregister_memory_isolate_notifier(struct notifier_block *nb)
> -{
> -	atomic_notifier_chain_unregister(&memory_isolate_chain, nb);
> -}
> -EXPORT_SYMBOL(unregister_memory_isolate_notifier);
> -
>  static void memory_block_release(struct device *dev)
>  {
>  	struct memory_block *mem = to_memory_block(dev);
> @@ -178,11 +164,6 @@ int memory_notify(unsigned long val, void *v)
>  	return blocking_notifier_call_chain(&memory_chain, val, v);
>  }
>  
> -int memory_isolate_notify(unsigned long val, void *v)
> -{
> -	return atomic_notifier_call_chain(&memory_isolate_chain, val, v);
> -}
> -
>  /*
>   * The probe routines leave the pages uninitialized, just as the bootmem code
>   * does. Make sure we do not access them, but instead use only information from
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index 0ebb105eb261..d3fde2d0d94b 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -55,19 +55,6 @@ struct memory_notify {
>  	int status_change_nid;
>  };
>  
> -/*
> - * During pageblock isolation, count the number of pages within the
> - * range [start_pfn, start_pfn + nr_pages) which are owned by code
> - * in the notifier chain.
> - */
> -#define MEM_ISOLATE_COUNT	(1<<0)
> -
> -struct memory_isolate_notify {
> -	unsigned long start_pfn;	/* Start of range to check */
> -	unsigned int nr_pages;		/* # pages in range to check */
> -	unsigned int pages_found;	/* # pages owned found by callbacks */
> -};
> -
>  struct notifier_block;
>  struct mem_section;
>  
> @@ -94,27 +81,13 @@ static inline int memory_notify(unsigned long val, void *v)
>  {
>  	return 0;
>  }
> -static inline int register_memory_isolate_notifier(struct notifier_block *nb)
> -{
> -	return 0;
> -}
> -static inline void unregister_memory_isolate_notifier(struct notifier_block *nb)
> -{
> -}
> -static inline int memory_isolate_notify(unsigned long val, void *v)
> -{
> -	return 0;
> -}
>  #else
>  extern int register_memory_notifier(struct notifier_block *nb);
>  extern void unregister_memory_notifier(struct notifier_block *nb);
> -extern int register_memory_isolate_notifier(struct notifier_block *nb);
> -extern void unregister_memory_isolate_notifier(struct notifier_block *nb);
>  int create_memory_block_devices(unsigned long start, unsigned long size);
>  void remove_memory_block_devices(unsigned long start, unsigned long size);
>  extern void memory_dev_init(void);
>  extern int memory_notify(unsigned long val, void *v);
> -extern int memory_isolate_notify(unsigned long val, void *v);
>  extern struct memory_block *find_memory_block(struct mem_section *);
>  typedef int (*walk_memory_blocks_func_t)(struct memory_block *, void *);
>  extern int walk_memory_blocks(unsigned long start, unsigned long size,
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 04ee1663cdbe..21af88b718aa 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -18,9 +18,7 @@
>  static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags)
>  {
>  	struct zone *zone;
> -	unsigned long flags, pfn;
> -	struct memory_isolate_notify arg;
> -	int notifier_ret;
> +	unsigned long flags;
>  	int ret = -EBUSY;
>  
>  	zone = page_zone(page);
> @@ -35,41 +33,11 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
>  	if (is_migrate_isolate_page(page))
>  		goto out;
>  
> -	pfn = page_to_pfn(page);
> -	arg.start_pfn = pfn;
> -	arg.nr_pages = pageblock_nr_pages;
> -	arg.pages_found = 0;
> -
> -	/*
> -	 * It may be possible to isolate a pageblock even if the
> -	 * migratetype is not MIGRATE_MOVABLE. The memory isolation
> -	 * notifier chain is used by balloon drivers to return the
> -	 * number of pages in a range that are held by the balloon
> -	 * driver to shrink memory. If all the pages are accounted for
> -	 * by balloons, are free, or on the LRU, isolation can continue.
> -	 * Later, for example, when memory hotplug notifier runs, these
> -	 * pages reported as "can be isolated" should be isolated(freed)
> -	 * by the balloon driver through the memory notifier chain.
> -	 */
> -	notifier_ret = memory_isolate_notify(MEM_ISOLATE_COUNT, &arg);
> -	notifier_ret = notifier_to_errno(notifier_ret);
> -	if (notifier_ret)
> -		goto out;
>  	/*
>  	 * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself.
>  	 * We just check MOVABLE pages.
>  	 */
> -	if (!has_unmovable_pages(zone, page, arg.pages_found, migratetype,
> -				 isol_flags))
> -		ret = 0;
> -
> -	/*
> -	 * immobile means "not-on-lru" pages. If immobile is larger than
> -	 * removable-by-driver pages reported by notifier, we'll fail.
> -	 */
> -
> -out:
> -	if (!ret) {
> +	if (!has_unmovable_pages(zone, page, 0, migratetype, isol_flags)) {
>  		unsigned long nr_pages;
>  		int mt = get_pageblock_migratetype(page);
>  
> @@ -79,8 +47,10 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
>  									NULL);
>  
>  		__mod_zone_freepage_state(zone, -nr_pages, mt);
> +		ret = 0;
>  	}
>  
> +out:
>  	spin_unlock_irqrestore(&zone->lock, flags);
>  	if (!ret)
>  		drain_all_pages(zone);
> -- 
> 2.21.0

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v2 1/2] mm: remove the memory isolate notifier

[Greg Kroah-Hartman]

On Thu, Nov 14, 2019 at 02:19:10PM +0100, David Hildenbrand wrote:
> Luckily, we have no users left, so we can get rid of it. Cleanup
> set_migratetype_isolate() a little bit.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Pingfan Liu <kernelfans@gmail.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  drivers/base/memory.c  | 19 -------------------
>  include/linux/memory.h | 27 ---------------------------
>  mm/page_isolation.c    | 38 ++++----------------------------------
>  3 files changed, 4 insertions(+), 80 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH v2 2/2] mm: remove "count" parameter from has_unmovable_pages()

[David Hildenbrand]

Now that the memory isolate notifier is gone, the parameter is always 0.
Drop it and cleanup has_unmovable_pages().

Cc: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Pingfan Liu <kernelfans@gmail.com>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
Cc: Wei Yang <richardw.yang@linux.intel.com>
Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Arun KS <arunks@codeaurora.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 include/linux/page-isolation.h |  4 ++--
 mm/memory_hotplug.c            |  2 +-
 mm/page_alloc.c                | 21 +++++++--------------
 mm/page_isolation.c            |  2 +-
 4 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 6861df759fad..148e65a9c606 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -33,8 +33,8 @@ static inline bool is_migrate_isolate(int migratetype)
 #define MEMORY_OFFLINE	0x1
 #define REPORT_FAILURE	0x2
 
-bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
-			 int migratetype, int flags);
+bool has_unmovable_pages(struct zone *zone, struct page *page, int migratetype,
+			 int flags);
 void set_pageblock_migratetype(struct page *page, int migratetype);
 int move_freepages_block(struct zone *zone, struct page *page,
 				int migratetype, int *num_movable);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 3b5cee4d3f4e..f06d33748607 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1158,7 +1158,7 @@ static bool is_pageblock_removable_nolock(unsigned long pfn)
 	if (!zone_spans_pfn(zone, pfn))
 		return false;
 
-	return !has_unmovable_pages(zone, page, 0, MIGRATE_MOVABLE,
+	return !has_unmovable_pages(zone, page, MIGRATE_MOVABLE,
 				    MEMORY_OFFLINE);
 }
 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f5ecbacb0e04..e4d8f3a1a6b6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8270,17 +8270,15 @@ void *__init alloc_large_system_hash(const char *tablename,
 
 /*
  * This function checks whether pageblock includes unmovable pages or not.
- * If @count is not zero, it is okay to include less @count unmovable pages
  *
  * PageLRU check without isolation or lru_lock could race so that
  * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
  * check without lock_page also may miss some movable non-lru pages at
  * race condition. So you can't expect this function should be exact.
  */
-bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
-			 int migratetype, int flags)
+bool has_unmovable_pages(struct zone *zone, struct page *page, int migratetype,
+			 int flags)
 {
-	unsigned long found;
 	unsigned long iter = 0;
 	unsigned long pfn = page_to_pfn(page);
 	const char *reason = "unmovable page";
@@ -8306,13 +8304,11 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
 		goto unmovable;
 	}
 
-	for (found = 0; iter < pageblock_nr_pages; iter++) {
-		unsigned long check = pfn + iter;
-
-		if (!pfn_valid_within(check))
+	for (; iter < pageblock_nr_pages; iter++) {
+		if (!pfn_valid_within(pfn + iter))
 			continue;
 
-		page = pfn_to_page(check);
+		page = pfn_to_page(pfn + iter);
 
 		if (PageReserved(page))
 			goto unmovable;
@@ -8361,11 +8357,9 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
 		if ((flags & MEMORY_OFFLINE) && PageHWPoison(page))
 			continue;
 
-		if (__PageMovable(page))
+		if (__PageMovable(page) || PageLRU(page))
 			continue;
 
-		if (!PageLRU(page))
-			found++;
 		/*
 		 * If there are RECLAIMABLE pages, we need to check
 		 * it.  But now, memory offline itself doesn't call
@@ -8379,8 +8373,7 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
 		 * is set to both of a memory hole page and a _used_ kernel
 		 * page at boot.
 		 */
-		if (found > count)
-			goto unmovable;
+		goto unmovable;
 	}
 	return false;
 unmovable:
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 21af88b718aa..1f8b9dfecbe8 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -37,7 +37,7 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	 * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself.
 	 * We just check MOVABLE pages.
 	 */
-	if (!has_unmovable_pages(zone, page, 0, migratetype, isol_flags)) {
+	if (!has_unmovable_pages(zone, page, migratetype, isol_flags)) {
 		unsigned long nr_pages;
 		int mt = get_pageblock_migratetype(page);
 
-- 
2.21.0

Re: [PATCH v2 2/2] mm: remove "count" parameter from has_unmovable_pages()

[Michal Hocko]

On Thu 14-11-19 14:19:11, David Hildenbrand wrote:
> Now that the memory isolate notifier is gone, the parameter is always 0.
> Drop it and cleanup has_unmovable_pages().
> 
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Pingfan Liu <kernelfans@gmail.com>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Cc: Wei Yang <richardw.yang@linux.intel.com>
> Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Arun KS <arunks@codeaurora.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: David Hildenbrand <david@redhat.com>

yay again! I have to say that this was a head scratcher when I've seen
that for the first time. It is really great to see it go
Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!

> ---
>  include/linux/page-isolation.h |  4 ++--
>  mm/memory_hotplug.c            |  2 +-
>  mm/page_alloc.c                | 21 +++++++--------------
>  mm/page_isolation.c            |  2 +-
>  4 files changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
> index 6861df759fad..148e65a9c606 100644
> --- a/include/linux/page-isolation.h
> +++ b/include/linux/page-isolation.h
> @@ -33,8 +33,8 @@ static inline bool is_migrate_isolate(int migratetype)
>  #define MEMORY_OFFLINE	0x1
>  #define REPORT_FAILURE	0x2
>  
> -bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
> -			 int migratetype, int flags);
> +bool has_unmovable_pages(struct zone *zone, struct page *page, int migratetype,
> +			 int flags);
>  void set_pageblock_migratetype(struct page *page, int migratetype);
>  int move_freepages_block(struct zone *zone, struct page *page,
>  				int migratetype, int *num_movable);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 3b5cee4d3f4e..f06d33748607 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1158,7 +1158,7 @@ static bool is_pageblock_removable_nolock(unsigned long pfn)
>  	if (!zone_spans_pfn(zone, pfn))
>  		return false;
>  
> -	return !has_unmovable_pages(zone, page, 0, MIGRATE_MOVABLE,
> +	return !has_unmovable_pages(zone, page, MIGRATE_MOVABLE,
>  				    MEMORY_OFFLINE);
>  }
>  
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f5ecbacb0e04..e4d8f3a1a6b6 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -8270,17 +8270,15 @@ void *__init alloc_large_system_hash(const char *tablename,
>  
>  /*
>   * This function checks whether pageblock includes unmovable pages or not.
> - * If @count is not zero, it is okay to include less @count unmovable pages
>   *
>   * PageLRU check without isolation or lru_lock could race so that
>   * MIGRATE_MOVABLE block might include unmovable pages. And __PageMovable
>   * check without lock_page also may miss some movable non-lru pages at
>   * race condition. So you can't expect this function should be exact.
>   */
> -bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
> -			 int migratetype, int flags)
> +bool has_unmovable_pages(struct zone *zone, struct page *page, int migratetype,
> +			 int flags)
>  {
> -	unsigned long found;
>  	unsigned long iter = 0;
>  	unsigned long pfn = page_to_pfn(page);
>  	const char *reason = "unmovable page";
> @@ -8306,13 +8304,11 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
>  		goto unmovable;
>  	}
>  
> -	for (found = 0; iter < pageblock_nr_pages; iter++) {
> -		unsigned long check = pfn + iter;
> -
> -		if (!pfn_valid_within(check))
> +	for (; iter < pageblock_nr_pages; iter++) {
> +		if (!pfn_valid_within(pfn + iter))
>  			continue;
>  
> -		page = pfn_to_page(check);
> +		page = pfn_to_page(pfn + iter);
>  
>  		if (PageReserved(page))
>  			goto unmovable;
> @@ -8361,11 +8357,9 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
>  		if ((flags & MEMORY_OFFLINE) && PageHWPoison(page))
>  			continue;
>  
> -		if (__PageMovable(page))
> +		if (__PageMovable(page) || PageLRU(page))
>  			continue;
>  
> -		if (!PageLRU(page))
> -			found++;
>  		/*
>  		 * If there are RECLAIMABLE pages, we need to check
>  		 * it.  But now, memory offline itself doesn't call
> @@ -8379,8 +8373,7 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
>  		 * is set to both of a memory hole page and a _used_ kernel
>  		 * page at boot.
>  		 */
> -		if (found > count)
> -			goto unmovable;
> +		goto unmovable;
>  	}
>  	return false;
>  unmovable:
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 21af88b718aa..1f8b9dfecbe8 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -37,7 +37,7 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
>  	 * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself.
>  	 * We just check MOVABLE pages.
>  	 */
> -	if (!has_unmovable_pages(zone, page, 0, migratetype, isol_flags)) {
> +	if (!has_unmovable_pages(zone, page, migratetype, isol_flags)) {
>  		unsigned long nr_pages;
>  		int mt = get_pageblock_migratetype(page);
>  
> -- 
> 2.21.0

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v2 0/2] mm: remove the memory isolate notifier

[David Hildenbrand]

On 14.11.19 14:19, David Hildenbrand wrote:
> This is the MM part of
> 	https://lkml.org/lkml/2019/10/31/487
> 
> "We can get rid of the memory isolate notifier by switching to balloon
> compaction in powerpc's CMM (Collaborative Memory Management). The memory
> isolate notifier was only necessary to allow to offline memory blocks that
> contain inflated/"loaned" pages - which also possible when the inflated
> pages are movable (via balloon compaction). [...]"
> 
> Michael queued the POWERPC bits that remove the single user, but I am
> missing ACKs for the MM bits. I think it makes sense to let these two
> patches also go via Michael's tree, to avoid collissions. Thoughts?

@Michael, the ACKs from Michal should be sufficient to take these two
patches via your tree. Fine with you? Thanks!


-- 

Thanks,

David / dhildenb

Re: [PATCH v2 0/2] mm: remove the memory isolate notifier

[David Hildenbrand]

On 14.11.19 14:19, David Hildenbrand wrote:
> This is the MM part of
> 	https://lkml.org/lkml/2019/10/31/487
> 
> "We can get rid of the memory isolate notifier by switching to balloon
> compaction in powerpc's CMM (Collaborative Memory Management). The memory
> isolate notifier was only necessary to allow to offline memory blocks that
> contain inflated/"loaned" pages - which also possible when the inflated
> pages are movable (via balloon compaction). [...]"
> 
> Michael queued the POWERPC bits that remove the single user, but I am
> missing ACKs for the MM bits. I think it makes sense to let these two
> patches also go via Michael's tree, to avoid collissions. Thoughts?

The prereqs (powerpc bits) are upstream - I assume Michael didn't want
to mess with MM patches. @Andrew, please pick these up once you feel
like time for them has come. :)


-- 
Thanks,

David / dhildenb

Re: [PATCH v2 0/2] mm: remove the memory isolate notifier

[Michael Ellerman]

David Hildenbrand <david@redhat.com> writes:
> On 14.11.19 14:19, David Hildenbrand wrote:
>> This is the MM part of
>> 	https://lkml.org/lkml/2019/10/31/487
>> 
>> "We can get rid of the memory isolate notifier by switching to balloon
>> compaction in powerpc's CMM (Collaborative Memory Management). The memory
>> isolate notifier was only necessary to allow to offline memory blocks that
>> contain inflated/"loaned" pages - which also possible when the inflated
>> pages are movable (via balloon compaction). [...]"
>> 
>> Michael queued the POWERPC bits that remove the single user, but I am
>> missing ACKs for the MM bits. I think it makes sense to let these two
>> patches also go via Michael's tree, to avoid collissions. Thoughts?
>
> The prereqs (powerpc bits) are upstream - I assume Michael didn't want
> to mess with MM patches.

Yes, sorry I meant to send you a mail saying so.

cheers