2024-06-07 21:14:27

by Barry Song

[permalink] [raw]
Subject: [PATCH v2 0/2] mm: introduce pmd|pte_needs_soft_dirty_wp helpers and utilize them

From: Barry Song <[email protected]>

This patchset introduces the pte_need_soft_dirty_wp and pmd_need_soft_dirty_wp
helpers to determine if write protection is required for softdirty tracking.
These helpers enhance code readability and improve the overall appearance.

They are then utilized in gup, mprotect, swap, and other related functions.

-v2:
* rename "need" to "needs" per David;
* separate the change of do_swap_page() per david;

Thanks to David for his original suggestions on this[1].
[1] https://lore.kernel.org/linux-mm/[email protected]/

Barry Song (2):
mm: introduce pmd|pte_needs_soft_dirty_wp helpers for softdirty
write-protect
mm: set pte writable while pmd_soft_dirty() is true in do_swap_page()

mm/gup.c | 4 ++--
mm/huge_memory.c | 2 +-
mm/internal.h | 10 ++++++++++
mm/memory.c | 2 +-
mm/mprotect.c | 2 +-
5 files changed, 15 insertions(+), 5 deletions(-)

--
2.34.1



2024-06-07 21:14:38

by Barry Song

[permalink] [raw]
Subject: [PATCH v2 1/2] mm: introduce pmd|pte_needs_soft_dirty_wp helpers for softdirty write-protect

From: Barry Song <[email protected]>

This patch introduces the pte_needs_soft_dirty_wp and
pmd_needs_soft_dirty_wp helpers to determine if write protection is
required for softdirty tracking. This can enhance code readability
and improve its overall appearance.
These new helpers are then utilized in gup, huge_memory, and mprotect.

Suggested-by: David Hildenbrand <[email protected]>
Signed-off-by: Barry Song <[email protected]>
---
mm/gup.c | 4 ++--
mm/huge_memory.c | 2 +-
mm/internal.h | 10 ++++++++++
mm/mprotect.c | 2 +-
4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index 83e279731d1b..f15756a3646a 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -820,7 +820,7 @@ static inline bool can_follow_write_pmd(pmd_t pmd, struct page *page,
return false;

/* ... and a write-fault isn't required for other reasons. */
- if (vma_soft_dirty_enabled(vma) && !pmd_soft_dirty(pmd))
+ if (pmd_needs_soft_dirty_wp(vma, pmd))
return false;
return !userfaultfd_huge_pmd_wp(vma, pmd);
}
@@ -941,7 +941,7 @@ static inline bool can_follow_write_pte(pte_t pte, struct page *page,
return false;

/* ... and a write-fault isn't required for other reasons. */
- if (vma_soft_dirty_enabled(vma) && !pte_soft_dirty(pte))
+ if (pte_needs_soft_dirty_wp(vma, pte))
return false;
return !userfaultfd_pte_wp(vma, pte);
}
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 3fbcd77f5957..b29daea9c317 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1625,7 +1625,7 @@ static inline bool can_change_pmd_writable(struct vm_area_struct *vma,
return false;

/* Do we need write faults for softdirty tracking? */
- if (vma_soft_dirty_enabled(vma) && !pmd_soft_dirty(pmd))
+ if (pmd_needs_soft_dirty_wp(vma, pmd))
return false;

/* Do we need write faults for uffd-wp tracking? */
diff --git a/mm/internal.h b/mm/internal.h
index 12e95fdf61e9..b7853fd57d9a 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1348,6 +1348,16 @@ static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma)
return !(vma->vm_flags & VM_SOFTDIRTY);
}

