On Sat, Apr 15, 2017 at 02:17:31PM +0200, Michal Hocko wrote: > Hi, > here I 3 more preparatory patches which I meant to send on Thursday but > forgot... After more thinking about pfn walkers I have realized that > the current code doesn't check offline holes in zones. From a quick > review that doesn't seem to be a problem currently. Pfn walkers can race > with memory offlining and with the original hotplug impementation those > offline pages can change the zone but I wasn't able to find any serious > problem other than small confusion. The new hotplug code, will not have > any valid zone, though so those code paths should check PageReserved > to rule offline holes. I hope I have addressed all of them in these 3 > patches. I would appreciate if Vlastimil and Jonsoo double check after > me. Hello, Michal. s/Jonsoo/Joonsoo. :) I'm not sure that it's a good idea to add PageResereved() check in pfn walkers. First, this makes struct page validity check as two steps, pfn_valid() and then PageResereved(). If we should not use struct page in this case, it's better to pfn_valid() returns false rather than adding a separate check. Anyway, we need to fix more places (all pfn walker?) if we want to check validity by two steps. The other problem I found is that your change will makes some contiguous zones to be considered as non-contiguous. Memory allocated by memblock API is also marked as PageResereved. If we consider this as a hole, we will set such a zone as non-contiguous. And, I guess that it's not enough to check PageResereved() in pageblock_pfn_to_page() in order to skip these pages in compaction. If holes are in the middle of the pageblock, pageblock_pfn_to_page() cannot catch it and compaction will use struct page for this hole. Therefore, I think that making pfn_valid() return false for not onlined memory is a better solution for this problem. I don't know the implementation detail for hotplug and I don't see your recent change but we may defer memmap initialization until the zone is determined. It will make pfn_valid() return false for un-initialized range. Thanks. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
