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 C16ABC6FD1F for ; Thu, 16 Mar 2023 16:50:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4DC97900004; Thu, 16 Mar 2023 12:50:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 48CCB900002; Thu, 16 Mar 2023 12:50:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 37B2E900004; Thu, 16 Mar 2023 12:50:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 2C7EC900002 for ; Thu, 16 Mar 2023 12:50:52 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id F3918C0236 for ; Thu, 16 Mar 2023 16:50:51 +0000 (UTC) X-FDA: 80575350702.15.A6294A5 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf28.hostedemail.com (Postfix) with ESMTP id 0C0A4C0014 for ; Thu, 16 Mar 2023 16:50:49 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf28.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1678985450; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fp7qAnG46hpk7xfrolb/xnzi6IE6bmrC7dys9zysw0M=; b=GCbauWHvmkDh2lFi9cFhZSOsxF/V66kMRgOSphE6W0r+3a/oeg6HC9L6xDVnDrEnHJHIuz EPKjolpM452Al3WYT1qFaoDXwNAHmAW2hX2Ob9Xk/SSX7DSgwKPROswhQUlmjv3zKiXCIM ivGmjVY0qufLhfGWZpQ15nXTb07NV/0= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf28.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678985450; a=rsa-sha256; cv=none; b=g+4K7nbwMqzu21zLGuaEHL+ojNgTTd9Bbce4+l9wU9+Pm5Z0yGlUU7+bF4RtnHUSRsDu2X t/GwVfhNY++5XTEIjeP3Fyf2R3BTRI6jrsyzX0oKavl+fudnedIOXeUW6AU64lzXsYaG2L rYLLcFpTopdnlzsTrn3uoxxYF7EtgnM= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D2E4E2F4; Thu, 16 Mar 2023 09:51:32 -0700 (PDT) Received: from [10.1.30.156] (C02CF1NRLVDN.cambridge.arm.com [10.1.30.156]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4AF8B3F885; Thu, 16 Mar 2023 09:50:48 -0700 (PDT) Message-ID: <3cbda55e-a09f-7745-4634-dcbe7c81da88@arm.com> Date: Thu, 16 Mar 2023 16:50:47 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH v4 35/36] mm: Convert do_set_pte() to set_pte_range() Content-Language: en-US To: "Yin, Fengwei" , "Matthew Wilcox (Oracle)" , linux-arch@vger.kernel.org, will@kernel.org Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20230315051444.3229621-1-willy@infradead.org> <20230315051444.3229621-36-willy@infradead.org> <6dd5cdf8-400e-8378-22be-994f0ada5cc2@arm.com> <2fa5a911-8432-2fce-c6e1-de4e592219d8@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 0C0A4C0014 X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: zou4aq35rd4s85faji9g36x1mi7gms4c X-HE-Tag: 1678985449-145018 X-HE-Meta: U2FsdGVkX1/kNcqvrcjlImngK3xalMWbjsmxRp3IapIhp73yj3OV7BHCtOEVIeSggzf3ZhqUBvQ4xuLE6P1SrPLQkAi3QUMsglRzfDDlpPSnzOJatD+FhgRzxTRw829iHekSqkO9C8ynqBHcmJAnG8BEm+FzAoa9Bl3DwbPdLa9crra08o6A4CaR5L22lvzRIYF7sAlfDqpgpHK2KOdJTiBHMNLTJUQGt1VrpHOdobDB8ckGH7Bf4zddH6B1bMxFYwucLSSTfnAdC6tFAEGJuTJNWNn47lHTRGnXHew8wOZypXPRjHbQoz3ibhHbrBFnU862bOc5omNGF57fKpNxW+h4Q3idv/TAOGllj7LCPcd9h7h02L7MG+DgEoBADDSSqX0VW27t9cWHK0VxSB8bQjijNiQPndESOU/IuL+1idVWvSGQMP78JnQXXNl7yeBwdQYeLbwcfewnYzLg9GFMHTF5fL7j3R9IpDJsKDLn4z1PFGxpH+sYczKuDOnQQqhrRySPwAmk5cT7g2zPMov5zRInmgzFxnsRYQRkehLabWLK9OA/byWrCW2oYjY8BfKMzQd2u7CnBMumxaaYZjga1M+48fD0IEQh+MrCsrwK7JASdclisEKkzxEb3fnhY+nQS2hmkcLv74r0yGewlpV/PoJLqzd9iSzxLmnct7Sfd3aO5NDKLPw+zo8zZVrbE1sd7c/ClxwCvex1PXKjMH+e8lmWGuEks1QRzV0uLAiwZwyh/CpBQ3IvqUiB9dgmOP7+GwhWhUBpbB0f47yZoCGN9AJzzebWw3sIpVV7Vac8mtut254g6JuP9fitQHyYhdAHbHoUx5Z/RWII0dBObzT1H7+3IDnlcu75cBTWnqmoEDNBN3fVNrP6tlgLBCuuR+7OkVQauH3Hwa2XobFxOSi23dNPw2t8JeNg6G+bBM9Qboo4yEhKZBvJoh9kPJBcenH2sAphW9f1HFW4+GivZxv xDdtvYj+ oI5BMK4MOqmgDsHskj20MiY289v2OoOxIL/f7QPW3EurhUkP4aj6W2z3JLlHhOL1EzkMKM6pp7sFDSUYvQUbxVa8SNPyJ1f3zqVTCmAlezEIGhubkS65hjsuQVFtr6wpQX05yES0tTciCyCqoQL4PznkYlKj+ckfgqFAv7YsNb1jVGQ/Nj8wO5WPER7ZckYG12Aut60ggjjdQLKzLC8sz/6195ZKFFYYRnOcSbqceYhN/2fK9kY/+ovL2euvsbPQwg2o7 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 16/03/2023 16:41, Yin, Fengwei wrote: > > > On 3/17/2023 12:38 AM, Ryan Roberts wrote: >> On 16/03/2023 16:23, Yin, Fengwei wrote: >>> >>> >>> On 3/15/2023 11:26 PM, Ryan Roberts wrote: >>>> On 15/03/2023 05:14, Matthew Wilcox (Oracle) wrote: >>>>> From: Yin Fengwei >>>>> >>>>> set_pte_range() allows to setup page table entries for a specific >>>>> range. It takes advantage of batched rmap update for large folio. >>>>> It now takes care of calling update_mmu_cache_range(). >>>>> >>>>> Signed-off-by: Yin Fengwei >>>>> Signed-off-by: Matthew Wilcox (Oracle) >>>>> --- >>>>> Documentation/filesystems/locking.rst | 2 +- >>>>> include/linux/mm.h | 3 ++- >>>>> mm/filemap.c | 3 +-- >>>>> mm/memory.c | 27 +++++++++++++++------------ >>>>> 4 files changed, 19 insertions(+), 16 deletions(-) >>>>> >>>>> diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst >>>>> index 7de7a7272a5e..922886fefb7f 100644 >>>>> --- a/Documentation/filesystems/locking.rst >>>>> +++ b/Documentation/filesystems/locking.rst >>>>> @@ -663,7 +663,7 @@ locked. The VM will unlock the page. >>>>> Filesystem should find and map pages associated with offsets from "start_pgoff" >>>>> till "end_pgoff". ->map_pages() is called with page table locked and must >>>>> not block. If it's not possible to reach a page without blocking, >>>>> -filesystem should skip it. Filesystem should use do_set_pte() to setup >>>>> +filesystem should skip it. Filesystem should use set_pte_range() to setup >>>>> page table entry. Pointer to entry associated with the page is passed in >>>>> "pte" field in vm_fault structure. Pointers to entries for other offsets >>>>> should be calculated relative to "pte". >>>>> diff --git a/include/linux/mm.h b/include/linux/mm.h >>>>> index ee755bb4e1c1..81788c985a8c 100644 >>>>> --- a/include/linux/mm.h >>>>> +++ b/include/linux/mm.h >>>>> @@ -1299,7 +1299,8 @@ static inline pte_t maybe_mkwrite(pte_t pte, struct vm_area_struct *vma) >>>>> } >>>>> >>>>> vm_fault_t do_set_pmd(struct vm_fault *vmf, struct page *page); >>>>> -void do_set_pte(struct vm_fault *vmf, struct page *page, unsigned long addr); >>>>> +void set_pte_range(struct vm_fault *vmf, struct folio *folio, >>>>> + struct page *page, unsigned int nr, unsigned long addr); >>>>> >>>>> vm_fault_t finish_fault(struct vm_fault *vmf); >>>>> vm_fault_t finish_mkwrite_fault(struct vm_fault *vmf); >>>>> diff --git a/mm/filemap.c b/mm/filemap.c >>>>> index 6e2b0778db45..e2317623dcbf 100644 >>>>> --- a/mm/filemap.c >>>>> +++ b/mm/filemap.c >>>>> @@ -3504,8 +3504,7 @@ static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf, >>>>> ret = VM_FAULT_NOPAGE; >>>>> >>>>> ref_count++; >>>>> - do_set_pte(vmf, page, addr); >>>>> - update_mmu_cache(vma, addr, vmf->pte); >>>>> + set_pte_range(vmf, folio, page, 1, addr); >>>>> } while (vmf->pte++, page++, addr += PAGE_SIZE, ++count < nr_pages); >>>>> >>>>> /* Restore the vmf->pte */ >>>>> diff --git a/mm/memory.c b/mm/memory.c >>>>> index 6aa21e8f3753..9a654802f104 100644 >>>>> --- a/mm/memory.c >>>>> +++ b/mm/memory.c >>>>> @@ -4274,7 +4274,8 @@ vm_fault_t do_set_pmd(struct vm_fault *vmf, struct page *page) >>>>> } >>>>> #endif >>>>> >>>>> -void do_set_pte(struct vm_fault *vmf, struct page *page, unsigned long addr) >>>>> +void set_pte_range(struct vm_fault *vmf, struct folio *folio, >>>>> + struct page *page, unsigned int nr, unsigned long addr) >>>>> { >>>>> struct vm_area_struct *vma = vmf->vma; >>>>> bool uffd_wp = vmf_orig_pte_uffd_wp(vmf); >>>>> @@ -4282,7 +4283,7 @@ void do_set_pte(struct vm_fault *vmf, struct page *page, unsigned long addr) >>>>> bool prefault = vmf->address != addr; >>>> >>>> I think you are changing behavior here - is this intentional? Previously this >>>> would be evaluated per page, now its evaluated once for the whole range. The >>>> intention below is that directly faulted pages are mapped young and prefaulted >>>> pages are mapped old. But now a whole range will be mapped the same. >>> >>> Yes. You are right here. >>> >>> Look at the prefault and cpu_has_hw_af for ARM64, it looks like we >>> can avoid to handle vmf->address == addr specially. It's OK to >>> drop prefault and change the logic here a little bit to: >>> if (arch_wants_old_prefaulted_pte()) >>> entry = pte_mkold(entry); >>> else >>> entry = pte_sw_mkyong(entry); >>> >>> It's not necessary to use pte_sw_mkyong for vmf->address == addr >>> because HW will set the ACCESS bit in page table entry. >>> >>> Add Will Deacon in case I missed something here. Thanks. >> >> I'll defer to Will's response, but not all arm HW supports HW access flag >> management. In that case it's done by SW, so I would imagine that by setting >> this to old initially, we will get a second fault to set the access bit, which >> will slow things down. I wonder if you will need to split this into (up to) 3 >> calls to set_ptes()? > If no HW access flag, arch_wants_old_prefaulted_pte() will return false. So > path will goto pte_sw_mkyong(entry). Right? Oops... yes, I agree with you - disregard my previous comment. > > > Regards > Yin, Fengwei > >> >>> >>> >>> Regards >>> Yin, Fengwei >>> >>>> >>>> Thanks, >>>> Ryan >>>> >>>>> pte_t entry; >>>>> >>>>> - flush_icache_page(vma, page); >>>>> + flush_icache_pages(vma, page, nr); >>>>> entry = mk_pte(page, vma->vm_page_prot); >>>>> >>>>> if (prefault && arch_wants_old_prefaulted_pte()) >>>>> @@ -4296,14 +4297,18 @@ void do_set_pte(struct vm_fault *vmf, struct page *page, unsigned long addr) >>>>> entry = pte_mkuffd_wp(entry); >>>>> /* copy-on-write page */ >>>>> if (write && !(vma->vm_flags & VM_SHARED)) { >>>>> - inc_mm_counter(vma->vm_mm, MM_ANONPAGES); >>>>> - page_add_new_anon_rmap(page, vma, addr); >>>>> - lru_cache_add_inactive_or_unevictable(page, vma); >>>>> + add_mm_counter(vma->vm_mm, MM_ANONPAGES, nr); >>>>> + VM_BUG_ON_FOLIO(nr != 1, folio); >>>>> + folio_add_new_anon_rmap(folio, vma, addr); >>>>> + folio_add_lru_vma(folio, vma); >>>>> } else { >>>>> - inc_mm_counter(vma->vm_mm, mm_counter_file(page)); >>>>> - page_add_file_rmap(page, vma, false); >>>>> + add_mm_counter(vma->vm_mm, mm_counter_file(page), nr); >>>>> + folio_add_file_rmap_range(folio, page, nr, vma, false); >>>>> } >>>>> - set_pte_at(vma->vm_mm, addr, vmf->pte, entry); >>>>> + set_ptes(vma->vm_mm, addr, vmf->pte, entry, nr); >>>>> + >>>>> + /* no need to invalidate: a not-present page won't be cached */ >>>>> + update_mmu_cache_range(vma, addr, vmf->pte, nr); >>>>> } >>>>> >>>>> static bool vmf_pte_changed(struct vm_fault *vmf) >>>>> @@ -4376,11 +4381,9 @@ vm_fault_t finish_fault(struct vm_fault *vmf) >>>>> >>>>> /* Re-check under ptl */ >>>>> if (likely(!vmf_pte_changed(vmf))) { >>>>> - do_set_pte(vmf, page, vmf->address); >>>>> - >>>>> - /* no need to invalidate: a not-present page won't be cached */ >>>>> - update_mmu_cache(vma, vmf->address, vmf->pte); >>>>> + struct folio *folio = page_folio(page); >>>>> >>>>> + set_pte_range(vmf, folio, page, 1, vmf->address); >>>>> ret = 0; >>>>> } else { >>>>> update_mmu_tlb(vma, vmf->address, vmf->pte); >>>> >>