From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3D3FFC4724C for ; Fri, 1 May 2020 14:06:49 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E406E206D6 for ; Fri, 1 May 2020 14:06:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E406E206D6 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=xmission.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 480188E0005; Fri, 1 May 2020 10:06:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4090A8E0001; Fri, 1 May 2020 10:06:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2D14D8E0005; Fri, 1 May 2020 10:06:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0148.hostedemail.com [216.40.44.148]) by kanga.kvack.org (Postfix) with ESMTP id 154118E0001 for ; Fri, 1 May 2020 10:06:48 -0400 (EDT) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id C9F2C2C1E for ; Fri, 1 May 2020 14:06:47 +0000 (UTC) X-FDA: 76768326054.18.veil58_10978ae9225a X-HE-Tag: veil58_10978ae9225a X-Filterd-Recvd-Size: 7103 Received: from out01.mta.xmission.com (out01.mta.xmission.com [166.70.13.231]) by imf06.hostedemail.com (Postfix) with ESMTP for ; Fri, 1 May 2020 14:06:46 +0000 (UTC) Received: from in02.mta.xmission.com ([166.70.13.52]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jUWJM-0004gf-TM; Fri, 01 May 2020 08:06:40 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jUWJK-0006hx-Mv; Fri, 01 May 2020 08:06:40 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: js1304@gmail.com Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Vlastimil Babka , Laura Abbott , "Aneesh Kumar K . V" , Mel Gorman , Michal Hocko , Johannes Weiner , Roman Gushchin , Minchan Kim , Rik van Riel , Christian Koenig , Huang Rui , "Rafael J . Wysocki" , Pavel Machek , kernel-team@lge.com, Christoph Hellwig , Joonsoo Kim References: <1588130803-20527-1-git-send-email-iamjoonsoo.kim@lge.com> <1588130803-20527-4-git-send-email-iamjoonsoo.kim@lge.com> Date: Fri, 01 May 2020 09:03:19 -0500 In-Reply-To: <1588130803-20527-4-git-send-email-iamjoonsoo.kim@lge.com> (js's message of "Wed, 29 Apr 2020 12:26:36 +0900") Message-ID: <87h7wzvjko.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1jUWJK-0006hx-Mv;;;mid=<87h7wzvjko.fsf@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19OrQA4UGeE4A+/sWRinwFintIJ4nH0kP4= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH v2 03/10] kexec: separate PageHighMem() and PageHighMemZone() use case X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: js1304@gmail.com writes: > From: Joonsoo Kim > > Until now, PageHighMem() is used for two different cases. One is to check > if there is a direct mapping for this page or not. The other is to check > the zone of this page, that is, weather it is the highmem type zone or not. > > Now, we have separate functions, PageHighMem() and PageHighMemZone() for > each cases. Use appropriate one. > > Note that there are some rules to determine the proper macro. > > 1. If PageHighMem() is called for checking if the direct mapping exists > or not, use PageHighMem(). > 2. If PageHighMem() is used to predict the previous gfp_flags for > this page, use PageHighMemZone(). The zone of the page is related to > the gfp_flags. > 3. If purpose of calling PageHighMem() is to count highmem page and > to interact with the system by using this count, use PageHighMemZone(). > This counter is usually used to calculate the available memory for an > kernel allocation and pages on the highmem zone cannot be available > for an kernel allocation. > 4. Otherwise, use PageHighMemZone(). It's safe since it's implementation > is just copy of the previous PageHighMem() implementation and won't > be changed. > > I apply the rule #2 for this patch. Hmm. What happened to the notion of deprecating and reducing the usage of highmem? I know that we have some embedded architectures where it is still important but this feels like it flies in the face of that. This part of kexec would be much more maintainable if it had a proper mm layer helper that tested to see if the page matched the passed in gfp flags. That way the mm layer could keep changing and doing weird gyrations and this code would not care. What would be really helpful is if there was a straight forward way to allocate memory whose physical address fits in the native word size. All I know for certain about this patch is that it takes a piece of code that looked like it made sense, and transfroms it into something I can not easily verify, and can not maintain. As it makes the code unmaintainable. Nacked-by: "Eric W. Biederman" Not to say that the code isn't questionable as it is, but this change just pushes it over the edge into gobbledy gook. Eric > Acked-by: Roman Gushchin > Signed-off-by: Joonsoo Kim > --- > kernel/kexec_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c > index ba1d91e..33097b7 100644 > --- a/kernel/kexec_core.c > +++ b/kernel/kexec_core.c > @@ -766,7 +766,7 @@ static struct page *kimage_alloc_page(struct kimage *image, > * gfp_flags honor the ones passed in. > */ > if (!(gfp_mask & __GFP_HIGHMEM) && > - PageHighMem(old_page)) { > + PageHighMemZone(old_page)) { > kimage_free_pages(old_page); > continue; > }