Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932667AbdDQFrb (ORCPT ); Mon, 17 Apr 2017 01:47:31 -0400 Received: from mail-pg0-f49.google.com ([74.125.83.49]:33758 "EHLO mail-pg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751030AbdDQFr2 (ORCPT ); Mon, 17 Apr 2017 01:47:28 -0400 Date: Mon, 17 Apr 2017 14:47:20 +0900 From: Joonsoo Kim To: Michal Hocko Cc: linux-mm@kvack.org, Andrew Morton , Mel Gorman , Vlastimil Babka , Andrea Arcangeli , Jerome Glisse , Reza Arbab , Yasuaki Ishimatsu , qiuxishi@huawei.com, Kani Toshimitsu , slaoub@gmail.com, Andi Kleen , David Rientjes , Daniel Kiper , Igor Mammedov , Vitaly Kuznetsov , LKML Subject: Re: your mail Message-ID: <20170417054718.GD1351@js1304-desktop> References: <20170410110351.12215-1-mhocko@kernel.org> <20170415121734.6692-1-mhocko@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170415121734.6692-1-mhocko@kernel.org> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2109 Lines: 42 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.