Messages in this thread | | | Date | Thu, 14 Mar 2019 09:09:22 +0100 | From | Oscar Salvador <> | Subject | Re: [PATCH v2] mm/hotplug: fix offline undo_isolate_page_range() |
| |
On Wed, Mar 13, 2019 at 10:31:33AM -0400, Qian Cai wrote: > Also, after calling the "useless" undo_isolate_page_range() here, it > reaches the point of no returning by notifying MEM_OFFLINE. Those pages > will be marked as MIGRATE_MOVABLE again once onlining. The only thing > left to do is to decrease the number of isolated pageblocks zone > counter which would make some paths of the page allocation slower that > the above commit introduced. A memory block is usually at most 1GiB in > size, so an "int" should be enough to represent the number of pageblocks > in a block. Fix an incorrect comment along the way.
Well, x86_64's memblocks can be up to 2GB depending on the memory we got. Plus the fact that alloc_contig_range can be used to isolate 16GB-hugetlb pages on powerpc, and that could be a lot of pageblocks.
While an "int" could still hold, I think we should use "long" just to be more future-proof.
> > Fixes: 2ce13640b3f4 ("mm: __first_valid_page skip over offline pages") > Signed-off-by: Qian Cai <cai@lca.pw> > --- > > v2: return the nubmer of isolated pageblocks in start_isolate_page_range() per > Oscar; take the zone lock when undoing zone->nr_isolate_pageblock per > Michal. > > mm/memory_hotplug.c | 17 +++++++++++++---- > mm/page_alloc.c | 2 +- > mm/page_isolation.c | 16 ++++++++++------ > mm/sparse.c | 2 +- > 4 files changed, 25 insertions(+), 12 deletions(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index cd23c081924d..8ffe844766da 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -1580,7 +1580,7 @@ static int __ref __offline_pages(unsigned long start_pfn, > { > unsigned long pfn, nr_pages; > long offlined_pages; > - int ret, node; > + int ret, node, count;
I would rather use a meaningful name here, like "nr_isolated_pageblocks" or something like that.
-- Oscar Salvador SUSE L3
| |