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=-2.5 required=3.0 tests=MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 08EADC2D0DB for ; Mon, 20 Jan 2020 10:12:07 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id BA18020684 for ; Mon, 20 Jan 2020 10:12:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BA18020684 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 3AFE26B0624; Mon, 20 Jan 2020 05:12:06 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 339F46B0625; Mon, 20 Jan 2020 05:12:06 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 201256B0626; Mon, 20 Jan 2020 05:12:06 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0031.hostedemail.com [216.40.44.31]) by kanga.kvack.org (Postfix) with ESMTP id 079406B0624 for ; Mon, 20 Jan 2020 05:12:06 -0500 (EST) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with SMTP id B7E9633C4 for ; Mon, 20 Jan 2020 10:12:05 +0000 (UTC) X-FDA: 76397597010.10.fan15_1238abe257a3c X-HE-Tag: fan15_1238abe257a3c X-Filterd-Recvd-Size: 7218 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by imf23.hostedemail.com (Postfix) with ESMTP for ; Mon, 20 Jan 2020 10:12:05 +0000 (UTC) Received: by mail-wr1-f66.google.com with SMTP id q10so28866678wrm.11 for ; Mon, 20 Jan 2020 02:12:04 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=adbdjiVau9etS2t3ssB2A258KBgSziRfDhmtS4rPNeg=; b=GPbr3MEDsz9vapAfVbmMleOQ8rGOsfKAoFwiKdvBuSCQds9DeZFK6lzyW5ErqTbwOL zyRn06/ji6KqTeHG1XeoyOR8lkGzcuTPXgZFFTmEUskXFjM6DBtIKOguKGDWEWFSu47c /qEAlcdYIB82XTaux3pYPxsb7lSx091VFR1t4bFUFhQHzla3sRNzdMtdV7ViFHIKcbR1 z/RsAU/qLUVvP9jKgHm70hsAAz7A8qO2MrWRWyN/RtPbfl8EaPRlaohqQkX+Admj1Bx0 FHvKEwNpZvHfdweZgimq7u5N8PvLilsmMC31tsSZz2HVIBj5jhKsAkN8TEBNjRl9lGh5 Lr1Q== X-Gm-Message-State: APjAAAXa3LiILuZSCmaDFCP6EBApDILaNya0Dg9+mSD0m75XaM54cPyd NZlpGWxTAsXMbvOVXVPQbuc= X-Google-Smtp-Source: APXvYqwW64eGQuTciaQ3HWJJV04lMNiwPvt8E1ucZmn7e11oSVtpFHrRA3RB5+CQWUXtndsga1WrSA== X-Received: by 2002:adf:fa12:: with SMTP id m18mr17148681wrr.309.1579515124093; Mon, 20 Jan 2020 02:12:04 -0800 (PST) Received: from localhost (prg-ext-pat.suse.com. [213.151.95.130]) by smtp.gmail.com with ESMTPSA id p26sm21404509wmc.24.2020.01.20.02.12.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Jan 2020 02:12:03 -0800 (PST) Date: Mon, 20 Jan 2020 11:12:02 +0100 From: Michal Hocko To: Li Xinhai Cc: "linux-mm@kvack.org" , akpm , torvalds , "linux-kernel@vger.kernel.org" , Mike Kravetz Subject: Re: [PATCH v4] mm/mempolicy,hugetlb: Checking hstate for hugetlbfs page in vma_migratable Message-ID: <20200120101202.GU18451@dhcp22.suse.cz> References: <1579147885-23511-1-git-send-email-lixinhai.lxh@gmail.com> <20200116095614.GO19428@dhcp22.suse.cz> <20200116215032206994102@gmail.com> <20200116151803.GV19428@dhcp22.suse.cz> <20200116233817972969139@gmail.com> <20200117111629898234212@gmail.com> <20200118111121432688303@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20200118111121432688303@gmail.com> User-Agent: Mutt/1.12.2 (2019-09-21) 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 Sat 18-01-20 11:11:23, Li Xinhai wrote: > On 2020-01-17=A0at 11:16=A0Li Xinhai=A0wrote: > >On 2020-01-16=A0at 23:38=A0Li Xinhai=A0wrote: > >>On 2020-01-16=A0at 23:18=A0Michal Hocko=A0wrote: > >>>On Thu 16-01-20 21:50:34, Li Xinhai wrote: > >>>> On 2020-01-16=A0at 17:56=A0Michal Hocko=A0wrote: > >>>> >On Thu 16-01-20 04:11:25, Li Xinhai wrote: > >>>> >> Checking hstate at early phase when isolating page, instead of = during > >>>> >> unmap and move phase, to avoid useless isolation. > >>>> > > >>>> >Could you be more specific what you mean by isolation and why doe= s it > >>>> >matter? The patch description should really explain _why_ the cha= nge is > >>>> >needed or desirable. > >>>> > >>>> The changelog can be improved: > >>>> > >>>> vma_migratable() is called to check if pages in vma can be migrate= d > >>>> before go ahead to isolate, unmap and move pages. For hugetlb page= s, > >>>> hugepage_migration_supported(struct hstate *h) is one factor which > >>>> decide if migration is supported. In current code, this function i= s called > >>>> from=A0unmap_and_move_huge_page(), after isolating page has > >>>> completed. > >>>> This patch checks hstate from vma_migratable() and avoids isolatin= g pages > >>>> which are not supported. > >>> > >>>This still explains what but not why this is relevant. If by isolati= ng > >>>pages you mean isolate_lru_page then this really a noop for hugetlb > >>>pages. Or do I still misread your changelog? > >> > >>I mean=A0isolate_huge_page will queue pages for moving, and > >>unmap_and_move_huge_page will call > >>hugepage_migration_supported then refuse moving. > >> > > > >Forgot to mention that this patch has no relevant with this one > >https://patchwork.kernel.org/patch/11331639/,=A0 > > > >Code change at here is common for avoids walking page table and > >isolate hugepage in case architecture or page size are not supported > >for migration. Comments from code are copied here: > > > >static int unmap_and_move_huge_page(...) > >{ > > /* > > * Migratability of hugepages depends on architectures and their size. > > * This check is necessary because some callers of hugepage migration > > * like soft offline and memory hotremove don't walk through page > > * tables or check whether the hugepage is pmd-based or not before > > * kicking migration. > > */ > > if (!hugepage_migration_supported(page_hstate(hpage))) { > > putback_active_hugepage(hpage); > > return -ENOSYS; > > } > >} > > > >For current code change, we are able to know the 'hstate' because we h= ave 'vma', so > >do early check instead of later. > >=20 >=20 > https://lore.kernel.org/linux-mm/20200117111629898234212@gmail.com/ >=20 > Revise with more details on changelog for reason why this patch > is need. Thanks for your comments. >=20 > --- > vma_migratable() is called to check if pages in vma can be migrated > before go ahead to further actions. Currently it is used in below code > path: > - task_numa_work (kernel\sched\fair.c) > - mbind (mm\mempolicy.c) > - move_pages (mm\migrate.c) >=20 > For hugetlb mapping, vma is migratable or not is determined by: > - CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATION > - arch_hugetlb_migration_supported >=20 > Issue: current code only checks for CONFIG_ARCH_ENABLE_HUGEPAGE_MIGRATI= ON. Explain why this is an issue. Because as things stand now this doesn't cause any problems at all. All architectures simply support migrating all hugetlb sizes AFAIK. If this is not the case then mention which of them are not. > This patch checks the two factors to impove code logic. Besides, caller > of vma_migratable can take action properly in case architecture don't > support migration, e.g., mbind don't go further to try isolating/moving > pages, but currently it do continue because vma_migratable say yes. I do not follow. What are you trying to say here? The changelog should be explicit it doesn't really fix any existing problem. And the whole purpose is a code robustness cleanup. Should we ever have an architecture that doesn't support all hugetlb sizes then we would safe pointless huge page isolation for a page that cannot be migrated in the first place. See how the above actually explains _why_ you want to make the change? > No adding for Fix tag, since vma_migratable was implemented before > arch_hugetlb_migration_supported, it is up to the caller to use it. This is also of no relevance. --=20 Michal Hocko SUSE Labs