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 21887C433F5 for ; Fri, 17 Dec 2021 03:01:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3DD0F6B0071; Thu, 16 Dec 2021 22:01:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 38D606B0072; Thu, 16 Dec 2021 22:01:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2A2EA6B0073; Thu, 16 Dec 2021 22:01:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0128.hostedemail.com [216.40.44.128]) by kanga.kvack.org (Postfix) with ESMTP id 1758A6B0071 for ; Thu, 16 Dec 2021 22:01:44 -0500 (EST) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id CCD048249980 for ; Fri, 17 Dec 2021 03:01:33 +0000 (UTC) X-FDA: 78925785666.21.257A216 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by imf22.hostedemail.com (Postfix) with ESMTP id 34292C0015 for ; Fri, 17 Dec 2021 03:01:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1639710092; x=1671246092; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=NBtkzA3wEMSmVnzg/oDmxi+GFnpsehUPWR9+h7WvZqA=; b=A/fIBLRzHkPEkUHdjpZ06xETY+k7FiUbcphML1tUYwzacy1a+I3tHQK7 SZk4saZh5nFrNDX7/yLs4wdxkLYJeeKRkTvYYIyJHg/aSpQeyJlELXj15 avcru+MIAHiHwBRxYBvJRA3imrAPfhzZuLNnEPTxFZYNJQMXhJ23SkaNT OjIeOrthB/l1A2QXBNXbW9jLQ5fxjOAb9zAkhCuwFlpQHJlUZZNB/z4Vr aUcp1/swz9FDI6jT4CoOefFR9mNkW4gSDKVFjnUVYrNxDd1w4eh4HPWvV O0/Xxyxnsu3fUzjMRcj8eo41LEc8KN1SbxXJCO3OcwoHpZO/wGw9Ctw7c g==; X-IronPort-AV: E=McAfee;i="6200,9189,10200"; a="300441529" X-IronPort-AV: E=Sophos;i="5.88,213,1635231600"; d="scan'208";a="300441529" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Dec 2021 19:01:31 -0800 X-IronPort-AV: E=Sophos;i="5.88,213,1635231600"; d="scan'208";a="506597615" Received: from unknown (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.239.13.11]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Dec 2021 19:01:29 -0800 From: "Huang, Ying" To: Li Xinhai Cc: linux-mm@kvack.org, akpm@linux-foundation.org, Zi Yan , "Kirill A. Shutemov" Subject: Re: [PATCH V3] mm/gup.c: stricter check on THP migration entry during follow_pmd_mask References: <20211217023418.731424-1-lixinhai.lxh@gmail.com> Date: Fri, 17 Dec 2021 11:01:27 +0800 In-Reply-To: <20211217023418.731424-1-lixinhai.lxh@gmail.com> (Li Xinhai's message of "Fri, 17 Dec 2021 10:34:18 +0800") Message-ID: <87wnk4rl2w.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 X-Rspamd-Queue-Id: 34292C0015 X-Stat-Signature: tc5wuzxpwmhms46qwsz8k4q69wjypxaw Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b="A/fIBLRz"; spf=none (imf22.hostedemail.com: domain of ying.huang@intel.com has no SPF policy when checking 134.134.136.31) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com X-Rspamd-Server: rspam10 X-HE-Tag: 1639710092-649966 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 exsiting code only check s/exsiting/existing/ Found some misspelling in the comments too. Please fix them with some tool. Best Regards, Huang, Ying > 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 eariler, 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" > --- > 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..94d0e586ca0b 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 miration 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