| Message ID | 20180816100628.26428-4-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/memory_hotplug: online/offline_pages refactorings | expand |
On Thu, Aug 16, 2018 at 12:06:26PM +0200, David Hildenbrand wrote: > + > +/* check if all mem sections are offline */ > +bool mem_sections_offline(unsigned long pfn, unsigned long end_pfn) > +{ > + for (; pfn < end_pfn; pfn += PAGES_PER_SECTION) { > + unsigned long section_nr = pfn_to_section_nr(pfn); > + > + if (WARN_ON(!valid_section_nr(section_nr))) > + continue; > + if (online_section_nr(section_nr)) > + return false; > + } > + return true; > +} AFAICS pages_correctly_probed will catch this first. pages_correctly_probed checks for the section to be: - present - valid - !online Maybe it makes sense to rename it, and write another pages_correctly_probed routine for the offline case. So all checks would stay in memory_block_action level, and we would not need the mem_sections_offline/online stuff. Thanks
On 16.08.2018 12:47, Oscar Salvador wrote: > On Thu, Aug 16, 2018 at 12:06:26PM +0200, David Hildenbrand wrote: > >> + >> +/* check if all mem sections are offline */ >> +bool mem_sections_offline(unsigned long pfn, unsigned long end_pfn) >> +{ >> + for (; pfn < end_pfn; pfn += PAGES_PER_SECTION) { >> + unsigned long section_nr = pfn_to_section_nr(pfn); >> + >> + if (WARN_ON(!valid_section_nr(section_nr))) >> + continue; >> + if (online_section_nr(section_nr)) >> + return false; >> + } >> + return true; >> +} > > AFAICS pages_correctly_probed will catch this first. > pages_correctly_probed checks for the section to be: > > - present > - valid > - !online Right, I missed that function. > > Maybe it makes sense to rename it, and write another pages_correctly_probed routine > for the offline case. > > So all checks would stay in memory_block_action level, and we would not need > the mem_sections_offline/online stuff. I guess I would rather have it all moved into online_pages/offline_pages, so we have a clean interface. > > Thanks >
On 8/16/18 7:00 AM, David Hildenbrand wrote: > On 16.08.2018 12:47, Oscar Salvador wrote: >> On Thu, Aug 16, 2018 at 12:06:26PM +0200, David Hildenbrand wrote: >> >>> + >>> +/* check if all mem sections are offline */ >>> +bool mem_sections_offline(unsigned long pfn, unsigned long end_pfn) >>> +{ >>> + for (; pfn < end_pfn; pfn += PAGES_PER_SECTION) { >>> + unsigned long section_nr = pfn_to_section_nr(pfn); >>> + >>> + if (WARN_ON(!valid_section_nr(section_nr))) >>> + continue; >>> + if (online_section_nr(section_nr)) >>> + return false; >>> + } >>> + return true; >>> +} >> >> AFAICS pages_correctly_probed will catch this first. >> pages_correctly_probed checks for the section to be: >> >> - present >> - valid >> - !online > > Right, I missed that function. > >> >> Maybe it makes sense to rename it, and write another pages_correctly_probed routine >> for the offline case. >> >> So all checks would stay in memory_block_action level, and we would not need >> the mem_sections_offline/online stuff. I am OK with that, but will wait for a patch to review. Pavel
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index 0859130e4db8..addfa41c047a 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -1228,6 +1228,8 @@ static inline int online_section_nr(unsigned long nr) } #ifdef CONFIG_MEMORY_HOTPLUG +bool mem_sections_online(unsigned long pfn, unsigned long end_pfn); +bool mem_sections_offline(unsigned long pfn, unsigned long end_pfn); void online_mem_sections(unsigned long start_pfn, unsigned long end_pfn); #ifdef CONFIG_MEMORY_HOTREMOVE void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn); diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 30d2fa42b0bb..3dc6d2a309c2 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -901,6 +901,8 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages, int online_typ return -EINVAL; if (!IS_ALIGNED(nr_pages, PAGES_PER_SECTION)) return -EINVAL; + if (!mem_sections_offline(pfn, pfn + nr_pages)) + return -EINVAL; /* * We can't use pfn_to_nid() because nid might be stored in struct page @@ -1609,6 +1611,9 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages) return -EINVAL; if (!IS_ALIGNED(nr_pages, PAGES_PER_SECTION)) return -EINVAL; + if (!mem_sections_online(start_pfn, end_pfn)) + return -EINVAL; + /* This makes hotplug much easier...and readable. we assume this for now. .*/ if (!test_pages_in_a_zone(start_pfn, end_pfn, &valid_start, &valid_end)) diff --git a/mm/sparse.c b/mm/sparse.c index 10b07eea9a6e..44693cf38ca9 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -520,6 +520,34 @@ void __init sparse_init(void) #ifdef CONFIG_MEMORY_HOTPLUG +/* check if all mem sections are online */ +bool mem_sections_online(unsigned long pfn, unsigned long end_pfn) +{ + for (; pfn < end_pfn; pfn += PAGES_PER_SECTION) { + unsigned long section_nr = pfn_to_section_nr(pfn); + + if (WARN_ON(!valid_section_nr(section_nr))) + continue; + if (!online_section_nr(section_nr)) + return false; + } + return true; +} + +/* check if all mem sections are offline */ +bool mem_sections_offline(unsigned long pfn, unsigned long end_pfn) +{ + for (; pfn < end_pfn; pfn += PAGES_PER_SECTION) { + unsigned long section_nr = pfn_to_section_nr(pfn); + + if (WARN_ON(!valid_section_nr(section_nr))) + continue; + if (online_section_nr(section_nr)) + return false; + } + return true; +} + /* Mark all memory sections within the pfn range as online */ void online_mem_sections(unsigned long start_pfn, unsigned long end_pfn) {
Let's add some more sanity checking now that onlining/offlining code works completely on section basis. This will make sure that we will never try to online/offline sections that are already (or partially) in the desired state. Signed-off-by: David Hildenbrand <david@redhat.com> --- include/linux/mmzone.h | 2 ++ mm/memory_hotplug.c | 5 +++++ mm/sparse.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+)