mm/memory: use folio lock/ref helpers in do_swap_page
Subsystems (2)
PATCH 1/1 mm/memory: use folio lock/ref helpers in do_swap_page
From: Hongfu Li <[email protected]>
Replace page-level lock/ref operations with their folio equivalents in the device_private migration path of do_swap_page().
Introduce a local fault_folio to hold page_folio(vmf->page) to avoid repeated page_folio() invocations.
--- mm/memory.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
mm/memory.c +7 -4
@@ -4852,6 +4852,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) vmf->page =softleaf_to_page (entry); ret =remove_device_exclusive_entry (vmf); }else if (softleaf_is_device_private (entry)) { +struct folio *fault_folio; +if (vmf->flags &FAULT_FLAG_VMA_LOCK ) {/* * migrate_to_ram is not yet ready to operate
@@ -4863,6 +4865,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) } vmf->page =softleaf_to_page (entry); + fault_folio =page_folio (vmf->page); vmf->pte =pte_offset_map_lock (vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl); if (unlikely(!vmf->pte ||
@@ -4874,15 +4877,15 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)* Get a page reference while we know the page can't be * freed. */ -if (trylock_page (vmf->page)) { +if (folio_trylock (fault_folio)) {struct dev_pagemap *pgmap; -get_page (vmf->page); +folio_get (fault_folio);pte_unmap_unlock (vmf->pte, vmf->ptl); pgmap =page_pgmap (vmf->page); ret = pgmap->ops->migrate_to_ram (vmf); -unlock_page (vmf->page); -put_page (vmf->page); +folio_unlock (fault_folio); +folio_put (fault_folio); }else {pte_unmap (vmf->pte);softleaf_entry_wait_on_locked (entry, vmf->ptl);
Signature · 1 line
2.54.0
Quoted 10 lines · Hongfu Li
From: Hongfu Li <[email protected]>
Replace page-level lock/ref operations with their folio equivalents in the
device_privatemigration path ofdo_swap_page().Introduce a local
fault_folioto holdpage_folio(vmf->page)to avoid repeatedpage_folio()invocations.Signed-off-by Hongfu Li <[email protected]>---
Signature · 3 lines
Cheers, David
Andrew Morton
maintainerQuoted 22 lines · On Tue, 21 Jul 2026 09:33:47 +0800 Hongfu Li <[email protected]> wrote:
From: Hongfu Li <[email protected]>
Replace page-level lock/ref operations with their folio equivalents in the
device_privatemigration path ofdo_swap_page().Introduce a local
fault_folioto holdpage_folio(vmf->page)to avoid repeatedpage_folio()invocations....
mm/memory.c +2
-0@@ -4852,6 +4852,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) vmf->page =softleaf_to_page (entry); ret =remove_device_exclusive_entry (vmf); }else if (softleaf_is_device_private (entry)) { +struct folio *fault_folio; +if (vmf->flags &FAULT_FLAG_VMA_LOCK ) {/* * migrate_to_ram is not yet ready to operate
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4870
Quoted 6 lines
mm/memory.c +1
-0@@ -4863,6 +4865,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) } vmf->page =softleaf_to_page (entry); + fault_folio =page_folio (vmf->page); vmf->pte =pte_offset_map_lock (vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl);
AI review thinks we shouldn't be reading vmf->page until after taking the page table lock:
https://sashiko.dev/#/patchset/[email protected]
Quoted 22 lines
Quoted 12 lines
mm/memory.c +2
-0@@ -4852,6 +4852,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) vmf->page =softleaf_to_page (entry); ret =remove_device_exclusive_entry (vmf); }else if (softleaf_is_device_private (entry)) { +struct folio *fault_folio; +if (vmf->flags &FAULT_FLAG_VMA_LOCK ) {/* * migrate_to_ram is not yet ready to operate mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4870
Quoted 6 lines
mm/memory.c +1
-0@@ -4863,6 +4865,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) } vmf->page =softleaf_to_page (entry); + fault_folio =page_folio (vmf->page); vmf->pte =pte_offset_map_lock (vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl);AI review thinks we shouldn't be reading
vmf->pageuntil after taking the page table lock:
Quoted 1 line
https://sashiko.dev/#/patchset/[email protected]
Thanks for the review.
The AI review makes a valid observation: vmf->page should not be accessed before holding the page table lock.
I've moved fault_folio = page_folio(vmf->page) after pte_offset_map_lock() and the PTE validation check, so vmf->page is now read under the page table lock. This fix will be included in v2.
Best regards,
Hongfu
Quoted 4 lines · Hongfu Li
From: Hongfu Li <[email protected]>
Replace page-level lock/ref operations with their folio equivalents in the
device_privatemigration path ofdo_swap_page().
When I do folio conversions, I like to mention the actual advantage of the patch. Here, I'd say something like ...
mm: Use a folio in the softleaf_is_device_private path
Use the folio APIs, replacing four calls to compound_head() with one.
Quoted 13 lines
Introduce a local
fault_folioto holdpage_folio(vmf->page)to avoid repeatedpage_folio()invocations.Signed-off-by Hongfu Li <[email protected]>--- mm/memory.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)mm/memory.c +0
-0
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4855
Quoted 4 lines
mm/memory.c +1
-0@@ -4852,6 +4852,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) vmf->page =softleaf_to_page (entry); ret =remove_device_exclusive_entry (vmf); }else if (softleaf_is_device_private (entry)) { +struct folio *fault_folio;
You can just use the existing 'folio' variable.
Although this function is far too long (400 lines! 14 function-scope variables!) so I don't blame you for not noticing that. Pulling a do_non_swap_page() out of it might be a kindness.
Quoted 12 lines
mm/memory.c +1
-0if (vmf->flags &FAULT_FLAG_VMA_LOCK ) {/* * migrate_to_ram is not yet ready to operate @@ -4863,6 +4865,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) } vmf->page =softleaf_to_page (entry); + fault_folio =page_folio (vmf->page); vmf->pte =pte_offset_map_lock (vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl); if (unlikely(!vmf->pte ||
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4878
Quoted 2 lines
mm/memory.c +0
-0@@ -4874,15 +4877,15 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)* Get a page reference while we know the page can't be * freed.
You should also replace 'page' with 'folio' here. Pages don't have refcounts, folios do.
Quoted 11 lines
Quoted 4 lines
From: Hongfu Li <[email protected]>
Replace page-level lock/ref operations with their folio equivalents in the
device_privatemigration path ofdo_swap_page().When I do folio conversions, I like to mention the actual advantage of the patch. Here, I'd say something like ...
mm: Use a folio in the
softleaf_is_device_privatepathUse the folio APIs, replacing four calls to
compound_head()with one.
Many thanks for your patient guidance.
I will update the commit message per your suggestion for v2.
Quoted 20 lines
Quoted 13 lines
Introduce a local
fault_folioto holdpage_folio(vmf->page)to avoid repeatedpage_folio()invocations.Signed-off-by Hongfu Li <[email protected]>--- mm/memory.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)mm/memory.c +0
-0mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4855
Quoted 4 lines
mm/memory.c +1
-0@@ -4852,6 +4852,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) vmf->page =softleaf_to_page (entry); ret =remove_device_exclusive_entry (vmf); }else if (softleaf_is_device_private (entry)) { +struct folio *fault_folio;You can just use the existing 'folio' variable.
Quoted 3 lines
Although this function is far too long (400 lines! 14 function-scope variables!) so I don't blame you for not noticing that. Pulling a
do_non_swap_page()out of it might be a kindness.
Thanks, will drop the local fault_folio and reuse the existing folio variable. This change will be included in v2.
Agreed that we can split out helpers from do_swap_page, and I will add a new patch for this refactoring in v2.
Quoted 6 lines
Quoted 6 lines
mm/memory.c +0
-0if (vmf->flags &FAULT_FLAG_VMA_LOCK ) {/* * migrate_to_ram is not yet ready to operate @@ -4863,6 +4865,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) }
Quoted 11 lines
mm/memory.c vm_fault_t do_swap_page(struct vm_fault *vmf) · line 4878
Quoted 8 lines
mm/memory.c +1
-0vmf->page =softleaf_to_page (entry); + fault_folio =page_folio (vmf->page); vmf->pte =pte_offset_map_lock (vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl); if (unlikely(!vmf->pte ||@@ -4874,15 +4877,15 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)* Get a page reference while we know the page can't be * freed. You should also replace 'page' with 'folio' here. Pages don't have refcounts, folios do.
Right, I'll update the comment to "folio" to match. Will include in v2.
Best regards,
Hongfu