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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DAEBAC433F5 for ; Fri, 17 Dec 2021 08:32:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2DB7C6B0073; Fri, 17 Dec 2021 03:32:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 28B096B0075; Fri, 17 Dec 2021 03:32:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 17A286B0078; Fri, 17 Dec 2021 03:32:22 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0155.hostedemail.com [216.40.44.155]) by kanga.kvack.org (Postfix) with ESMTP id 08A166B0073 for ; Fri, 17 Dec 2021 03:32:22 -0500 (EST) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id C326186335 for ; Fri, 17 Dec 2021 08:32:11 +0000 (UTC) X-FDA: 78926618862.21.D1C464F Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by imf08.hostedemail.com (Postfix) with ESMTP id 5109716001A for ; Fri, 17 Dec 2021 08:32:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1639729930; x=1671265930; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=/906HC+MuyOxkjeUHQfzkRCXma0b/vzkgYvxqpSdayA=; b=KOIsRbqyHH7lF1Ggcy93xelFVhRE/xCJ8Xerm8Efc2PlfwM2GdMOAwc+ +cjMyO+cexG2505qzA6MTQs7AvJFnsRchXrnI8ILzPqcy5LozIdOOKZOT XsUqAwjDFFF2ikNtl+OxN9OGLUrl9SkRF1HVNVzqFWjVzp9TD8jthnRsV yBAtIZp734br+Kf7yZBDtS69oaf/M7/I9qYDCN5w7e9Fqedx0fM4Ra3Js 1ELa9imyTXO+SJDccxII/StP11GolNjGtnJUyCR6OUvMsavcAHufmZxVc sNDTh7uAGmQq/XoVkkC2ojzz5Cq3BY+hO8PP7FMvff+cLGNP2GwQVm84t A==; X-IronPort-AV: E=McAfee;i="6200,9189,10200"; a="239935088" X-IronPort-AV: E=Sophos;i="5.88,213,1635231600"; d="scan'208";a="239935088" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Dec 2021 00:32:08 -0800 X-IronPort-AV: E=Sophos;i="5.88,213,1635231600"; d="scan'208";a="519655790" Received: from unknown (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.239.13.11]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Dec 2021 00:32:05 -0800 From: "Huang, Ying" To: Li Xinhai Cc: linux-mm@kvack.org, akpm@linux-foundation.org, Zi Yan , "Kirill A. Shutemov" Subject: Re: [PATCH V4] mm/gup.c: stricter check on THP migration entry during follow_pmd_mask References: <20211217062559.737063-1-lixinhai.lxh@gmail.com> Date: Fri, 17 Dec 2021 16:32:03 +0800 In-Reply-To: <20211217062559.737063-1-lixinhai.lxh@gmail.com> (Li Xinhai's message of "Fri, 17 Dec 2021 14:25:59 +0800") Message-ID: <87czlvskcc.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=KOIsRbqy; spf=none (imf08.hostedemail.com: domain of ying.huang@intel.com has no SPF policy when checking 192.55.52.115) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com X-Rspamd-Queue-Id: 5109716001A X-Stat-Signature: 3sihz9rntafzkim17ck3cyy4atoh37x8 X-Rspamd-Server: rspam04 X-HE-Tag: 1639729925-266656 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: Li Xinhai writes: > When BUG_ON check for THP migration entry, the existing code only check > thp_migration_supported case, but not for !thp_migration_supported case. > If !thp_migration_supported() and !pmd_present(), the original code may > dead loop in theory. To make the BUG_ON check consistent, we need catch > both cases. > > Move the BUG_ON check one step earlier, because if the bug happen we > should know it instead of depend on FOLL_MIGRATION been used by caller. > > Because pmdval instead of *pmd is read by the is_pmd_migration_entry() > check, the existing code don't help to avoid useless locking within > pmd_migration_entry_wait(), so remove that check. > > Signed-off-by: Li Xinhai > Cc: Zi Yan > Cc: "Huang, Ying" > Cc: "Kirill A. Shutemov" > --- > V3->V4: > Fix typos > > V2->V3: > mention about the dead loop in commit message. > > V1->V2: > Move the BUG_ON() check before if(!(flags & FOLL_MIGRATION)); and add comments > for it. > > mm/gup.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/mm/gup.c b/mm/gup.c > index 2c51e9748a6a..1b500ca2a1b8 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -642,12 +642,17 @@ static struct page *follow_pmd_mask(struct vm_area_struct *vma, > } > retry: > if (!pmd_present(pmdval)) { > + /* > + * Should never reach here, if thp migration is not supported; > + * Otherwise, it must be a thp migration entry. > + */ > + VM_BUG_ON(!thp_migration_supported() || > + !is_pmd_migration_entry(pmdval)); > + > if (likely(!(flags & FOLL_MIGRATION))) > return no_page_table(vma, flags); > - VM_BUG_ON(thp_migration_supported() && > - !is_pmd_migration_entry(pmdval)); > - if (is_pmd_migration_entry(pmdval)) > - pmd_migration_entry_wait(mm, pmd); > + > + pmd_migration_entry_wait(mm, pmd); > pmdval = READ_ONCE(*pmd); > /* > * MADV_DONTNEED may convert the pmd to null because Looks good to me. Reviewed-by: "Huang, Ying" Best Regards, Huang, Ying