From: Pavel Tatashin <Pavel.Tatashin@microsoft.com>
To: "osalvador@techadventures.net" <osalvador@techadventures.net>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Cc: "mhocko@suse.com" <mhocko@suse.com>,
"dan.j.williams@intel.com" <dan.j.williams@intel.com>,
"pasha.tatashin@oracle.com" <pasha.tatashin@oracle.com>,
"jglisse@redhat.com" <jglisse@redhat.com>,
"david@redhat.com" <david@redhat.com>,
"yasu.isimatu@gmail.com" <yasu.isimatu@gmail.com>,
"logang@deltatee.com" <logang@deltatee.com>,
"dave.jiang@intel.com" <dave.jiang@intel.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Oscar Salvador <osalvador@suse.de>
Subject: RE: [RFC PATCH 0/3] Do not touch pages in remove_memory path
Date: Wed, 15 Aug 2018 14:05:35 +0000 [thread overview]
Message-ID: <DM5PR21MB0508CEC7F586EBC89D2CCFBB9D3F0@DM5PR21MB0508.namprd21.prod.outlook.com> (raw)
In-Reply-To: <20180807133757.18352-1-osalvador@techadventures.net>
> This tries to fix [1], which was reported by David Hildenbrand, and also
> does some cleanups/refactoring.
Hi Oscar,
I would like to review this work. Are you in process of sending a new version? If so, I will wait for it.
Thank you,
Pavel
>
> I am sending this as RFC to see if the direction I am going is right before
> spending more time into it.
> And also to gather feedback about hmm/zone_device stuff.
> The code compiles and I tested it successfully with normal memory-hotplug
> operations.
>
> Here we go:
>
> With the following scenario:
>
> 1) We add memory
> 2) We do not online it
> 3) We remove the memory
>
> an invalid access is being made to those pages.
>
> The reason is that the pages are initialized in online_pages() path:
>
> / online_pages
> | move_pfn_range
> ONLINE | move_pfn_range_to_zone
> PAGES | ...
> | memmap_init_zone
>
> But depending on our policy about onlining the pages by default, we might not
> online them right after having added the memory, and so, those pages might
> be
> left unitialized.
>
> This is a problem because we access those pages in arch_remove_memory:
>
> ...
> if (altmap)
> page += vmem_altmap_offset(altmap);
> zone = page_zone(page);
> ...
>
> So we are accessing unitialized data basically.
>
>
> Currently, we need to have the zone from arch_remove_memory to all the
> way down
> because
>
> 1) we call __remove_zone zo shrink spanned pages from pgdat/zone
> 2) we get the pgdat from the zone
>
> Number 1 can be fixed by moving __remove_zone back to offline_pages(),
> where it should be.
> This, besides fixing the bug, will make the code more consistent because all
> the reveserse
> operations from online_pages() will be made in offline_pages().
>
> Number 2 can be fixed by passing nid instead of zone.
>
> The tricky part of all this is the hmm code and the zone_device stuff.
>
> Fixing the calls to arch_remove_memory in the arch code is easy, but
> arch_remove_memory
> is being used in:
>
> kernel/memremap.c: devm_memremap_pages_release()
> mm/hmm.c: hmm_devmem_release()
>
> I did my best to get my head around this, but my knowledge in that area is 0,
> so I am pretty sure
> I did not get it right.
>
> The thing is:
>
> devm_memremap_pages(), which is the counterpart of
> devm_memremap_pages_release(),
> calls arch_add_memory(), and then calls move_pfn_range_to_zone() (to
> ZONE_DEVICE).
> So it does not go through online_pages().
> So there I call shrink_pages() (it does pretty much as __remove_zone) before
> calling
> to arch_remove_memory.
> But as I said, I do now if that is right.
>
> [1] https://patchwork.kernel.org/patch/10547445/
>
> Oscar Salvador (3):
> mm/memory_hotplug: Add nid parameter to arch_remove_memory
> mm/memory_hotplug: Create __shrink_pages and move it to offline_pages
> mm/memory_hotplug: Refactor shrink_zone/pgdat_span
>
> arch/ia64/mm/init.c | 6 +-
> arch/powerpc/mm/mem.c | 13 +--
> arch/s390/mm/init.c | 2 +-
> arch/sh/mm/init.c | 6 +-
> arch/x86/mm/init_32.c | 6 +-
> arch/x86/mm/init_64.c | 10 +--
> include/linux/memory_hotplug.h | 8 +-
> kernel/memremap.c | 9 +-
> mm/hmm.c | 6 +-
> mm/memory_hotplug.c | 190 +++++++++++++++++++++--------------------
> mm/sparse.c | 4 +-
> 11 files changed, 127 insertions(+), 133 deletions(-)
>
> --
> 2.13.6
next prev parent reply other threads:[~2018-08-15 14:05 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-07 13:37 [RFC PATCH 0/3] Do not touch pages in remove_memory path osalvador
2018-08-07 13:37 ` [RFC PATCH 1/3] mm/memory_hotplug: Add nid parameter to arch_remove_memory osalvador
2018-08-07 13:37 ` [RFC PATCH 2/3] mm/memory_hotplug: Create __shrink_pages and move it to offline_pages osalvador
2018-08-07 13:52 ` Jerome Glisse
2018-08-07 14:54 ` David Hildenbrand
2018-08-07 15:19 ` Jerome Glisse
2018-08-07 15:28 ` David Hildenbrand
2018-08-07 20:48 ` Oscar Salvador
2018-08-07 22:13 ` Jerome Glisse
2018-08-08 7:38 ` Oscar Salvador
2018-08-08 7:45 ` David Hildenbrand
2018-08-08 7:56 ` Oscar Salvador
2018-08-08 8:08 ` David Hildenbrand
2018-08-08 13:42 ` Oscar Salvador
2018-08-08 17:55 ` Jerome Glisse
2018-08-08 21:29 ` Oscar Salvador
2018-08-09 7:50 ` Oscar Salvador
2018-08-09 7:52 ` Oscar Salvador
2018-08-08 7:51 ` David Hildenbrand
2018-08-08 8:00 ` Oscar Salvador
2018-08-07 14:59 ` Michal Hocko
2018-08-07 15:18 ` Jerome Glisse
2018-08-08 6:47 ` Michal Hocko
2018-08-08 16:58 ` Jerome Glisse
2018-08-08 21:28 ` Oscar Salvador
2018-08-09 8:24 ` Michal Hocko
2018-08-09 14:27 ` Jerome Glisse
2018-08-09 15:09 ` Michal Hocko
2018-08-09 16:58 ` Jerome Glisse
2018-08-09 20:50 ` Oscar Salvador
2018-08-16 14:58 ` Oscar Salvador
2018-08-16 17:32 ` Jerome Glisse
2018-08-08 9:45 ` Oscar Salvador
2018-08-08 17:33 ` Jerome Glisse
2018-08-07 13:37 ` [RFC PATCH 3/3] mm/memory_hotplug: Refactor shrink_zone/pgdat_span osalvador
2018-08-07 14:16 ` [RFC PATCH 0/3] Do not touch pages in remove_memory path David Hildenbrand
2018-08-07 14:19 ` Oscar Salvador
2018-08-07 14:20 ` David Hildenbrand
2018-08-07 14:28 ` Oscar Salvador
2018-08-07 14:41 ` David Hildenbrand
2018-08-07 14:52 ` Oscar Salvador
2018-08-15 14:05 ` Pavel Tatashin [this message]
2018-08-15 14:32 ` 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=DM5PR21MB0508CEC7F586EBC89D2CCFBB9D3F0@DM5PR21MB0508.namprd21.prod.outlook.com \
--to=pavel.tatashin@microsoft.com \
--cc=akpm@linux-foundation.org \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=david@redhat.com \
--cc=jglisse@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=logang@deltatee.com \
--cc=mhocko@suse.com \
--cc=osalvador@suse.de \
--cc=osalvador@techadventures.net \
--cc=pasha.tatashin@oracle.com \
--cc=yasu.isimatu@gmail.com \
/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).