mm/rmap: Refactor try_to_unmap_one
Subsystems (3)
v3 cover mm/rmap: Refactor try_to_unmap_one
In preparation for batching anonymous large folio unmapping to optimize it, refactor try_to_unmap_one. This series refactors hugetlb, anon-lazyfree and anon-swapbacked logic into their own functions, significantly reducing the length of the huge try_to_unmap_one.
---
Split out from
https://lore.kernel.org/all/[email protected]/
mm-selftests pass on arm64, built on arm64 and x86.
v2->v3:
- Patch 2: fix comment to say that TTU_HWPOISON can be true for small folios only in try_to_unmap_one, since hugetlb bits are now refactored away
- Patch 5: Directly do if (WARN_ON_ONCE())
- Rebase on latest mm-unstable to resolve conflicts with 30c83145c5b3 ("mm: rename uffd-wp PTE accessors to uffd")
v1->v2:
- constify ttu flags in try_to_unmap_hugetlb_one, move const variables up
- Assert address passed to try_to_unmap_hugetlb_one equals pvmw.address
- Patch 3: code and comment polishing, do the stat update in the function
- Remove unnecessary inline hints, remove set_swp_pte_at, instead have swp_pte_prepare + set_ptes. Rename subpage -> page. constify anon_exclusive.
- Do pte restoration in caller
Dev Jain (5):
mm/rmap: convert page -> folio for hwpoison checks mm/rmap: Add try_to_unmap_hugetlb_one
mm/rmap: refactor some code around lazyfree folio unmapping mm/rmap: refactor anon folio unmap in try_to_unmap_one mm/rmap: add anon folio unmap dispatcher
include/linux/hugetlb.h | 1 + mm/rmap.c | 415 ++++++++++++++++++++++------------------ 2 files changed, 232 insertions(+), 184 deletions(-)
Signature · 1 line
2.43.0
PATCH v3 1/5 mm/rmap: convert page -> folio for hwpoison checks
try_to_unmap() receives hugetlb folios only from the hwpoison path. hugetlb_update_hwpoison() sets the hugetlb folio's head-page hwpoison bit, and page_vma_mapped_walk() reports the hugetlb mapping at the head PFN, so the previous PageHWPoison(subpage) check happened to work for hugetlb.
For non-hugetlb folios, unmap_poisoned_folio() currently rejects large folios before calling try_to_unmap(). Hence it is always the case that if try_to_unmap_one() handles an hwpoisoned folio, then the head page is marked with the poison bit.
Therefore, convert the poisoned subpage checks to folio_test_hwpoison().
No functional change intended, except that, while at it, convert VM_BUG_* to VM_WARN_*.
--- mm/rmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
mm/rmap.c +9 -4
@@ -2122,10 +2122,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,bool anon =folio_test_anon (folio);/* -* The try_to_unmap() is only passed a hugetlb page -* in the case where the hugetlb page is poisoned. +* The try_to_unmap() is only passed a hugetlb folio +* in the case where the hugetlb folio contains a +* poisoned page. */ -VM_BUG_ON_PAGE (!PageHWPoison (subpage), subpage); +VM_WARN_ON_FOLIO (!folio_test_hwpoison (folio), folio);/* * huge_pmd_unshare may unmap an entire PMD page. * There is no way of knowing exactly which PMDs may
@@ -2204,7 +2205,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* Update high watermark before we lower rss */ update_hiwater_rss (mm); -if (PageHWPoison (subpage) && (flags &TTU_HWPOISON )) { +/* +* With TTU_HWPOISON, we only expect small folios or hugetlb +* folios here for now. +*/ +if (folio_test_hwpoison (folio) && (flags &TTU_HWPOISON )) { pteval =swp_entry_to_pte (make_hwpoison_entry (subpage));if (folio_test_hugetlb (folio)) {hugetlb_count_sub (folio_nr_pages (folio), mm);
Signature · 1 line
2.43.0
Re: [PATCH v3 1/5] mm/rmap: convert page -> folio for hwpoison checks
Quoted 11 lines · Dev Jain
try_to_unmap()receives hugetlb folios only from the hwpoison path.hugetlb_update_hwpoison()sets the hugetlb folio's head-page hwpoison bit, andpage_vma_mapped_walk()reports the hugetlb mapping at the head PFN, so the previous PageHWPoison(subpage) check happened to work for hugetlb.For non-hugetlb folios,
unmap_poisoned_folio()currently rejects large folios before callingtry_to_unmap(). Hence it is always the case that iftry_to_unmap_one()handles an hwpoisoned folio, then the head page is marked with the poison bit.
Quoted 7 lines
Therefore, convert the poisoned subpage checks to
folio_test_hwpoison().No functional change intended, except that, while at it, convert VM_BUG_* to VM_WARN_*.
Signed-off-by Dev Jain <[email protected]>
Oops reviewed v2 by mistake. This LGTM so:
Quoted 10 lines
--- mm/rmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)mm/rmap.c +0
-0@@ -2122,10 +2122,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,bool anon =folio_test_anon (folio);
mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2129
Quoted 10 lines
mm/rmap.c +4
-3/* -* The try_to_unmap() is only passed a hugetlb page -* in the case where the hugetlb page is poisoned. +* The try_to_unmap() is only passed a hugetlb folio +* in the case where the hugetlb folio contains a +* poisoned page. */ -VM_BUG_ON_PAGE (!PageHWPoison (subpage), subpage); +VM_WARN_ON_FOLIO (!folio_test_hwpoison (folio), folio);
Thanks for converting this also!
mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2212
Quoted 13 lines
mm/rmap.c +5
-1/* * huge_pmd_unshare may unmap an entire PMD page. * There is no way of knowing exactly which PMDs may @@ -2204,7 +2205,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* Update high watermark before we lower rss */ update_hiwater_rss (mm); -if (PageHWPoison (subpage) && (flags &TTU_HWPOISON )) { +/* +* With TTU_HWPOISON, we only expect small folios or hugetlb +* folios here for now. +*/ +if (folio_test_hwpoison (folio) && (flags &TTU_HWPOISON )) {
As per v2 discussion, I feel this could be tightened up, maybe something like:
/* unmap_poisoned_folio() only refs order-0 or hugetlb folios */
Quoted 6 lines
mm/rmap.c +0
-1pteval =swp_entry_to_pte (make_hwpoison_entry (subpage));if (folio_test_hugetlb (folio)) {hugetlb_count_sub (folio_nr_pages (folio), mm); --2.43.0
Cheers, Lorenzo
Re: [PATCH v3 1/5] mm/rmap: convert page -> folio for hwpoison checks
Quoted 17 lines · Lorenzo Stoakes (ARM)
Quoted 16 lines · Dev Jain
try_to_unmap()receives hugetlb folios only from the hwpoison path.hugetlb_update_hwpoison()sets the hugetlb folio's head-page hwpoison bit, andpage_vma_mapped_walk()reports the hugetlb mapping at the head PFN, so the previous PageHWPoison(subpage) check happened to work for hugetlb.For non-hugetlb folios,
unmap_poisoned_folio()currently rejects large folios before callingtry_to_unmap(). Hence it is always the case that iftry_to_unmap_one()handles an hwpoisoned folio, then the head page is marked with the poison bit.Therefore, convert the poisoned subpage checks to
folio_test_hwpoison().No functional change intended, except that, while at it, convert VM_BUG_* to VM_WARN_*.
Quoted 7 lines
Quoted 2 lines
Signed-off-by Dev Jain <[email protected]>Oops reviewed v2 by mistake. This LGTM so:
Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>
Thanks for your review.
Quoted 39 lines
Quoted 10 lines
--- mm/rmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)mm/rmap.c +0
-0@@ -2122,10 +2122,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,bool anon =folio_test_anon (folio);mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2129
Quoted 10 lines
mm/rmap.c +4
-3/* -* The try_to_unmap() is only passed a hugetlb page -* in the case where the hugetlb page is poisoned. +* The try_to_unmap() is only passed a hugetlb folio +* in the case where the hugetlb folio contains a +* poisoned page. */ -VM_BUG_ON_PAGE (!PageHWPoison (subpage), subpage); +VM_WARN_ON_FOLIO (!folio_test_hwpoison (folio), folio);Thanks for converting this also!
mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2212
Quoted 13 lines
mm/rmap.c +5
-1/* * huge_pmd_unshare may unmap an entire PMD page. * There is no way of knowing exactly which PMDs may @@ -2204,7 +2205,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* Update high watermark before we lower rss */ update_hiwater_rss (mm); -if (PageHWPoison (subpage) && (flags &TTU_HWPOISON )) { +/* +* With TTU_HWPOISON, we only expect small folios or hugetlb +* folios here for now. +*/ +if (folio_test_hwpoison (folio) && (flags &TTU_HWPOISON )) {As per v2 discussion, I feel this could be tightened up, maybe something like:
Quoted 2 lines
/* unmap_poisoned_folio() only refs order-0 or hugetlb folios */
I can do this. But note that this series is in mm-unstable already - what should I base the v4 on?
Quoted 8 lines
Quoted 6 lines
mm/rmap.c +0
-1pteval =swp_entry_to_pte (make_hwpoison_entry (subpage));if (folio_test_hugetlb (folio)) {hugetlb_count_sub (folio_nr_pages (folio), mm); --2.43.0
Cheers, Lorenzo
Andrew Morton
maintainerRe: [PATCH v3 1/5] mm/rmap: convert page -> folio for hwpoison checks
Quoted 13 lines · On Sat, 25 Jul 2026 13:06:25 +0530 Dev Jain <[email protected]> wrote:
Quoted 13 lines
mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2212
Quoted 10 lines
mm/rmap.c +5
-1@@ -2204,7 +2205,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* Update high watermark before we lower rss */ update_hiwater_rss (mm); -if (PageHWPoison (subpage) && (flags &TTU_HWPOISON )) { +/* +* With TTU_HWPOISON, we only expect small folios or hugetlb +* folios here for now. +*/ +if (folio_test_hwpoison (folio) && (flags &TTU_HWPOISON )) {As per v2 discussion, I feel this could be tightened up, maybe something like:
Quoted 5 lines
Quoted 2 lines
/* unmap_poisoned_folio() only refs order-0 or hugetlb folios */I can do this. But note that this series is in mm-unstable already - what should I base the v4 on?
Ordinarily basing on latest Linus works. But this patchset has ~450 patches ahead of it and a quick trial merge wasn't promising.
I could drop the v3 series so you can base on next mm-new, please lmk.
Or
mm> git log --oneline | grep "mm/rmap: convert page -> folio for hwpoison checks" bb2f8a7eb126 mm/rmap: convert page -> folio for hwpoison checks
mm> git checkout bb2f8a7eb126^
Re: [PATCH v3 1/5] mm/rmap: convert page -> folio for hwpoison checks
Quoted 24 lines · Andrew Morton
Quoted 13 lines · On Sat, 25 Jul 2026 13:06:25 +0530 Dev Jain <[email protected]> wrote:
Quoted 13 lines
mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2212
Quoted 10 lines
mm/rmap.c +5
-1@@ -2204,7 +2205,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* Update high watermark before we lower rss */ update_hiwater_rss (mm); -if (PageHWPoison (subpage) && (flags &TTU_HWPOISON )) { +/* +* With TTU_HWPOISON, we only expect small folios or hugetlb +* folios here for now. +*/ +if (folio_test_hwpoison (folio) && (flags &TTU_HWPOISON )) {As per v2 discussion, I feel this could be tightened up, maybe something like:
Quoted 5 lines
Quoted 2 lines
/* unmap_poisoned_folio() only refs order-0 or hugetlb folios */I can do this. But note that this series is in mm-unstable already - what should I base the v4 on?
Ordinarily basing on latest Linus works. But this patchset has ~450 patches ahead of it and a quick trial merge wasn't promising.
Quoted 7 lines
I could drop the v3 series so you can base on next mm-new, please lmk.
Or
mm> git log --oneline | grep "mm/rmap: convert page -> folio for hwpoison checks"
bb2f8a7eb126mm/rmap: convert page -> folio for hwpoison checks
mm> git checkout bb2f8a7eb126^
This one looks good, I'll do this.
I'll wait for Lorenzo's (or any others) potential review comments for two days before I send this.
PATCH v3 2/5 mm/rmap: Add try_to_unmap_hugetlb_one
In reply to Dev JainSimplify try_to_unmap_one() by separating the hugetlb parts into try_to_unmap_hugetlb_one().
To understand the correctness of the refactoring, the following points are noted:
1. try_to_unmap() is called for hugetlb folios only when they are hwpoisoned.
2. A hugetlb VMA cannot be mlocked.
3. page_vma_mapped_walk() returns at most one hugetlb mapping in a VMA, and that mapping points at the head PFN.
4. We won't ever process a softleaf entry that encodes a hugetlb folio; hugetlb folios are never swapped out, migration entries will be skipped (PVMW_MIGRATION not passed), and device-exclusive does not work for hugetlb.
5. The hwpoison entry is constructed from the poisoned folio, just as in the pre-refactor code. Any previous uffd-wp state is deliberately not preserved for the hwpoison entry.
6. TTU_HWPOISON is always present; for it to not be present, either the folio has to be in swapcache, or mapping_can_writeback() is true (see unmap_poisoned_folio), none of which is true for hugetlb folios.
7. Hugetlb uses separate counters from normal rss counters, therefore update_highwater_rss() need not be called.
While at it:
- Change VM_BUG_* to VM_WARN_*.
- Do not declare variables which are only used once.
- Constify some variables.
- Add some more VM_WARN_* to assert some invariants.
Except the above 4 points, no functional change intended.
--- include/linux/hugetlb.h | 1 + mm/rmap.c | 183 +++++++++++++++++++++------------------- 2 files changed, 98 insertions(+), 86 deletions(-)
include/linux/hugetlb.h +1 -0
@@ -1271,6 +1271,7 @@ static inline void hugetlb_count_sub(long l, struct mm_struct *mm) }pte_t huge_ptep_get (struct mm_struct *mm,unsigned long addr,pte_t *ptep); +unsigned long huge_pte_dirty (pte_t pte);static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep)
mm/rmap.c +97 -86
239 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v3 2/5] mm/rmap: Add try_to_unmap_hugetlb_one
Quoted 2 lines · Dev Jain
Simplify
try_to_unmap_one()by separating the hugetlb parts intotry_to_unmap_hugetlb_one().
I hate that we have this separate hugetlb stuff but while we have it, better to be explicit :)
Quoted 21 lines
To understand the correctness of the refactoring, the following points are noted:
1.
try_to_unmap()is called for hugetlb folios only when they are hwpoisoned.2. A hugetlb VMA cannot be mlocked.
3.
page_vma_mapped_walk()returns at most one hugetlb mapping in a VMA, and that mapping points at the head PFN.4. We won't ever process a softleaf entry that encodes a hugetlb folio; hugetlb folios are never swapped out, migration entries will be skipped (
PVMW_MIGRATIONnot passed), and device-exclusive does not work for hugetlb.5. The hwpoison entry is constructed from the poisoned folio, just as in the pre-refactor code. Any previous uffd-wp state is deliberately not preserved for the hwpoison entry.
Quoted 6 lines
6.
TTU_HWPOISONis always present; for it to not be present, either the folio has to be in swapcache, ormapping_can_writeback()is true (seeunmap_poisoned_folio), none of which is true for hugetlb folios.7. Hugetlb uses separate counters from normal rss counters, therefore
update_highwater_rss()need not be called.
I wonder whether you could bundle some of this up into a comment around try_to_unmap_hugetlb_one()?
Quoted 9 lines
While at it:
- Change VM_BUG_* to VM_WARN_*.
- Do not declare variables which are only used once.
- Constify some variables.
Quoted 7 lines
- Add some more VM_WARN_* to assert some invariants.
Except the above 4 points, no functional change intended.
Suggested-by David Hildenbrand (Arm) <[email protected]>Signed-off-by Dev Jain <[email protected]>
A bunch of nits but those addressed LGTM:
Thanks very much for doing this is a big improvement!
Quoted 30 lines
--- include/linux/hugetlb.h | 1 + mm/rmap.c | 183 +++++++++++++++++++++------------------- 2 files changed, 98 insertions(+), 86 deletions(-)include/linux/hugetlb.h +1
-0@@ -1271,6 +1271,7 @@ static inline void hugetlb_count_sub(long l, struct mm_struct *mm) }pte_t huge_ptep_get (struct mm_struct *mm,unsigned long addr,pte_t *ptep); +unsigned long huge_pte_dirty (pte_t pte);static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep)mm/rmap.c +5
-0@@ -1978,6 +1978,96 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio, FPB_RESPECT_WRITE | FPB_RESPECT_SOFT_DIRTY); } +static bool try_to_unmap_hugetlb_one (struct folio *folio, +struct vm_area_struct *vma,unsigned long address,void *arg) +{ +DEFINE_FOLIO_VMA_WALK (pvmw, folio, vma, address,0 ); +const unsigned long hsz =huge_page_size (hstate_vma (vma));
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 1995
Quoted 10 lines
mm/rmap.c +10
-0+const enum ttu_flags flags = (enum ttu_flags )(long )arg; +struct mm_struct *mm = vma->vm_mm; +struct mmu_notifier_range range; +bool ret = true; +pte_t pteval; + +/* +* The try_to_unmap() is only passed a hugetlb folio in the case +* where the hugetlb folio is poisoned. +*/
I wonder if the function should be try_to_unmap_poisoned_hugetlb_one() as a result?
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 1996
Quoted 1 line
mm/rmap.c +1
-0+VM_WARN_ON_FOLIO (!folio_test_hwpoison (folio), folio);
NIT: Should be VM_WARN_ON_ONCE_FOLIO() for consistency with below and to avoid repeated warnings?
Quoted 7 lines
mm/rmap.c +7
-0+VM_WARN_ON_ONCE (!(flags &TTU_HWPOISON )); + + range.end =vma_address_end (&pvmw); +mmu_notifier_range_init (&range,MMU_NOTIFY_CLEAR ,0 , vma->vm_mm, + address, range.end); +adjust_range_if_pmd_sharing_possible (vma, &range.start, &range.end); +mmu_notifier_invalidate_range_start (&range);
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 2013
Quoted 10 lines
mm/rmap.c +10
-0+ +/* There is only a single mapping in a VMA. */ +if (!page_vma_mapped_walk (&pvmw)) + goto range_end; + +VM_WARN_ON_ONCE (address != pvmw.address); + + pteval =huge_ptep_get (mm, address, pvmw.pte); +VM_WARN_ON_ONCE (!pte_present (pteval)); +VM_WARN_ON_ONCE (pte_pfn (pteval) !=folio_pfn (folio));
I guess no TTU_SYNC is possible for hugetlb poison unmap?
Quoted 11 lines
mm/rmap.c +11
-0+ +/* +* huge_pmd_unshare may unmap an entire PMD page. There is no way of +* knowing exactly which PMDs may be cached for this mm, so we must +* flush them all. start/end were already adjusted above to cover this +* range. +*/ +flush_cache_range (vma, range.start, range.end); + +/* +* To call huge_pmd_unshare, i_mmap_rwsem must be held in write mode.
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 2034
Quoted 10 lines
mm/rmap.c +10
-0+* Caller needs to explicitly do this outside rmap routines. +* +* We also must hold hugetlb vma_lock in write mode. Lock order dictates +* acquiring vma_lock BEFORE i_mmap_rwsem. We can only try lock here and +* fail if unsuccessful. +*/ +if (!folio_test_anon (folio)) { +struct mmu_gather tlb; + +VM_WARN_ON (!(flags &TTU_RMAP_LOCKED ));
VM_WARN_ON_ONCE()?
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 2035
Quoted 1 line
mm/rmap.c +1
-0+if (!hugetlb_vma_trylock_write (vma)) {
How I hate that hugetlb calls their lock a 'VMA lock'...
Quoted 17 lines
mm/rmap.c +17
-0+ ret = false; + goto walk_done; + } + +tlb_gather_mmu_vma (&tlb, vma); +if (huge_pmd_unshare (&tlb, vma, address, pvmw.pte)) { +hugetlb_vma_unlock_write (vma); +huge_pmd_unshare_flush (&tlb, vma); +tlb_finish_mmu (&tlb); +/* +* The PMD table was unmapped, consequently unmapping +* the folio. +*/ + goto walk_done; + } +hugetlb_vma_unlock_write (vma); +tlb_finish_mmu (&tlb);
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 2062
Quoted 10 lines
mm/rmap.c +10
-0+ } + pteval =huge_ptep_clear_flush (vma, address, pvmw.pte); +if (huge_pte_dirty (pteval)) +folio_mark_dirty (folio); + + pteval =swp_entry_to_pte (make_hwpoison_entry (folio_page (folio,0 ))); +hugetlb_count_sub (folio_nr_pages (folio), mm); +set_huge_pte_at (mm, address, pvmw.pte, pteval, hsz); +hugetlb_remove_rmap (folio); +folio_put_refs (folio,1 );
Do we want an assert here somehow that we are only walking one folio?
Quoted 115 lines
mm/rmap.c +9
-68+ +walk_done: +page_vma_mapped_walk_done (&pvmw); +range_end: +mmu_notifier_invalidate_range_end (&range); +return ret; +} +/* * @arg: enum ttu_flags will be passed to this argument */ @@ -1993,7 +2083,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,enum ttu_flags flags = (enum ttu_flags )(long )arg;unsigned long nr_pages =1 , end_addr;unsigned long pfn; -unsigned long hsz =0 ;int ptes =0 ;/* @@ -2007,8 +2096,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* * For THP, we have to assume the worse case ie pmd for invalidation. -* For hugetlb, it could be much worse if we need to do pud -* invalidation in the case of pmd sharing. * * Note that the folio can not be freed in this function as call of * try_to_unmap() must hold a reference on the folio. @@ -2016,17 +2103,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, range.end =vma_address_end (&pvmw);mmu_notifier_range_init (&range,MMU_NOTIFY_CLEAR ,0 , vma->vm_mm, address, range.end); -if (folio_test_hugetlb (folio)) { -/* -* If sharing is possible, start and end will be adjusted -* accordingly. -*/ -adjust_range_if_pmd_sharing_possible (vma, &range.start, - &range.end); - -/* We need the huge page size for set_huge_pte_at() */ - hsz =huge_page_size (hstate_vma (vma)); - }mmu_notifier_invalidate_range_start (&range);while (page_vma_mapped_walk (&pvmw)) {@@ -2111,66 +2187,13 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,const softleaf_t entry =softleaf_from_pte (pteval); pfn =softleaf_to_pfn (entry); -VM_WARN_ON_FOLIO (folio_test_hugetlb (folio), folio); } subpage =folio_page (folio, pfn -folio_pfn (folio)); anon_exclusive =folio_test_anon (folio) &&PageAnonExclusive (subpage); -if (folio_test_hugetlb (folio)) { -bool anon =folio_test_anon (folio); - -/* -* The try_to_unmap() is only passed a hugetlb folio -* in the case where the hugetlb folio contains a -* poisoned page. -*/ -VM_WARN_ON_FOLIO (!folio_test_hwpoison (folio), folio); -/* -* huge_pmd_unshare may unmap an entire PMD page. -* There is no way of knowing exactly which PMDs may -* be cached for this mm, so we must flush them all. -* start/end were already adjusted above to cover this -* range. -*/ -flush_cache_range (vma, range.start, range.end); - -/* -* To call huge_pmd_unshare, i_mmap_rwsem must be -* held in write mode. Caller needs to explicitly -* do this outside rmap routines. -* -* We also must hold hugetlb vma_lock in write mode. -* Lock order dictates acquiring vma_lock BEFORE -* i_mmap_rwsem. We can only try lock here and fail -* if unsuccessful. -*/ -if (!anon) { -struct mmu_gather tlb; - - VM_BUG_ON(!(flags & T][\TU_RMAP_LOCKED)); -if (!hugetlb_vma_trylock_write (vma)) - goto walk_abort; - -tlb_gather_mmu_vma (&tlb, vma); -if (huge_pmd_unshare (&tlb, vma, address, pvmw.pte)) { -hugetlb_vma_unlock_write (vma); -huge_pmd_unshare_flush (&tlb, vma); -tlb_finish_mmu (&tlb); -/* -* The PMD table was unmapped, -* consequently unmapping the folio. -*/ - goto walk_done; - } -hugetlb_vma_unlock_write (vma); -tlb_finish_mmu (&tlb); - } - pteval =huge_ptep_clear_flush (vma, address, pvmw.pte); -if (pte_dirty (pteval)) -folio_mark_dirty (folio); - }else if (likely (pte_present (pteval))) { +if (likely (pte_present (pteval))) { nr_pages =folio_unmap_pte_batch (folio, &pvmw, flags, pteval); end_addr = address + nr_pages *PAGE_SIZE ;flush_cache_range (vma, address, end_addr);
mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2231
Quoted 8 lines
mm/rmap.c +1
-4@@ -2205,20 +2228,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* Update high watermark before we lower rss */ update_hiwater_rss (mm); -/* -* With TTU_HWPOISON, we only expect small folios or hugetlb -* folios here for now. -*/ +/* With TTU_HWPOISON, we only expect small folios here. */
I mean you can further simplify the simplified version this way :)
Quoted 39 lines
mm/rmap.c +5
-15if (folio_test_hwpoison (folio) && (flags &TTU_HWPOISON )) { pteval =swp_entry_to_pte (make_hwpoison_entry (subpage)); -if (folio_test_hugetlb (folio)) { -hugetlb_count_sub (folio_nr_pages (folio), mm); -set_huge_pte_at (mm, address, pvmw.pte, pteval, - hsz); - }else { -dec_mm_counter (mm,mm_counter (folio)); -set_pte_at (mm, address, pvmw.pte, pteval); - } +dec_mm_counter (mm,mm_counter (folio)); +set_pte_at (mm, address, pvmw.pte, pteval); }else if (likely (pte_present (pteval)) &&pte_unused (pteval) && !userfaultfd_armed (vma)) {/* @@ -2346,11 +2360,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,add_mm_counter (mm,mm_counter_file (folio), -nr_pages); } discard: -if (unlikely (folio_test_hugetlb (folio))) { -hugetlb_remove_rmap (folio); - }else { -folio_remove_rmap_ptes (folio, subpage, nr_pages, vma); - } +folio_remove_rmap_ptes (folio, subpage, nr_pages, vma);if (vma->vm_flags &VM_LOCKED )mlock_drain_local ();folio_put_refs (folio, nr_pages);@@ -2398,7 +2408,8 @@ static int folio_not_mapped(struct folio *folio)void try_to_unmap (struct folio *folio,enum ttu_flags flags) {struct rmap_walk_control rwc = { - .rmap_one = try_to_unmap_one, + .rmap_one =folio_test_hugetlb (folio) ? + try_to_unmap_hugetlb_one : try_to_unmap_one, .arg = (void *)flags, .done = folio_not_mapped, .anon_lock = folio_lock_anon_vma_read, --
Quoted 2 lines
2.43.0
Cheers, Lorenzo
Re: [PATCH v3 2/5] mm/rmap: Add try_to_unmap_hugetlb_one
Quoted 32 lines · Lorenzo Stoakes (ARM)
Quoted 2 lines · Dev Jain
Simplify
try_to_unmap_one()by separating the hugetlb parts intotry_to_unmap_hugetlb_one().I hate that we have this separate hugetlb stuff but while we have it, better to be explicit :)
Quoted 25 lines
To understand the correctness of the refactoring, the following points are noted:
1.
try_to_unmap()is called for hugetlb folios only when they are hwpoisoned.2. A hugetlb VMA cannot be mlocked.
3.
page_vma_mapped_walk()returns at most one hugetlb mapping in a VMA, and that mapping points at the head PFN.4. We won't ever process a softleaf entry that encodes a hugetlb folio; hugetlb folios are never swapped out, migration entries will be skipped (
PVMW_MIGRATIONnot passed), and device-exclusive does not work for hugetlb.5. The hwpoison entry is constructed from the poisoned folio, just as in the pre-refactor code. Any previous uffd-wp state is deliberately not preserved for the hwpoison entry.
6.
TTU_HWPOISONis always present; for it to not be present, either the folio has to be in swapcache, ormapping_can_writeback()is true (seeunmap_poisoned_folio), none of which is true for hugetlb folios.
Quoted 5 lines
Quoted 2 lines
7. Hugetlb uses separate counters from normal rss counters, therefore
update_highwater_rss()need not be called.I wonder whether you could bundle some of this up into a comment around
try_to_unmap_hugetlb_one()?
I don't think that is going to simplify things. All of the above mentioned stuff is obvious if you know you are looking at hugetlb code, but isn't obvious when the hugetlb bits are tangled with non-hugetlb, that is why I had to mention all of these points.
Quoted 18 lines
Quoted 10 lines
While at it:
- Change VM_BUG_* to VM_WARN_*.
- Do not declare variables which are only used once.
- Constify some variables.
Quoted 7 lines
- Add some more VM_WARN_* to assert some invariants.
Except the above 4 points, no functional change intended.
Suggested-by David Hildenbrand (Arm) <[email protected]>Signed-off-by Dev Jain <[email protected]>
Quoted 5 lines
A bunch of nits but those addressed LGTM:
Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Thanks very much for doing this is a big improvement!
Thanks. I remember two years back looking at try_to_unmap_one and thinking what the hell is this :) getting to refactor it almost feels cathartic.
Quoted 36 lines
Quoted 36 lines
--- include/linux/hugetlb.h | 1 + mm/rmap.c | 183 +++++++++++++++++++++------------------- 2 files changed, 98 insertions(+), 86 deletions(-)include/linux/hugetlb.h +1
-0@@ -1271,6 +1271,7 @@ static inline void hugetlb_count_sub(long l, struct mm_struct *mm) }pte_t huge_ptep_get (struct mm_struct *mm,unsigned long addr,pte_t *ptep); +unsigned long huge_pte_dirty (pte_t pte);static inline pte_t huge_ptep_clear_flush (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep)mm/rmap.c +10
-0@@ -1978,6 +1978,96 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio, FPB_RESPECT_WRITE | FPB_RESPECT_SOFT_DIRTY); } +static bool try_to_unmap_hugetlb_one (struct folio *folio, +struct vm_area_struct *vma,unsigned long address,void *arg) +{ +DEFINE_FOLIO_VMA_WALK (pvmw, folio, vma, address,0 ); +const unsigned long hsz =huge_page_size (hstate_vma (vma)); +const enum ttu_flags flags = (enum ttu_flags )(long )arg; +struct mm_struct *mm = vma->vm_mm; +struct mmu_notifier_range range; +bool ret = true; +pte_t pteval;
Quoted 8 lines
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 1995
Quoted 5 lines
mm/rmap.c +5
-0+ +/* +* The try_to_unmap() is only passed a hugetlb folio in the case +* where the hugetlb folio is poisoned. +*/ I wonder if the function should be
try_to_unmap_poisoned_hugetlb_one()as a result?
I can, no strong opinion here.
Quoted 5 lines
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 1996
Quoted 2 lines
mm/rmap.c +1
-0+VM_WARN_ON_FOLIO (!folio_test_hwpoison (folio), folio);NIT: Should be
VM_WARN_ON_ONCE_FOLIO()for consistency with below and to avoid repeated warnings?
Ok.
Quoted 12 lines
Quoted 12 lines
mm/rmap.c +11
-0+VM_WARN_ON_ONCE (!(flags &TTU_HWPOISON )); + + range.end =vma_address_end (&pvmw); +mmu_notifier_range_init (&range,MMU_NOTIFY_CLEAR ,0 , vma->vm_mm, + address, range.end); +adjust_range_if_pmd_sharing_possible (vma, &range.start, &range.end); +mmu_notifier_invalidate_range_start (&range); + +/* There is only a single mapping in a VMA. */ +if (!page_vma_mapped_walk (&pvmw)) + goto range_end;
Quoted 8 lines
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 2013
Quoted 6 lines
mm/rmap.c +6
-0+ +VM_WARN_ON_ONCE (address != pvmw.address); + + pteval =huge_ptep_get (mm, address, pvmw.pte); +VM_WARN_ON_ONCE (!pte_present (pteval)); +VM_WARN_ON_ONCE (pte_pfn (pteval) !=folio_pfn (folio));I guess no
TTU_SYNCis possible for hugetlb poison unmap?
Yes.
Quoted 16 lines
Quoted 16 lines
mm/rmap.c +15
-0+ +/* +* huge_pmd_unshare may unmap an entire PMD page. There is no way of +* knowing exactly which PMDs may be cached for this mm, so we must +* flush them all. start/end were already adjusted above to cover this +* range. +*/ +flush_cache_range (vma, range.start, range.end); + +/* +* To call huge_pmd_unshare, i_mmap_rwsem must be held in write mode. +* Caller needs to explicitly do this outside rmap routines. +* +* We also must hold hugetlb vma_lock in write mode. Lock order dictates +* acquiring vma_lock BEFORE i_mmap_rwsem. We can only try lock here and
Quoted 8 lines
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 2034
Quoted 6 lines
mm/rmap.c +6
-0+* fail if unsuccessful. +*/ +if (!folio_test_anon (folio)) { +struct mmu_gather tlb; + +VM_WARN_ON (!(flags &TTU_RMAP_LOCKED ));
VM_WARN_ON_ONCE()?
Okay.
Quoted 26 lines
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 2035
Quoted 2 lines
mm/rmap.c +1
-0+if (!hugetlb_vma_trylock_write (vma)) {How I hate that hugetlb calls their lock a 'VMA lock'...
Quoted 21 lines
mm/rmap.c +21
-0+ ret = false; + goto walk_done; + } + +tlb_gather_mmu_vma (&tlb, vma); +if (huge_pmd_unshare (&tlb, vma, address, pvmw.pte)) { +hugetlb_vma_unlock_write (vma); +huge_pmd_unshare_flush (&tlb, vma); +tlb_finish_mmu (&tlb); +/* +* The PMD table was unmapped, consequently unmapping +* the folio. +*/ + goto walk_done; + } +hugetlb_vma_unlock_write (vma); +tlb_finish_mmu (&tlb); + } + pteval =huge_ptep_clear_flush (vma, address, pvmw.pte); +if (huge_pte_dirty (pteval)) +folio_mark_dirty (folio);
Quoted 8 lines
mm/rmap.c static inline unsigned int folio_unmap_pte_batch(struct folio *folio, · line 2062
Quoted 6 lines
mm/rmap.c +6
-0+ + pteval =swp_entry_to_pte (make_hwpoison_entry (folio_page (folio,0 ))); +hugetlb_count_sub (folio_nr_pages (folio), mm); +set_huge_pte_at (mm, address, pvmw.pte, pteval, hsz); +hugetlb_remove_rmap (folio); +folio_put_refs (folio,1 );Do we want an assert here somehow that we are only walking one folio?
I did not get you. For the try_to_* family in rmap.c, everywhere we walk only one folio don't we?
Quoted 170 lines
Quoted 116 lines
mm/rmap.c +9
-68+ +walk_done: +page_vma_mapped_walk_done (&pvmw); +range_end: +mmu_notifier_invalidate_range_end (&range); +return ret; +} +/* * @arg: enum ttu_flags will be passed to this argument */ @@ -1993,7 +2083,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,enum ttu_flags flags = (enum ttu_flags )(long )arg;unsigned long nr_pages =1 , end_addr;unsigned long pfn; -unsigned long hsz =0 ;int ptes =0 ;/* @@ -2007,8 +2096,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* * For THP, we have to assume the worse case ie pmd for invalidation. -* For hugetlb, it could be much worse if we need to do pud -* invalidation in the case of pmd sharing. * * Note that the folio can not be freed in this function as call of * try_to_unmap() must hold a reference on the folio. @@ -2016,17 +2103,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, range.end =vma_address_end (&pvmw);mmu_notifier_range_init (&range,MMU_NOTIFY_CLEAR ,0 , vma->vm_mm, address, range.end); -if (folio_test_hugetlb (folio)) { -/* -* If sharing is possible, start and end will be adjusted -* accordingly. -*/ -adjust_range_if_pmd_sharing_possible (vma, &range.start, - &range.end); - -/* We need the huge page size for set_huge_pte_at() */ - hsz =huge_page_size (hstate_vma (vma)); - }mmu_notifier_invalidate_range_start (&range);while (page_vma_mapped_walk (&pvmw)) {@@ -2111,66 +2187,13 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,const softleaf_t entry =softleaf_from_pte (pteval); pfn =softleaf_to_pfn (entry); -VM_WARN_ON_FOLIO (folio_test_hugetlb (folio), folio); } subpage =folio_page (folio, pfn -folio_pfn (folio)); anon_exclusive =folio_test_anon (folio) &&PageAnonExclusive (subpage); -if (folio_test_hugetlb (folio)) { -bool anon =folio_test_anon (folio); - -/* -* The try_to_unmap() is only passed a hugetlb folio -* in the case where the hugetlb folio contains a -* poisoned page. -*/ -VM_WARN_ON_FOLIO (!folio_test_hwpoison (folio), folio); -/* -* huge_pmd_unshare may unmap an entire PMD page. -* There is no way of knowing exactly which PMDs may -* be cached for this mm, so we must flush them all. -* start/end were already adjusted above to cover this -* range. -*/ -flush_cache_range (vma, range.start, range.end); - -/* -* To call huge_pmd_unshare, i_mmap_rwsem must be -* held in write mode. Caller needs to explicitly -* do this outside rmap routines. -* -* We also must hold hugetlb vma_lock in write mode. -* Lock order dictates acquiring vma_lock BEFORE -* i_mmap_rwsem. We can only try lock here and fail -* if unsuccessful. -*/ -if (!anon) { -struct mmu_gather tlb; - - VM_BUG_ON(!(flags & T][\TU_RMAP_LOCKED)); -if (!hugetlb_vma_trylock_write (vma)) - goto walk_abort; - -tlb_gather_mmu_vma (&tlb, vma); -if (huge_pmd_unshare (&tlb, vma, address, pvmw.pte)) { -hugetlb_vma_unlock_write (vma); -huge_pmd_unshare_flush (&tlb, vma); -tlb_finish_mmu (&tlb); -/* -* The PMD table was unmapped, -* consequently unmapping the folio. -*/ - goto walk_done; - } -hugetlb_vma_unlock_write (vma); -tlb_finish_mmu (&tlb); - } - pteval =huge_ptep_clear_flush (vma, address, pvmw.pte); -if (pte_dirty (pteval)) -folio_mark_dirty (folio); - }else if (likely (pte_present (pteval))) { +if (likely (pte_present (pteval))) { nr_pages =folio_unmap_pte_batch (folio, &pvmw, flags, pteval); end_addr = address + nr_pages *PAGE_SIZE ;flush_cache_range (vma, address, end_addr);mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2231
Quoted 8 lines
mm/rmap.c +1
-4@@ -2205,20 +2228,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* Update high watermark before we lower rss */ update_hiwater_rss (mm); -/* -* With TTU_HWPOISON, we only expect small folios or hugetlb -* folios here for now. -*/ +/* With TTU_HWPOISON, we only expect small folios here. */ I mean you can further simplify the simplified version this way :)
Quoted 39 lines
mm/rmap.c +5
-15if (folio_test_hwpoison (folio) && (flags &TTU_HWPOISON )) { pteval =swp_entry_to_pte (make_hwpoison_entry (subpage)); -if (folio_test_hugetlb (folio)) { -hugetlb_count_sub (folio_nr_pages (folio), mm); -set_huge_pte_at (mm, address, pvmw.pte, pteval, - hsz); - }else { -dec_mm_counter (mm,mm_counter (folio)); -set_pte_at (mm, address, pvmw.pte, pteval); - } +dec_mm_counter (mm,mm_counter (folio)); +set_pte_at (mm, address, pvmw.pte, pteval); }else if (likely (pte_present (pteval)) &&pte_unused (pteval) && !userfaultfd_armed (vma)) {/* @@ -2346,11 +2360,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,add_mm_counter (mm,mm_counter_file (folio), -nr_pages); } discard: -if (unlikely (folio_test_hugetlb (folio))) { -hugetlb_remove_rmap (folio); - }else { -folio_remove_rmap_ptes (folio, subpage, nr_pages, vma); - } +folio_remove_rmap_ptes (folio, subpage, nr_pages, vma);if (vma->vm_flags &VM_LOCKED )mlock_drain_local ();folio_put_refs (folio, nr_pages);@@ -2398,7 +2408,8 @@ static int folio_not_mapped(struct folio *folio)void try_to_unmap (struct folio *folio,enum ttu_flags flags) {struct rmap_walk_control rwc = { - .rmap_one = try_to_unmap_one, + .rmap_one =folio_test_hugetlb (folio) ? + try_to_unmap_hugetlb_one : try_to_unmap_one, .arg = (void *)flags, .done = folio_not_mapped, .anon_lock = folio_lock_anon_vma_read, --Quoted 2 lines
2.43.0
Cheers, Lorenzo
PATCH v3 3/5 mm/rmap: refactor some code around lazyfree folio unmapping
In reply to Dev JainFor lazyfree folio unmapping, after clearing the ptes we must abort the operation if the folio got dirtied or it has unexpected references.
Refactor this logic into a function which will return whether we need to abort or not.
If we abort, we restore the ptes and bail out of try_to_unmap_one. Otherwise adjust the rss stats of the mm and jump to a label.
Also rename that label from "discard" to "finish_unmap"; the former is appropriate in the lazyfree context, but the code following the label is executed for other successful unmap code paths too, so 'discard' does not sound correct for them.
No functional change intended.
--- mm/rmap.c | 87 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 48 insertions(+), 39 deletions(-)
mm/rmap.c +48 -39
@@ -2068,6 +2068,50 @@ static bool try_to_unmap_hugetlb_one(struct folio *folio,return ret; } +static bool ttu_anon_lazyfree_folio (struct vm_area_struct *vma, +struct folio *folio,unsigned long nr_pages) +{ +int ref_count, map_count; + +/* +* Synchronize with gup_pte_range(): +* - clear PTE; barrier; read refcount +* - inc refcount; barrier; read PTE +*/ +smp_mb (); + + ref_count =folio_ref_count (folio); + map_count =folio_mapcount (folio); + +/* +* Order reads for page refcount and dirty flag +* (see comments in __remove_mapping()). +*/ +smp_rmb (); + +if (folio_test_dirty (folio) && !(vma->vm_flags &VM_DROPPABLE )) { +/* +* redirtied either using the page table or a previously +* obtained GUP reference. +*/ +folio_set_swapbacked (folio); +return false; + } + +/* +* Additional references could be due to GUP or speculative lookups. +* GUP users must mark the folio dirty if there was a modification. +* This folio cannot be reclaimed right now either way, so act just +* like nothing happened. We'll come back here later and detect if the +* folio was dirtied when the additional reference is gone. +*/ +if (ref_count !=1 + map_count) +return false; + +add_mm_counter (vma->vm_mm,MM_ANONPAGES , -nr_pages); +return true; +} +/* * @arg: enum ttu_flags will be passed to this argument */
@@ -2261,47 +2305,12 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* MADV_FREE page check */ if (!folio_test_swapbacked (folio)) { -int ref_count, map_count; - -/* -* Synchronize with gup_pte_range(): -* - clear PTE; barrier; read refcount -* - inc refcount; barrier; read PTE -*/ -smp_mb (); - - ref_count =folio_ref_count (folio); - map_count =folio_mapcount (folio); - -/* -* Order reads for page refcount and dirty flag -* (see comments in __remove_mapping()). -*/ -smp_rmb (); - -if (folio_test_dirty (folio) && !(vma->vm_flags &VM_DROPPABLE )) { -/* -* redirtied either using the page table or a previously -* obtained GUP reference. -*/ -set_ptes (mm, address, pvmw.pte, pteval, nr_pages); -folio_set_swapbacked (folio); - goto walk_abort; - }else if (ref_count !=1 + map_count) { -/* -* Additional reference. Could be a GUP reference or any -* speculative reference. GUP users must mark the folio -* dirty if there was a modification. This folio cannot be -* reclaimed right now either way, so act just like nothing -* happened. -* We'll come back here later and detect if the folio was -* dirtied when the additional reference is gone. -*/ +if (!ttu_anon_lazyfree_folio (vma, folio, + nr_pages)) {set_ptes (mm, address, pvmw.pte, pteval, nr_pages); goto walk_abort; } -add_mm_counter (mm,MM_ANONPAGES , -nr_pages); - goto discard; + goto finish_unmap; }if (folio_dup_swap (folio, subpage) <0 ) {
@@ -2359,7 +2368,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,*/ add_mm_counter (mm,mm_counter_file (folio), -nr_pages); } -discard: +finish_unmap:folio_remove_rmap_ptes (folio, subpage, nr_pages, vma);if (vma->vm_flags &VM_LOCKED )mlock_drain_local ();
Signature · 1 line
2.43.0
PATCH v3 4/5 mm/rmap: refactor anon folio unmap in try_to_unmap_one
In reply to Dev JainRefactor anonymous swapbacked folio unmap to ttu_anon_swapbacked_folio(). While at it, rename the remaining subpage variables to page; folios have pages, not subpages.
No functional change intended.
--- mm/rmap.c | 115 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 68 insertions(+), 47 deletions(-)
mm/rmap.c +68 -47
@@ -2112,6 +2112,66 @@ static bool ttu_anon_lazyfree_folio(struct vm_area_struct *vma,return true; } +static pte_t swp_pte_prepare (swp_entry_t entry,pte_t old_pte, +bool anon_exclusive) +{ +pte_t swp_pte =swp_entry_to_pte (entry); + +if (anon_exclusive) + swp_pte =pte_swp_mkexclusive (swp_pte); + +if (likely (pte_present (old_pte))) { +if (pte_soft_dirty (old_pte)) + swp_pte =pte_swp_mksoft_dirty (swp_pte); +if (pte_uffd (old_pte)) + swp_pte =pte_swp_mkuffd (swp_pte); + }else { +/* Device-exclusive entry */ +if (pte_swp_soft_dirty (old_pte)) + swp_pte =pte_swp_mksoft_dirty (swp_pte); +if (pte_swp_uffd (old_pte)) + swp_pte =pte_swp_mkuffd (swp_pte); + } + +return swp_pte; +} + +static bool ttu_anon_swapbacked_folio (struct vm_area_struct *vma, +struct folio *folio,struct page *page,unsigned long address, +pte_t *ptep,pte_t pteval) +{ +const bool anon_exclusive =folio_test_anon (folio) && +PageAnonExclusive (page); +swp_entry_t entry =page_swap_entry (page); +struct mm_struct *mm = vma->vm_mm; + +if (folio_dup_swap (folio, page) <0 ) +return false; + +/* +* arch_unmap_one() is expected to be a NOP on +* architectures where we could have PFN swap PTEs, +* so we'll not check/care. +*/ +if (arch_unmap_one (mm, vma, address, pteval) <0 ) { +folio_put_swap (folio, page); +return false; + } + +/* See folio_try_share_anon_rmap(): clear PTE first. */ +if (anon_exclusive &&folio_try_share_anon_rmap_pte (folio, page)) { +folio_put_swap (folio, page); +return false; + } + +mm_prepare_for_swap_entries (mm); +dec_mm_counter (mm,MM_ANONPAGES ); +inc_mm_counter (mm,MM_SWAPENTS ); +set_pte_at (mm, address, ptep, +swp_pte_prepare (entry, pteval, anon_exclusive)); +return true; +} +/* * @arg: enum ttu_flags will be passed to this argument */
@@ -2120,9 +2180,9 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, {struct mm_struct *mm = vma->vm_mm;DEFINE_FOLIO_VMA_WALK (pvmw, folio, vma, address,0 ); -bool anon_exclusive, ret = true; +bool ret = true;pte_t pteval; -struct page *subpage; +struct page *page;struct mmu_notifier_range range;enum ttu_flags flags = (enum ttu_flags )(long )arg;unsigned long nr_pages =1 , end_addr;
@@ -2233,9 +2293,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, pfn =softleaf_to_pfn (entry); } - subpage =folio_page (folio, pfn -folio_pfn (folio)); - anon_exclusive =folio_test_anon (folio) && -PageAnonExclusive (subpage); + page =folio_page (folio, pfn -folio_pfn (folio));if (likely (pte_present (pteval))) { nr_pages =folio_unmap_pte_batch (folio, &pvmw, flags, pteval);
@@ -2274,7 +2332,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,/* With TTU_HWPOISON, we only expect small folios here. */ if (folio_test_hwpoison (folio) && (flags &TTU_HWPOISON )) { - pteval =swp_entry_to_pte (make_hwpoison_entry (subpage)); + pteval =swp_entry_to_pte (make_hwpoison_entry (page));dec_mm_counter (mm,mm_counter (folio));set_pte_at (mm, address, pvmw.pte, pteval); } else if (likely(pte_present(pteval)) && pte_unused(pteval) &&
@@ -2291,8 +2349,6 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,*/ dec_mm_counter (mm,mm_counter (folio)); }else if (folio_test_anon (folio)) { -swp_entry_t entry =page_swap_entry (subpage); -pte_t swp_pte;/* * Store the swap location in the pte. * See handle_pte_fault() ...
@@ -2313,47 +2369,12 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, goto finish_unmap; } -if (folio_dup_swap (folio, subpage) <0 ) { -set_pte_at (mm, address, pvmw.pte, pteval); - goto walk_abort; - } - -/* -* arch_unmap_one() is expected to be a NOP on -* architectures where we could have PFN swap PTEs, -* so we'll not check/care. -*/ -if (arch_unmap_one (mm, vma, address, pteval) <0 ) { -folio_put_swap (folio, subpage); -set_pte_at (mm, address, pvmw.pte, pteval); - goto walk_abort; - } - -/* See folio_try_share_anon_rmap(): clear PTE first. */ -if (anon_exclusive && -folio_try_share_anon_rmap_pte (folio, subpage)) { -folio_put_swap (folio, subpage); +if (!ttu_anon_swapbacked_folio (vma, folio, page, address, + pvmw.pte, pteval)) {set_pte_at (mm, address, pvmw.pte, pteval); goto walk_abort; } -mm_prepare_for_swap_entries (mm); -dec_mm_counter (mm,MM_ANONPAGES ); -inc_mm_counter (mm,MM_SWAPENTS ); - swp_pte =swp_entry_to_pte (entry); -if (anon_exclusive) - swp_pte =pte_swp_mkexclusive (swp_pte); -if (likely (pte_present (pteval))) { -if (pte_soft_dirty (pteval)) - swp_pte =pte_swp_mksoft_dirty (swp_pte); -if (pte_uffd (pteval)) - swp_pte =pte_swp_mkuffd (swp_pte); - }else { -if (pte_swp_soft_dirty (pteval)) - swp_pte =pte_swp_mksoft_dirty (swp_pte); -if (pte_swp_uffd (pteval)) - swp_pte =pte_swp_mkuffd (swp_pte); - } -set_pte_at (mm, address, pvmw.pte, swp_pte); + goto finish_unmap; }else {/* * This is a locked file-backed folio,
@@ -2369,7 +2390,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,add_mm_counter (mm,mm_counter_file (folio), -nr_pages); } finish_unmap: -folio_remove_rmap_ptes (folio, subpage, nr_pages, vma); +folio_remove_rmap_ptes (folio, page, nr_pages, vma);if (vma->vm_flags &VM_LOCKED )mlock_drain_local ();folio_put_refs (folio, nr_pages);
Signature · 1 line
2.43.0
Re: [PATCH v3 4/5] mm/rmap: refactor anon folio unmap in try_to_unmap_one
Quoted 7 lines · Dev Jain
Refactor anonymous swapbacked folio unmap to
ttu_anon_swapbacked_folio(). While at it, rename the remaining subpage variables to page; folios have pages, not subpages.No functional change intended.
Signed-off-by Dev Jain <[email protected]>
Signature · 3 lines
Cheers, David
PATCH v3 5/5 mm/rmap: add anon folio unmap dispatcher
In reply to Dev JainAdd ttu_anon_folio() as the common entry point for anonymous folio unmapping. It dispatches to the lazyfree or swapbacked helper as appropriate.
Keep restoration of cleared PTEs in the caller, where the PTEs were removed.
No functional change intended.
--- mm/rmap.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-)
mm/rmap.c +23 -22
@@ -2172,6 +2172,26 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,return true; } +static bool ttu_anon_folio (struct vm_area_struct *vma,struct folio *folio, +struct page *page,unsigned long address,pte_t *ptep, +pte_t pteval,unsigned long nr_pages) +{ +/* +* Store the swap location in the pte. +* See handle_pte_fault() ... +*/ +if (WARN_ON_ONCE (folio_test_swapbacked (folio) != +folio_test_swapcache (folio))) +return false; + +if (!folio_test_swapbacked (folio)) +return ttu_anon_lazyfree_folio (vma, folio, nr_pages); + +/* nr_pages > 1 not supported yet */ +return ttu_anon_swapbacked_folio (vma, folio, page, address, ptep, + pteval); +} +/* * @arg: enum ttu_flags will be passed to this argument */
@@ -2349,31 +2369,12 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,*/ dec_mm_counter (mm,mm_counter (folio)); }else if (folio_test_anon (folio)) { -/* -* Store the swap location in the pte. -* See handle_pte_fault() ... -*/ -if (unlikely (folio_test_swapbacked (folio) != -folio_test_swapcache (folio))) { -WARN_ON_ONCE (1 ); +if (!ttu_anon_folio (vma, folio, page, address, + pvmw.pte, pteval, nr_pages)) { +set_ptes (mm, address, pvmw.pte, pteval, nr_pages); goto walk_abort; } -/* MADV_FREE page check */ -if (!folio_test_swapbacked (folio)) { -if (!ttu_anon_lazyfree_folio (vma, folio, - nr_pages)) { -set_ptes (mm, address, pvmw.pte, pteval, nr_pages); - goto walk_abort; - } - goto finish_unmap; - } - -if (!ttu_anon_swapbacked_folio (vma, folio, page, address, - pvmw.pte, pteval)) { -set_pte_at (mm, address, pvmw.pte, pteval); - goto walk_abort; - } goto finish_unmap; }else {/*
Signature · 1 line
2.43.0