linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Wei Yang <richard.weiyang@gmail.com>
To: linux-mm@kvack.org
Cc: akpm@linux-foundation.org, mhocko@suse.com, osalvador@suse.de,
	david@redhat.com, Wei Yang <richard.weiyang@gmail.com>
Subject: [PATCH v4] mm: remove extra drain pages on pcp list
Date: Sun,  6 Jan 2019 07:31:41 +0800	[thread overview]
Message-ID: <20190105233141.2329-1-richard.weiyang@gmail.com> (raw)
In-Reply-To: <20181221170228.10686-1-richard.weiyang@gmail.com>

In current implementation, there are two places to isolate a range of
page: __offline_pages() and alloc_contig_range(). During this procedure,
it will drain pages on pcp list.

Below is a brief call flow:

  __offline_pages()/alloc_contig_range()
      start_isolate_page_range()
          set_migratetype_isolate()
              drain_all_pages()
      drain_all_pages()                 <--- A

>From this snippet we can see current logic is isolate and drain pcp list
for each pageblock and drain pcp list again for the whole range.

start_isolate_page_range is responsible for isolating the given pfn
range. One part of that job is to make sure that also pages that are on
the allocator pcp lists are properly isolated. Otherwise they could be
reused and the range wouldn't be completely isolated until the memory is
freed back.  While there is no strict guarantee here because pages might
get allocated at any time before drain_all_pages is called there doesn't
seem to be any strong demand for such a guarantee.

In any case, draining is already done at the isolation level and there
is no need to do it again later by start_isolate_page_range callers
(memory hotplug and CMA allocator currently). Therefore remove pointless
draining in existing callers to make the code more clear and
functionally correct.

[mhocko@suse.com: provide a clearer changelog for the last two paragraph]

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>

---
v4:
  * adjust last two paragraph changelog from Michal's comment
v3:
  * it is not proper to rely on caller to drain pages, so keep to drain
    pages during iteration and remove the one in callers.