+static inline bool pmd_needs_soft_dirty_wp(struct vm_area_struct *vma, pmd_t pmd)
+{
+ return vma_soft_dirty_enabled(vma) && !pmd_soft_dirty(pmd);
+}
+
+static inline bool pte_needs_soft_dirty_wp(struct vm_area_struct *vma, pte_t pte)
+{
+ return vma_soft_dirty_enabled(vma) && !pte_soft_dirty(pte);
+}
+
static inline void vma_iter_config(struct vma_iterator *vmi,
unsigned long index, unsigned long last)
{
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 888ef66468db..222ab434da54 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -53,7 +53,7 @@ bool can_change_pte_writable(struct vm_area_struct *vma, unsigned long addr,
return false;

/* Do we need write faults for softdirty tracking? */
- if (vma_soft_dirty_enabled(vma) && !pte_soft_dirty(pte))
+ if (pte_needs_soft_dirty_wp(vma, pte))
return false;

/* Do we need write faults for uffd-wp tracking? */
--
2.34.1


2024-06-07 21:14:51

by Barry Song

[permalink] [raw]
Subject: [PATCH v2 2/2] mm: set pte writable while pte_soft_dirty() is true in do_swap_page()

From: Barry Song <[email protected]>

This patch leverages the new pte_needs_soft_dirty_wp() helper to optimize
a scenario where softdirty is enabled, but the softdirty flag has already
been set in do_swap_page(). In this situation, we can use pte_mkwrite
instead of applying write-protection since we don't depend on write
faults.

Suggested-by: David Hildenbrand <[email protected]>
Signed-off-by: Barry Song <[email protected]>
---
mm/memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index db9130488231..a063e489446d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -4322,7 +4322,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
if (!folio_test_ksm(folio) &&
(exclusive || folio_ref_count(folio) == 1)) {
if ((vma->vm_flags & VM_WRITE) && !userfaultfd_pte_wp(vma, pte) &&
- !vma_soft_dirty_enabled(vma)) {
+ !pte_needs_soft_dirty_wp(vma, pte)) {
pte = pte_mkwrite(pte, vma);
if (vmf->flags & FAULT_FLAG_WRITE) {
pte = pte_mkdirty(pte);
--
2.34.1


2024-06-10 08:31:06

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] mm: introduce pmd|pte_needs_soft_dirty_wp helpers for softdirty write-protect

On 07.06.24 23:13, Barry Song wrote:
> From: Barry Song <[email protected]>
>
> This patch introduces the pte_needs_soft_dirty_wp and
> pmd_needs_soft_dirty_wp helpers to determine if write protection is
> required for softdirty tracking. This can enhance code readability
> and improve its overall appearance.
> These new helpers are then utilized in gup, huge_memory, and mprotect.
>
> Suggested-by: David Hildenbrand <[email protected]>
> Signed-off-by: Barry Song <[email protected]>
> ---

Acked-by: David Hildenbrand <[email protected]>

--
Cheers,

David / dhildenb


2024-06-10 08:33:12

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] mm: set pte writable while pte_soft_dirty() is true in do_swap_page()

On 07.06.24 23:13, Barry Song wrote:
> From: Barry Song <[email protected]>
>
> This patch leverages the new pte_needs_soft_dirty_wp() helper to optimize
> a scenario where softdirty is enabled, but the softdirty flag has already
> been set in do_swap_page(). In this situation, we can use pte_mkwrite
> instead of applying write-protection since we don't depend on write
> faults.
>
> Suggested-by: David Hildenbrand <[email protected]>
> Signed-off-by: Barry Song <[email protected]>
> ---
> mm/memory.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index db9130488231..a063e489446d 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4322,7 +4322,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> if (!folio_test_ksm(folio) &&
> (exclusive || folio_ref_count(folio) == 1)) {
> if ((vma->vm_flags & VM_WRITE) && !userfaultfd_pte_wp(vma, pte) &&
> - !vma_soft_dirty_enabled(vma)) {
> + !pte_needs_soft_dirty_wp(vma, pte)) {
> pte = pte_mkwrite(pte, vma);
> if (vmf->flags & FAULT_FLAG_WRITE) {
> pte = pte_mkdirty(pte);

Acked-by: David Hildenbrand <[email protected]>

--
Cheers,

David / dhildenb