linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] Do not touch pages in remove_memory path
@ 2018-08-07 13:37 osalvador
  2018-08-07 13:37 ` [RFC PATCH 1/3] mm/memory_hotplug: Add nid parameter to arch_remove_memory osalvador
                   ` (4 more replies)
  0 siblings, 5 replies; 43+ messages in thread
From: osalvador @ 2018-08-07 13:37 UTC (permalink / raw)
  To: akpm
  Cc: mhocko, dan.j.williams, pasha.tatashin, jglisse, david,
	yasu.isimatu, logang, dave.jiang, linux-mm, linux-kernel,
	Oscar Salvador

From: Oscar Salvador <osalvador@suse.de>

This tries to fix [1], which was reported by David Hildenbrand, and also
does some cleanups/refactoring.

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


^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2018-08-16 17:32 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2018-08-15 14:32   ` Oscar Salvador

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).