v2: adjust changelog with MIGRATE_ISOLATE effects for the isolated range
---
 mm/memory_hotplug.c | 1 -
 mm/page_alloc.c     | 1 -
 2 files changed, 2 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 6910e0eea074..d2fa6cbbb2db 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1599,7 +1599,6 @@ static int __ref __offline_pages(unsigned long start_pfn,
 
 	cond_resched();
 	lru_add_drain_all();
-	drain_all_pages(zone);
 
 	pfn = scan_movable_pages(start_pfn, end_pfn);
 	if (pfn) { /* We have movable pages */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f1edd36a1e2b..d9ee4bb3a1a7 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8041,7 +8041,6 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 */
 
 	lru_add_drain_all();
-	drain_all_pages(cc.zone);
 
 	order = 0;
 	outer_start = start;
-- 
2.15.1

WARNING: multiple messages have this Message-ID (diff)
From: Wei Yang <richard.weiyang@gmail.com>
To: linux-mm@kvack.org
Cc: akpm@linux-foundation.org, mhocko@suse.com, osalvador@suse.de,
	david@redhat.com, Wei Yang <richard.weiyang@gmail.com>
Subject: [PATCH v4] mm: remove extra drain pages on pcp list
Date: Sun,  6 Jan 2019 07:31:41 +0800	[thread overview]
Message-ID: <20190105233141.2329-1-richard.weiyang@gmail.com> (raw)
Message-ID: <20190105233141.7gim0kYV3mtPcpG0URNBUh_IHQ7XMaeVsgJ0eXtjJGk@z> (raw)
In-Reply-To: <20181221170228.10686-1-richard.weiyang@gmail.com>

In current implementation, there are two places to isolate a range of
page: __offline_pages() and alloc_contig_range(). During this procedure,
it will drain pages on pcp list.

Below is a brief call flow:

  __offline_pages()/alloc_contig_range()
      start_isolate_page_range()
          set_migratetype_isolate()
              drain_all_pages()
      drain_all_pages()                 <--- A

From this snippet we can see current logic is isolate and drain pcp list
for each pageblock and drain pcp list again for the whole range.

start_isolate_page_range is responsible for isolating the given pfn
range. One part of that job is to make sure that also pages that are on
the allocator pcp lists are properly isolated. Otherwise they could be
reused and the range wouldn't be completely isolated until the memory is
freed back.  While there is no strict guarantee here because pages might
get allocated at any time before drain_all_pages is called there doesn't
seem to be any strong demand for such a guarantee.

In any case, draining is already done at the isolation level and there
is no need to do it again later by start_isolate_page_range callers
(memory hotplug and CMA allocator currently). Therefore remove pointless
draining in existing callers to make the code more clear and
functionally correct.

[mhocko@suse.com: provide a clearer changelog for the last two paragraph]

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>

---
v4:
  * adjust last two paragraph changelog from Michal's comment
v3:
  * it is not proper to rely on caller to drain pages, so keep to drain
    pages during iteration and remove the one in callers.
v2: adjust changelog with MIGRATE_ISOLATE effects for the isolated range
---
 mm/memory_hotplug.c | 1 -
 mm/page_alloc.c     | 1 -
 2 files changed, 2 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 6910e0eea074..d2fa6cbbb2db 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1599,7 +1599,6 @@ static int __ref __offline_pages(unsigned long start_pfn,
 
 	cond_resched();
 	lru_add_drain_all();
-	drain_all_pages(zone);
 
 	pfn = scan_movable_pages(start_pfn, end_pfn);
 	if (pfn) { /* We have movable pages */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f1edd36a1e2b..d9ee4bb3a1a7 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8041,7 +8041,6 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	 */
 
 	lru_add_drain_all();
-	drain_all_pages(cc.zone);
 
 	order = 0;
 	outer_start = start;
-- 
2.15.1


  parent reply	other threads:[~2019-01-05 23:31 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-14  2:39 [PATCH] mm, page_isolation: remove drain_all_pages() in set_migratetype_isolate() Wei Yang
2018-12-14  3:57 ` Andrew Morton
2018-12-14  7:01   ` Wei Yang
2018-12-14 15:17   ` Wei Yang
2018-12-17 12:21     ` Michal Hocko
2018-12-18 20:48       ` Wei Yang
2018-12-17 12:25 ` Michal Hocko
2018-12-17 15:08   ` Wei Yang
2018-12-17 15:48     ` Michal Hocko
2018-12-18 14:44       ` Wei Yang
2018-12-18 20:46 ` [PATCH v2] " Wei Yang
2018-12-18 21:14   ` David Hildenbrand
2018-12-18 21:49     ` Wei Yang
2018-12-18 22:18       ` David Hildenbrand
2018-12-18 23:29   ` Oscar Salvador
2018-12-19  9:51   ` Michal Hocko
2018-12-19  9:57     ` Oscar Salvador
2018-12-19 13:53       ` Wei Yang
2018-12-19 14:13         ` Michal Hocko
2018-12-19 14:33           ` Wei Yang
2018-12-19 14:39             ` Michal Hocko
2018-12-20 15:58               ` Wei Yang
2018-12-20 16:23                 ` Michal Hocko
2018-12-21  3:37                   ` Wei Yang
2018-12-19 13:29     ` Wei Yang
2018-12-19 13:40       ` Michal Hocko
2018-12-19 13:56         ` Wei Yang
2018-12-19 14:12           ` Michal Hocko
2018-12-19 14:41             ` Wei Yang
2018-12-19 10:05   ` Michal Hocko
2018-12-21 17:02   ` [PATCH v3] mm: remove extra drain pages on pcp list Wei Yang
2018-12-21 17:02     ` Wei Yang
2019-01-03 13:56     ` Michal Hocko
2019-01-05 23:27       ` Wei Yang
2019-01-05 23:31     ` Wei Yang [this message]
2019-01-05 23:31       ` [PATCH v4] " Wei Yang
2019-01-07 11:34       ` David Hildenbrand
2019-01-08  9:10       ` Oscar Salvador

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190105233141.2329-1-richard.weiyang@gmail.com \
    --to=richard.weiyang@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=osalvador@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).