lkml.org 
[lkml]   [2017]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 07/14] mm: consider zone which is not fully populated to have holes
On Thu 18-05-17 18:14:39, Vlastimil Babka wrote:
> On 05/15/2017 10:58 AM, Michal Hocko wrote:
[...]
> > #ifdef CONFIG_MEMORY_HOTPLUG
> > +/*
> > + * Return page for the valid pfn only if the page is online. All pfn
> > + * walkers which rely on the fully initialized page->flags and others
> > + * should use this rather than pfn_valid && pfn_to_page
> > + */
> > +#define pfn_to_online_page(pfn) \
> > +({ \
> > + struct page *___page = NULL; \
> > + \
> > + if (online_section_nr(pfn_to_section_nr(pfn))) \
> > + ___page = pfn_to_page(pfn); \
> > + ___page; \
> > +})
>
> This seems to be already assuming pfn_valid() to be true. There's no
> "pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS" check and the comment
> suggests as such, but...

Yes, we should check the validity of the section number. We do not have
to check whether the section is valid because online sections are a
subset of those that are valid.

> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index 05796ee974f7..c3a146028ba6 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -929,6 +929,9 @@ static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
> > unsigned long i;
> > unsigned long onlined_pages = *(unsigned long *)arg;
> > struct page *page;
> > +
> > + online_mem_sections(start_pfn, start_pfn + nr_pages);
>
> Shouldn't this be moved *below* the loop that initializes struct pages?
> In the offline case you do mark sections offline before "tearing" struct
> pages, so that should be symmetric.

You are right! Andrew, could you fold the following intot the patch?
---
From 0550b61203d6970b47fd79f5e6372dccd143cbec Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Thu, 18 May 2017 18:38:24 +0200
Subject: [PATCH] fold me "mm: consider zone which is not fully populated to
have holes"

- check valid section number in pfn_to_online_page - Vlastimil
- mark sections online after all struct pages are initialized in
online_pages_range - Vlastimil

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

diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index aec88657ec49..9e0249d0f5e4 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -22,8 +22,9 @@ struct resource;
#define pfn_to_online_page(pfn) \
({ \
struct page *___page = NULL; \
+ unsigned long ___nr = pfn_to_section_nr(pfn); \
\
- if (online_section_nr(pfn_to_section_nr(pfn))) \
+ if (___nr < NR_MEM_SECTIONS && online_section_nr(___nr))\
___page = pfn_to_page(pfn); \
___page; \
})
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index de30822642c6..a31b547c11f0 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -746,14 +746,15 @@ static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
unsigned long onlined_pages = *(unsigned long *)arg;
struct page *page;

- online_mem_sections(start_pfn, start_pfn + nr_pages);
-
if (PageReserved(pfn_to_page(start_pfn)))
for (i = 0; i < nr_pages; i++) {
page = pfn_to_page(start_pfn + i);
(*online_page_callback)(page);
onlined_pages++;
}
+
+ online_mem_sections(start_pfn, start_pfn + nr_pages);
+
*(unsigned long *)arg = onlined_pages;
return 0;
}
--
2.11.0

--
Michal Hocko
SUSE Labs

\
 
 \ /
  Last update: 2017-05-18 18:42    [W:0.162 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site