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=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 3AB80C33CAA for ; Tue, 21 Jan 2020 03:21:15 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F35D122522 for ; Tue, 21 Jan 2020 03:21:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F35D122522 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 761E36B0003; Mon, 20 Jan 2020 22:21:14 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6EBE26B0005; Mon, 20 Jan 2020 22:21:14 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5DA826B0006; Mon, 20 Jan 2020 22:21:14 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0240.hostedemail.com [216.40.44.240]) by kanga.kvack.org (Postfix) with ESMTP id 436F06B0003 for ; Mon, 20 Jan 2020 22:21:14 -0500 (EST) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with SMTP id DBA9B180AD80F for ; Tue, 21 Jan 2020 03:21:13 +0000 (UTC) X-FDA: 76400190426.03.bear46_7f0ea3e2eb805 X-HE-Tag: bear46_7f0ea3e2eb805 X-Filterd-Recvd-Size: 4963 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf17.hostedemail.com (Postfix) with ESMTP for ; Tue, 21 Jan 2020 03:21:13 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0703831B; Mon, 20 Jan 2020 19:21:12 -0800 (PST) Received: from [10.162.16.78] (p8cg001049571a15.blr.arm.com [10.162.16.78]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9435E3F6C4; Mon, 20 Jan 2020 19:21:10 -0800 (PST) Subject: Re: [PATCH v4] mm/mempolicy,hugetlb: Checking hstate for hugetlbfs page in vma_migratable To: Michal Hocko Cc: Li Xinhai , "linux-mm@kvack.org" , akpm , Mike Kravetz References: <1579147885-23511-1-git-send-email-lixinhai.lxh@gmail.com> <20200116095614.GO19428@dhcp22.suse.cz> <20200116215032206994102@gmail.com> <20200116151803.GV19428@dhcp22.suse.cz> <20200120113200.GZ18451@dhcp22.suse.cz> From: Anshuman Khandual Message-ID: <586fec3c-fb67-9756-1599-1d632eeb12d9@arm.com> Date: Tue, 21 Jan 2020 08:52:32 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200120113200.GZ18451@dhcp22.suse.cz> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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: On 01/20/2020 05:02 PM, Michal Hocko wrote: > On Mon 20-01-20 14:51:31, Anshuman Khandual wrote: >> >> >> On 01/16/2020 08:48 PM, Michal Hocko wrote: >>> On Thu 16-01-20 21:50:34, Li Xinhai wrote: >>>> On 2020-01-16=C2=A0at 17:56=C2=A0Michal Hocko=C2=A0wrote: >>>>> On Thu 16-01-20 04:11:25, Li Xinhai wrote: >>>>>> Checking hstate at early phase when isolating page, instead of dur= ing >>>>>> unmap and move phase, to avoid useless isolation. >>>>> >>>>> Could you be more specific what you mean by isolation and why does = it >>>>> matter? The patch description should really explain _why_ the chang= e is >>>>> needed or desirable.=20 >>>> >>>> The changelog can be improved: >>>> >>>> vma_migratable() is called to check if pages in vma can be migrated >>>> before go ahead to isolate, unmap and move pages. For hugetlb pages, >>>> hugepage_migration_supported(struct hstate *h) is one factor which >>>> decide if migration is supported. In current code, this function is = called >>>> from=C2=A0unmap_and_move_huge_page(), after isolating page has >>>> completed. >>>> This patch checks hstate from vma_migratable() and avoids isolating = pages >>>> which are not supported. >>> >>> This still explains what but not why this is relevant. If by isolatin= g >>> pages you mean isolate_lru_page then this really a noop for hugetlb >>> pages. Or do I still misread your changelog? >> >> unmap_and_move_hugepage() aborts migrating a HugeTLB page (from the li= st) >> if it's corresponding hstate does not support migration. >=20 > But all architectures support all hugeltb sizes unless I am missing > something. If there is some which doesn't then the changelog should > mention that. I have already asked for runtime effects with no data > provided. You are right that all hugetlb sizes are supported right now whether the platform defines arch_hugetlb_migration_supported() callback or not. But in theory an override for the arch callback can deny migration support for certain huge page sizes. >=20 > Just to make it clear. I am not objecting to the patch itself. I am > objecting to the very vague justification. The changelog doesn't explai= n > _why_ do we need to change this. Is it a bug, non-optimal code, pure > code clean up for a more robust code? AFAICS this tries to solve the problem like a sub-optimal code. But for now as there are no real HugeTLB cases for an early bail out, there can be an argument not to add new cost into via vma_migratable() which will be called more often for non-HugeTLB VMAs. Probably adding a comment in the code like this might just be sufficient. diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h index 5228c62..ca9c343 100644 --- a/include/linux/mempolicy.h +++ b/include/linux/mempolicy.h @@ -186,6 +186,13 @@ static inline bool vma_migratable(struct vm_area_str= uct *vma) return false; =20 #ifndef CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION + /* + * NOTE: hugepage_migration_supported() should have been called h= ere + * for an early bail out in cases where HugeTLB page sizes are no= t + * supported for migration. But for now as there are no such real + * cases, hence it is better not to add any additional cost here = by + * calling hugepage_migration_supported(). + */ if (vma->vm_flags & VM_HUGETLB) return false; #endif