Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <f4dba5b5-2e6e-4c5a-a269-4abe8fe2bcd8@intel.com>
Date:   Thu, 23 Nov 2023 09:09:38 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH] mm: filemap: avoid unnecessary major faults in
 filemap_fault()
Content-Language: en-US
To:     Peng Zhang <zhangpeng362@huawei.com>, <linux-mm@kvack.org>,
        <linux-kernel@vger.kernel.org>
CC:     <akpm@linux-foundation.org>, <willy@infradead.org>,
        <ying.huang@intel.com>, <aneesh.kumar@linux.ibm.com>,
        <shy828301@gmail.com>, <hughd@google.com>, <david@redhat.com>,
        <wangkefeng.wang@huawei.com>, <sunnanyong@huawei.com>
References: <20231122140052.4092083-1-zhangpeng362@huawei.com>
From:   Yin Fengwei <fengwei.yin@intel.com>
In-Reply-To: <20231122140052.4092083-1-zhangpeng362@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?T0Vibm9EeU9LVmlWdzYvSGRoR3R1WkdkMWpJMVEwaUVlWDN1U0QzZzM3RmlY?=
 =?utf-8?B?d3A3UStLYWd6ZHdnK1MyT2FLbEtHT0d1UWM2eFBJaUFnZGtrME4zcVk5VlVW?=
 =?utf-8?B?ckEzbjAvMVVLUnVvZXJlcng1TXZSYVpqRSttVG56REQ2ZmVwT21RUmxGVXJz?=
 =?utf-8?B?U2FyTDAwY3RvcXZGUWRjR1E1N2V0MnBpNGp1M1dvRkswMHJvcWs4ZEFKQ1k3?=
 =?utf-8?B?NjdLeTRGbU8vcjZHWGhkd2wwTTJjWHl2VTlKM0ptRmNEWnA2Vis2VWpwazhC?=
 =?utf-8?B?ZzduZFNiSVZGaGxGZFhXeEd0VFBrbHZWTlgydkhrNitsUEsycTgrckc5ZWZw?=
 =?utf-8?B?OXNDNURmWVFXbjNzakR3REZpSCtEeGZNZzljK1puVDZWVkZhTzRTSjhsTGNx?=
 =?utf-8?B?TEdIRjhnUllMNnlMMGIxR0lSRmJIaElaUjJoTEg0UHBUclpJZHJBSDFBSGFx?=
 =?utf-8?B?SnBGVERNbUxTNTJNNytkeWF0VXU1QmxKV3o5TnpjUDV0K3YrMk9Rem4rYVVG?=
 =?utf-8?B?WVk3LzNHN2FYWnRyMFlBdFRlUWU1NDFodUl2NjY3NEVub0l0aUtUQ25QOW1F?=
 =?utf-8?B?N2RCNnBjdzZROEQrWmxpVHNtSjF2dGF3TXJVS3pwTUhLM2pMN1p5VjVmZHhX?=
 =?utf-8?B?NGpDdk8rQXNFSmFKZmpTR1hDZ1dLWUptZnN6a1ROSnhMRE1DTXMrWGdYdTZm?=
 =?utf-8?B?U3VtUnhxa0lkdVhseFdzbktQcmYvaDVGWFRoTHY4M3haeXl2N2RRNVUxMU1Y?=
 =?utf-8?B?UXl0M0FtWFA3SVRjUTZqNTBNRU1OTjErTm1JZGVLYWVuaFRnMXI1K0liSkow?=
 =?utf-8?B?Ukt4RTlyaFZOd3RoRDVrQkphYmltckRXcG9GUitySlZ5c2h0N1A0Uzh0YjAx?=
 =?utf-8?B?RHFtb2YxaDJzUEVISWRCc25Xazl0NkFEeldCYUdmYzloa3hMOXczc2NyU0I0?=
 =?utf-8?B?ZW4weFpDUVhDU3JkVzRSSWI2N0ZjTEMwRmRndGcvSUhKLy9rMEpiTFQzMWZz?=
 =?utf-8?B?eHJNb1JGWDNWaFRjcXdkWTBXN3lhZHdHR1JmL2VQVExYK0lNMnJCMDQ3UkFT?=
 =?utf-8?B?OXFzTnd4Y0VzWVJEcEpLZTlJVmprVW1vTjlBYm5zNER3eUVQMkhGWktRa1BB?=
 =?utf-8?B?K2pNVVpNQkZuRm82VFVQNnpvNzhWMGRPR25TQzhQNVFrQk5uVHF5MWZYaUdu?=
 =?utf-8?B?TEhmTnR3N3R4NStWWmVLbWxPaFdLeC9nTm90RGFxVTl6aFNlcldlQ3NITENM?=
 =?utf-8?B?NUVLVnhSWGxhMnRvVmE3V0pYVWZsZFA0Q2NVOTlCOXVUbTJjaGJXNnFaM05Z?=
 =?utf-8?B?WnVGY1g4Zlg4N1YzZW5TWVdWeU1Jbkp1QkxKaVFpeHdxR2dTcUNWVzdwazI2?=
 =?utf-8?B?RHRZZ2p3blFpOEUrUkZwb3FXNGh4ZmkvNnZwMVlOaUhYcnpoUy9DbjNJajlJ?=
 =?utf-8?B?QXY5bnh1c2UvNTlFMXVzZ0FmN0lpMzRjVHZ3R1dJNW5GYlZpTy85Y2VJRWoy?=
 =?utf-8?B?dEJISFpBRWI5cjdYbWVlVFpiL1N4eVlmY2FhYVlpUzFZeW5VWkV2ZzZtQ0NR?=
 =?utf-8?B?TmdIcm5kdzB0bXMrSFNPZjZvMCtIcnNyMHo1TDhUTkx3b29KakJoMXMwSUUz?=
 =?utf-8?B?YkVZY242T3FCQWFsdzBYMVpnWS9LOS95RFgzcExsQURkaDdtc2paZ2dwelVk?=
 =?utf-8?B?R2FhWGNnbThlMGthdzFWaHM4dDZLN3BENytxZFJ0V1FCSWVmcXpsVTRXMndl?=
 =?utf-8?B?UTVGeENjSUwxOW5iREYyazJjVTVTS0tINmZBbFFTQ2l1eXY2L0R3Yjd1UkNm?=
 =?utf-8?B?cnRzRDRFcnRWT2ErdkZ1RkFHSjMwOFhjU2ZMcnFRV1YwWCtTZFhoLzM5Vjg3?=
 =?utf-8?B?ZWRUM2xpcUwvS2h4QjIyRGRvMzBCT1k5SHZ0bG9iazBCaDNYYjNEK21GcmhM?=
 =?utf-8?B?Szg0bU0xRk9NWGdGYURpYU5meTgzTkhGRkIwczhRMFZqU3lqcHU1N0FmdDZV?=
 =?utf-8?B?WndhVkY2VTBvRXU2VFZkQy9lSGlxRnVsNkRxL2FocmRDeU5ORmJNMVBMTmht?=
 =?utf-8?B?UFFBWllBVU9pekpQSDVuSHlzVU5OYzBTcTFtRGxkZUx3VkpRd2xsMzAvdVVZ?=
 =?utf-8?Q?xfMnBCc/zIJfD4HeDsnP4EJlI?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4031455f-ec45-4961-0570-08dbebc11600
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Nov 2023 01:11:13.6744
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: E1acK8kxGxBcrRPtVloWRDe4CYvBVbEPy/oXhZwntmFcEF1ljv0vq8K40O1Pn+L1Xi2h1YCLPTC7E35ZF+8PMw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5261
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 22 Nov 2023 17:11:52 -0800 (PST)

