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=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 9DC3DC43462 for ; Fri, 9 Apr 2021 08:50:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5EE2961185 for ; Fri, 9 Apr 2021 08:50:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232320AbhDIIul (ORCPT ); Fri, 9 Apr 2021 04:50:41 -0400 Received: from mga05.intel.com ([192.55.52.43]:45794 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232156AbhDIIui (ORCPT ); Fri, 9 Apr 2021 04:50:38 -0400 IronPort-SDR: Drgww6uLaV4L8ECM4dKXbxLpnQVdrc6hGDBEnavJ7bWZJSVI58gTkHl7al8x1awTiheOSA+z0G 7kECTuI0mIlQ== X-IronPort-AV: E=McAfee;i="6000,8403,9948"; a="278997562" X-IronPort-AV: E=Sophos;i="5.82,209,1613462400"; d="scan'208";a="278997562" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Apr 2021 01:50:25 -0700 IronPort-SDR: 2nVKxe0lWGZr4gIsqpDas1QZRO3N/XmeLyWI1dgidfpFhm0vVZAym3tcAZ4cB7jhA/kAIuGcOS 0oU0VWjOTplQ== X-IronPort-AV: E=Sophos;i="5.82,209,1613462400"; d="scan'208";a="422644512" Received: from yhuang6-desk1.sh.intel.com (HELO yhuang6-desk1.ccr.corp.intel.com) ([10.239.13.1]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Apr 2021 01:50:22 -0700 From: "Huang, Ying" To: Miaohe Lin Cc: , , , , , , , , , , , , Subject: Re: [PATCH 4/5] mm/swap_state: fix potential faulted in race in swap_ra_info() References: <20210408130820.48233-1-linmiaohe@huawei.com> <20210408130820.48233-5-linmiaohe@huawei.com> Date: Fri, 09 Apr 2021 16:50:18 +0800 In-Reply-To: <20210408130820.48233-5-linmiaohe@huawei.com> (Miaohe Lin's message of "Thu, 8 Apr 2021 09:08:19 -0400") Message-ID: <874kgfyh85.fsf@yhuang6-desk1.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 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Miaohe Lin writes: > While we released the pte lock, somebody else might faulted in this pte. > So we should check whether it's swap pte first to guard against such race > or swp_type would be unexpected. And we can also avoid some unnecessary > readahead cpu cycles possibly. > > Fixes: ec560175c0b6 ("mm, swap: VMA based swap readahead") > Signed-off-by: Miaohe Lin > --- > mm/swap_state.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/mm/swap_state.c b/mm/swap_state.c > index 709c260d644a..3bf0d0c297bc 100644 > --- a/mm/swap_state.c > +++ b/mm/swap_state.c > @@ -724,10 +724,10 @@ static void swap_ra_info(struct vm_fault *vmf, > { > struct vm_area_struct *vma = vmf->vma; > unsigned long ra_val; > - swp_entry_t entry; > + swp_entry_t swap_entry; > unsigned long faddr, pfn, fpfn; > unsigned long start, end; > - pte_t *pte, *orig_pte; > + pte_t *pte, *orig_pte, entry; > unsigned int max_win, hits, prev_win, win, left; > #ifndef CONFIG_64BIT > pte_t *tpte; > @@ -742,8 +742,13 @@ static void swap_ra_info(struct vm_fault *vmf, > > faddr = vmf->address; > orig_pte = pte = pte_offset_map(vmf->pmd, faddr); > - entry = pte_to_swp_entry(*pte); > - if ((unlikely(non_swap_entry(entry)))) { > + entry = *pte; > + if (unlikely(!is_swap_pte(entry))) { > + pte_unmap(orig_pte); > + return; > + } > + swap_entry = pte_to_swp_entry(entry); > + if ((unlikely(non_swap_entry(swap_entry)))) { > pte_unmap(orig_pte); > return; > } This isn't a real issue. entry or swap_entry isn't used in this function. And we have enough checking when we really operate the PTE entries later. But I admit it's confusing. So I suggest to just remove the checking. We will check it when necessary. Best Regards, Huang, Ying 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=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 6B9FBC433B4 for ; Fri, 9 Apr 2021 08:50:29 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EDD7661185 for ; Fri, 9 Apr 2021 08:50:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EDD7661185 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 7CB376B0070; Fri, 9 Apr 2021 04:50:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 77B5F6B0071; Fri, 9 Apr 2021 04:50:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 61C286B0072; Fri, 9 Apr 2021 04:50:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0087.hostedemail.com [216.40.44.87]) by kanga.kvack.org (Postfix) with ESMTP id 455506B0070 for ; Fri, 9 Apr 2021 04:50:28 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id EBA93A76D for ; Fri, 9 Apr 2021 08:50:27 +0000 (UTC) X-FDA: 78012207294.27.6DB96D2 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by imf17.hostedemail.com (Postfix) with ESMTP id 7A48440002CE for ; Fri, 9 Apr 2021 08:50:25 +0000 (UTC) IronPort-SDR: +7Q9VCcrjbINJGz0QYXUPbwqUjeA3TUt/YBv5FmDEkz7z2LHCelbVNQ/31bCYEG02LcCEBY6jT y51uT8bg5ZLA== X-IronPort-AV: E=McAfee;i="6000,8403,9948"; a="193761181" X-IronPort-AV: E=Sophos;i="5.82,209,1613462400"; d="scan'208";a="193761181" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Apr 2021 01:50:25 -0700 IronPort-SDR: 2nVKxe0lWGZr4gIsqpDas1QZRO3N/XmeLyWI1dgidfpFhm0vVZAym3tcAZ4cB7jhA/kAIuGcOS 0oU0VWjOTplQ== X-IronPort-AV: E=Sophos;i="5.82,209,1613462400"; d="scan'208";a="422644512" Received: from yhuang6-desk1.sh.intel.com (HELO yhuang6-desk1.ccr.corp.intel.com) ([10.239.13.1]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Apr 2021 01:50:22 -0700 From: "Huang, Ying" To: Miaohe Lin Cc: , , , , , , , , , , , , Subject: Re: [PATCH 4/5] mm/swap_state: fix potential faulted in race in swap_ra_info() References: <20210408130820.48233-1-linmiaohe@huawei.com> <20210408130820.48233-5-linmiaohe@huawei.com> Date: Fri, 09 Apr 2021 16:50:18 +0800 In-Reply-To: <20210408130820.48233-5-linmiaohe@huawei.com> (Miaohe Lin's message of "Thu, 8 Apr 2021 09:08:19 -0400") Message-ID: <874kgfyh85.fsf@yhuang6-desk1.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-Server: rspam05 X-Rspamd-Queue-Id: 7A48440002CE X-Stat-Signature: gxxfkctzbzgqa1ee67w8kyh7aapftj5p Received-SPF: none (intel.com>: No applicable sender policy available) receiver=imf17; identity=mailfrom; envelope-from=""; helo=mga03.intel.com; client-ip=134.134.136.65 X-HE-DKIM-Result: none/none X-HE-Tag: 1617958225-706820 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: Miaohe Lin writes: > While we released the pte lock, somebody else might faulted in this pte. > So we should check whether it's swap pte first to guard against such race > or swp_type would be unexpected. And we can also avoid some unnecessary > readahead cpu cycles possibly. > > Fixes: ec560175c0b6 ("mm, swap: VMA based swap readahead") > Signed-off-by: Miaohe Lin > --- > mm/swap_state.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/mm/swap_state.c b/mm/swap_state.c > index 709c260d644a..3bf0d0c297bc 100644 > --- a/mm/swap_state.c > +++ b/mm/swap_state.c > @@ -724,10 +724,10 @@ static void swap_ra_info(struct vm_fault *vmf, > { > struct vm_area_struct *vma = vmf->vma; > unsigned long ra_val; > - swp_entry_t entry; > + swp_entry_t swap_entry; > unsigned long faddr, pfn, fpfn; > unsigned long start, end; > - pte_t *pte, *orig_pte; > + pte_t *pte, *orig_pte, entry; > unsigned int max_win, hits, prev_win, win, left; > #ifndef CONFIG_64BIT > pte_t *tpte; > @@ -742,8 +742,13 @@ static void swap_ra_info(struct vm_fault *vmf, > > faddr = vmf->address; > orig_pte = pte = pte_offset_map(vmf->pmd, faddr); > - entry = pte_to_swp_entry(*pte); > - if ((unlikely(non_swap_entry(entry)))) { > + entry = *pte; > + if (unlikely(!is_swap_pte(entry))) { > + pte_unmap(orig_pte); > + return; > + } > + swap_entry = pte_to_swp_entry(entry); > + if ((unlikely(non_swap_entry(swap_entry)))) { > pte_unmap(orig_pte); > return; > } This isn't a real issue. entry or swap_entry isn't used in this function. And we have enough checking when we really operate the PTE entries later. But I admit it's confusing. So I suggest to just remove the checking. We will check it when necessary. Best Regards, Huang, Ying