| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <edf768ec-e874-4ca6-9fd7-b94ccc1c1059@linux.dev>
Date: Mon, 28 Apr 2025 20:50:11 +0800
From: Lance Yang <lance.yang@...ux.dev>
To: Dev Jain <dev.jain@....com>, akpm@...ux-foundation.org
Cc: ryan.roberts@....com, david@...hat.com, willy@...radead.org,
linux-mm@...ck.org, linux-kernel@...r.kernel.org, catalin.marinas@....com,
will@...nel.org, Liam.Howlett@...cle.com, lorenzo.stoakes@...cle.com,
vbabka@...e.cz, jannh@...gle.com, anshuman.khandual@....com,
peterx@...hat.com, joey.gouly@....com, ioworker0@...il.com,
baohua@...nel.org, kevin.brodsky@....com, quic_zhenhuah@...cinc.com,
christophe.leroy@...roup.eu, yangyicong@...ilicon.com,
linux-arm-kernel@...ts.infradead.org, namit@...are.com, hughd@...gle.com,
yang@...amperecomputing.com, ziy@...dia.com
Subject: Re: [PATCH 6/7] mm: Batch around can_change_pte_writable()
Hey Dev,
On 2025/4/28 20:04, Dev Jain wrote:
> In preparation for patch 7, we need to properly batch around
> can_change_pte_writable(). We batch around pte_needs_soft_dirty_wp() by
> the corresponding fpb flag, we batch around the page-anon exclusive check
> using folio_maybe_mapped_shared(); modify_prot_start_ptes() collects the
> dirty and access bits across the batch, therefore batching across
> pte_dirty(): this is correct since the dirty bit on the PTE really
> is just an indication that the folio got written to, so even if
> the PTE is not actually dirty (but one of the PTEs in the batch is),
> the wp-fault optimization can be made.
>
> Signed-off-by: Dev Jain <dev.jain@....com>
> ---
> include/linux/mm.h | 4 ++--
> mm/gup.c | 2 +-
> mm/huge_memory.c | 4 ++--
> mm/memory.c | 6 +++---
> mm/mprotect.c | 9 ++++++---
> 5 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 5eb0d77c4438..ffa02e15863f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2710,8 +2710,8 @@ int get_cmdline(struct task_struct *task, char *buffer, int buflen);
> #define MM_CP_UFFD_WP_ALL (MM_CP_UFFD_WP | \
> MM_CP_UFFD_WP_RESOLVE)
>
> -bool can_change_pte_writable(struct vm_area_struct *vma, unsigned long addr,
> - pte_t pte);
> +bool can_change_ptes_writable(struct vm_area_struct *vma, unsigned long addr,
> + pte_t pte, struct folio *folio, unsigned int nr);
> extern long change_protection(struct mmu_gather *tlb,
> struct vm_area_struct *vma, unsigned long start,
> unsigned long end, unsigned long cp_flags);
> diff --git a/mm/gup.c b/mm/gup.c
> index 84461d384ae2..6a605fc5f2cb 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -614,7 +614,7 @@ static inline bool can_follow_write_common(struct page *page,
> return false;
>
> /*
> - * See can_change_pte_writable(): we broke COW and could map the page
> + * See can_change_ptes_writable(): we broke COW and could map the page
> * writable if we have an exclusive anonymous page ...
> */
> return page && PageAnon(page) && PageAnonExclusive(page);
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 28c87e0e036f..e5496c0d9e7e 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2032,12 +2032,12 @@ static inline bool can_change_pmd_writable(struct vm_area_struct *vma,
> return false;
>
> if (!(vma->vm_flags & VM_SHARED)) {
> - /* See can_change_pte_writable(). */
> + /* See can_change_ptes_writable(). */
> page = vm_normal_page_pmd(vma, addr, pmd);
> return page && PageAnon(page) && PageAnonExclusive(page);
> }
>
> - /* See can_change_pte_writable(). */
> + /* See can_change_ptes_writable(). */
> return pmd_dirty(pmd);
> }
>
> diff --git a/mm/memory.c b/mm/memory.c
> index b9e8443aaa86..b1fda3de8d27 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -750,7 +750,7 @@ static void restore_exclusive_pte(struct vm_area_struct *vma,
> pte = pte_mkuffd_wp(pte);
>
> if ((vma->vm_flags & VM_WRITE) &&
> - can_change_pte_writable(vma, address, pte)) {
> + can_change_ptes_writable(vma, address, pte, NULL, 1)) {
> if (folio_test_dirty(folio))
> pte = pte_mkdirty(pte);
> pte = pte_mkwrite(pte, vma);
> @@ -5767,7 +5767,7 @@ static void numa_rebuild_large_mapping(struct vm_fault *vmf, struct vm_area_stru
> ptent = pte_modify(ptent, vma->vm_page_prot);
> writable = pte_write(ptent);
> if (!writable && pte_write_upgrade &&
> - can_change_pte_writable(vma, addr, ptent))
> + can_change_ptes_writable(vma, addr, ptent, NULL, 1))
> writable = true;
> }
>
> @@ -5808,7 +5808,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf)
> */
> writable = pte_write(pte);
> if (!writable && pte_write_upgrade &&
> - can_change_pte_writable(vma, vmf->address, pte))
> + can_change_ptes_writable(vma, vmf->address, pte, NULL, 1))
> writable = true;
>
> folio = vm_normal_folio(vma, vmf->address, pte);
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 33eabc995584..362fd7e5457d 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -40,8 +40,8 @@
>
> #include "internal.h"
>
> -bool can_change_pte_writable(struct vm_area_struct *vma, unsigned long addr,
> - pte_t pte)
> +bool can_change_ptes_writable(struct vm_area_struct *vma, unsigned long addr,
> + pte_t pte, struct folio *folio, unsigned int nr)
> {
> struct page *page;
>
> @@ -67,6 +67,9 @@ bool can_change_pte_writable(struct vm_area_struct *vma, unsigned long addr,
> * write-fault handler similarly would map them writable without
> * any additional checks while holding the PT lock.
> */
> + if (unlikely(nr != 1))
> + return !folio_maybe_mapped_shared(folio);
> +
> page = vm_normal_page(vma, addr, pte);
> return page && PageAnon(page) && PageAnonExclusive(page);
> }
IIUC, As mentioned in the comment above, we should do the same anonymous
check
to large folios. And folio_maybe_mapped_shared() already handles both
order-0
and large folios nicely, so we could simplify the logic as follows:
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 1605e89349d2..df56a30bb241 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -43,8 +43,6 @@
bool can_change_ptes_writable(struct vm_area_struct *vma, unsigned
long addr,
pte_t pte, struct folio *folio, unsigned
int nr)
{
- struct page *page;
-
if (WARN_ON_ONCE(!(vma->vm_flags & VM_WRITE)))
return false;
@@ -67,11 +65,7 @@ bool can_change_ptes_writable(struct vm_area_struct
*vma, unsigned long addr,
* write-fault handler similarly would map them
writable without
* any additional checks while holding the PT lock.
*/
- if (unlikely(nr != 1))
- return !folio_maybe_mapped_shared(folio);
-
- page = vm_normal_page(vma, addr, pte);
- return page && PageAnon(page) && PageAnonExclusive(page);
+ return folio_test_anon(folio) &&
!folio_maybe_mapped_shared(folio);
}
VM_WARN_ON_ONCE(is_zero_pfn(pte_pfn(pte)) && pte_dirty(pte));
--
Thanks,
Lance
> @@ -222,7 +225,7 @@ static long change_pte_range(struct mmu_gather *tlb,
> */
> if ((cp_flags & MM_CP_TRY_CHANGE_WRITABLE) &&
> !pte_write(ptent) &&
> - can_change_pte_writable(vma, addr, ptent))
> + can_change_ptes_writable(vma, addr, ptent, folio, 1))
> ptent = pte_mkwrite(ptent, vma);
>
> ptep_modify_prot_commit(vma, addr, pte, oldpte, ptent);
Powered by blists - more mailing lists