Hi Peng,

On 11/22/23 22:00, Peng Zhang wrote:
> From: ZhangPeng <zhangpeng362@huawei.com>
> 
> The major fault occurred when using mlockall(MCL_CURRENT | MCL_FUTURE)
> in application, which leading to an unexpected performance issue[1].
> 
> This caused by temporarily cleared pte during a read/modify/write update
> of the pte, eg, do_numa_page()/change_pte_range().
> 
> For the data segment of the user-mode program, the global variable area
> is a private mapping. After the pagecache is loaded, the private anonymous
> page is generated after the COW is triggered. Mlockall can lock COW pages
> (anonymous pages), but the original file pages cannot be locked and may
> be reclaimed. If the global variable (private anon page) is accessed when
> vmf->pte is zeroed in numa fault, a file page fault will be triggered.
> 
> At this time, the original private file page may have been reclaimed.
> If the page cache is not available at this time, a major fault will be
> triggered and the file will be read, causing additional overhead.
> 
> Fix this by rechecking the pte by holding ptl in filemap_fault() before
> triggering a major fault.
> 
> [1] https://lore.kernel.org/linux-mm/9e62fd9a-bee0-52bf-50a7-498fa17434ee@huawei.com/
> 
> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  mm/filemap.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 71f00539ac00..bb5e6a2790dc 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3226,6 +3226,20 @@ vm_fault_t filemap_fault(struct vm_fault *vmf)
>  			mapping_locked = true;
>  		}
>  	} else {
> +		pte_t *ptep = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd,
> +						  vmf->address, &vmf->ptl);
> +		if (ptep) {
> +			/*
> +			 * Recheck pte with ptl locked as the pte can be cleared
> +			 * temporarily during a read/modify/write update.
> +			 */
> +			if (unlikely(!pte_none(ptep_get(ptep))))
> +				ret = VM_FAULT_NOPAGE;
> +			pte_unmap_unlock(ptep, vmf->ptl);
> +			if (unlikely(ret))
> +				return ret;
> +		}
I am curious. Did you try not to take PTL here and just check whether PTE is not NONE?


Regards
Yin, Fengwei

> +
>  		/* No page in the page cache at all */
>  		count_vm_event(PGMAJFAULT);
>  		count_memcg_event_mm(vmf->vma->vm_mm, PGMAJFAULT);