On 25.08.20 10:26, Michal Hocko wrote: > On Mon 24-08-20 14:58:11, Li Xinhai wrote: >> In has_unmovable_pages(), the page parameter would not always be the >> first page within a pageblock (see how the page pointer is passed in from >> start_isolate_page_range() after call __first_valid_page()), so that >> would cause checking unmovable pages span two pageblocks. > > This might lead to false negatives when an unrelated block would cause > an isolation failure. > >> After this patch, the checking is enforced within one pageblock no matter >> the page is first one or not, and obey the semantics of this function. >> >> This issue is found by code inspection. >> >> Signed-off-by: Li Xinhai <lixinhai.lxh@xxxxxxxxx> > > I have tried to find the commit which has introduced this but there was > so much churn around that I gave up. Not that we care all that much > because it seems that we simply never try to isolate pageblocks with > holes. Memory hotplug disallows that explicitly and the CMA allocator > doesn't trip over that either. Or maybe we were just lucky or a silent > failure didn't really trigger any attention. It will never happen. - memory offlining excludes ranges without holes. - virtio-mem (alloc_contig_range()) never has ranges with holes. - gigantic pages (alloc_contig_range()) never use ranges with holes - CMA (alloc_contig_range()) never uses ranges with holes. Trying to allocate something that's not there would be troublesome already. I *guess* the handling is in place for corner cases of alloc_contig_range(), whereby one tries to allocate a sub-MAX_ORDER-1 page, and alloc_contig_range() automatically tries to isolate all pageblocks spanning full MAX_ORDER-1 pages. See pfn_max_align_down/pfn_max_align_up. If there would be a memory hole close by, that could trigger. It would, however, only trigger for CMA, as that is the only user allocating in sub-MAX_ORDER-1 granularity right now. I think we should just disallow/check for holes in the extended range (pfn_max_align_down/pfn_max_align_up) in alloc_contig_range() and document the expected behavior for start_isolate_page_range() - to not contain any holes. This get's rid of a whole bunch of unnecessary pfn_to_online_page() calls. We would no longer able to alloc_contig_range() pages close to a memory hole in case of CMA (in corner cases) - but I doubt this is an actual issue. If we agree, I can send patches. -- Thanks, David / dhildenb
