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 915A4C6FD1F for ; Thu, 16 Mar 2023 16:39:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2755B900004; Thu, 16 Mar 2023 12:39:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 225E4900002; Thu, 16 Mar 2023 12:39:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 11FED900004; Thu, 16 Mar 2023 12:39:04 -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 02AA4900002 for ; Thu, 16 Mar 2023 12:39:04 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 73D3C121330 for ; Thu, 16 Mar 2023 16:39:03 +0000 (UTC) X-FDA: 80575320966.09.E2F7F3B Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf08.hostedemail.com (Postfix) with ESMTP id 8A33D160024 for ; Thu, 16 Mar 2023 16:39:01 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=none; spf=pass (imf08.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678984741; a=rsa-sha256; cv=none; b=sRhhl+RumU86I+cLg+oYZd6/kTY0Tnx+DeAInjAmJQq+OkHZ3MU+uoHSA7CeMyxWdQkG3w APZMsgc2fpcGryK2ki11sf7pCWIwmSNWHwkM6lmdbge3iUANg0m9wNz0GBzjYPyouMl2zc JrAuny9gOMEoAFT5jWvwnxdisPTxLLQ= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=none; spf=pass (imf08.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1678984741; 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=YaKyuo81ywzIM7ZomVV6klf8FEPI3tH8CEDPe41JbNE=; b=Z3CR9E0/rnpQB923vTWJ6yr67LWg/Lun1BEcGcC2afR0uPmaDEtRwnfMt5eoRBu5Dph1Pc OQThICUOH6IJVHJrJuYek9+Zf5x+zlni0ZegzjSxDyXaL48HwNTEodwFFBHm5FSaYeshkF 1KNCaj4duIPBhUk8NA2kziGqk7ugrrE= 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 77BF82F4; Thu, 16 Mar 2023 09:39:44 -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 E5D0A3F885; Thu, 16 Mar 2023 09:38:59 -0700 (PDT) Message-ID: <2fa5a911-8432-2fce-c6e1-de4e592219d8@arm.com> Date: Thu, 16 Mar 2023 16:38:58 +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> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Queue-Id: 8A33D160024 X-Rspamd-Server: rspam01 X-Stat-Signature: qqjf4jh99zqadizjxh7oqheqaspoyw9t X-HE-Tag: 1678984741-726858 X-HE-Meta: U2FsdGVkX1/A3RZA9UtbBj60mGgnPJNFgImcQrgNq9szMHsVy6uphyPg0FNUFSN86bTFuOnxXezeGYRqsdRS86iBY1rumfmnkFI4K5wrcR8+CrKIEyiM2UCTfriszX5kaZ1crddH2eYQiYiDZQBiAQ/hSBwGTBtidt8/f9xHbpw/7wTNpb0xwppnwWiHvXfTXOxuaE8qMF5ddpiK0mnbSY3HVFC6dxlsWE8N0fRUDAOxBGn+XVBr0pKVurPaeP56Wxe12uIxdd0841odsCy14ne7UQWV5rlbKc1nPUrtXtQmQ/B49ykJ+R4trOH3odPfzNWdeYmNVN6XVvkaQt072FIARfEcCaA1B8ucvxIKM/N2eiRAXSTBRr3bwiPgRkQmKOjoNQiZXt8tz2xHyh+BSTV9TAb1F/5QJw6W+4o/5FM+q/EOKVf2ZB3UgixrGQOz8WkxWz50ZzTJdzdhLEYlr2Q3DGvPqw5UkJmng+Pf4yhPV+7zfghXwwudNcouYYEqAicHsiDDihdQ6DC8mqUiUnTQnc+l91pkUJNMZjn2adx6RZiL/IC9+RzJkZyYmua5QOf3V6RPM5UPZ5D4qJbO8HYPGNQaju+t2uI/m63LreIqs+0J8J2UVd1IowDKGIAgFyC5pe+C4hAdzS8/gvFEbqxdgZ85vuRz0DtXiSGPhbGupTzWCtVLJOuWaQD+XVvYz11I5y0emhcAAe9QugiTtMOOuR5+Cz09JvBNJzSOacPzyGyjUqVgiZAB5LceGMYClrOldxr9jMRChwjvIfBe1Q2fQMKuNyaOhkE4D/BKs4t3ZIM5f3G2QWQV66N5DK/wnJ2r6Mlb2yQ7LVg4XMWGbKFAv9lphqgkCdtISIjloXCxyBha2SP6+csHBgCqC3MKZFG2y3x8Xo62w05EXPi3dClcLLKI1vAmLJDB6ViILQqRWEvYehU7JI7ADXuC/hQHjYbVSf1UgLKw6moxcZX AeUQsphV pNJinMedPPLWzPKVyDW9x6CKzozpQ1OTu1EFlcsyQXbWO9TEky8P6QimCRp0zGsnfw6Icfjwv4qz5CXyi0I3p2E4H87o4P8iPdSpJeTXxh8QgDMeaSJ0cH3HFTWuOHtEZnVA8JtWkYOg25qkqjKHcGfE9q0PLnXDQK6ebLlhv23fEtJU4qNno9uAxD99Pzq+0KEW6pJM0PzxaPqAImnhamUq+gUe6LhBjev9QDkZxxQh4a5PFTXycTD9MpQb2x4DCLcCX 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: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()? > > > 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); >>