Re: [PATCH 2/3] mm: Add generic helper to hint a large folio
From: Dev Jain
Date: Fri May 09 2025 - 01:25:46 EST
On 08/05/25 4:25 pm, David Hildenbrand wrote:
(2) Do we really need "must be part of the same folio", or could be just
batch over present
ptes that map consecutive PFNs? In that case, a helper that avoids
folio_pte_batch() completely
might be better.
I am not sure I get you here. folio_pte_batch() seems to be the simplest
thing we can do as being done around in the code elsewhere, I am not
aware of any alternate.
If we don't need the folio, then we can have a batching function that
doesn't require the folio.
Likely, we could even factor that (non-folio batching) out from
folio_pte_batch().
The recent fix [1] might make that easier. See below.
So my question is: is something relying on all of these PTEs to point at
the same folio?
Hmm...get_and_clear_full_ptes, as you say in another mail, will require
that...
[1] https://lkml.kernel.org/r/20250502215019.822-2-arkamar@xxxxxxxx
Something like this: (would need kerneldoc, probably remove "addr"
parameter from folio_pte_batch(),
and look into other related cleanups as discussed with Andrew)
I like this refactoring! Can you tell the commit hash on which you make
the patch, I cannot apply it.
So we need to collect/not collect a/d bits according to whether the pte
batch belongs to a large folio/small folios. Seems complicated :)
From f56f67ee5ae9879adb99a8da37fa7ec848c4d256 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@xxxxxxxxxx>
Date: Thu, 8 May 2025 12:53:52 +0200
Subject: [PATCH] tmp
Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
---
mm/internal.h | 84 ++++++++++++++++++++++++++++-----------------------
1 file changed, 46 insertions(+), 38 deletions(-)
diff --git a/mm/internal.h b/mm/internal.h
index 25a29872c634b..53ff8f8a7c8f9 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -217,36 +217,8 @@ static inline pte_t __pte_batch_clear_ignored(pte_t
pte, fpb_t flags)
return pte_wrprotect(pte_mkold(pte));
}
-/**
- * folio_pte_batch - detect a PTE batch for a large folio
- * @folio: The large folio to detect a PTE batch for.
- * @addr: The user virtual address the first page is mapped at.
- * @start_ptep: Page table pointer for the first entry.
- * @pte: Page table entry for the first page.
- * @max_nr: The maximum number of table entries to consider.
- * @flags: Flags to modify the PTE batch semantics.
- * @any_writable: Optional pointer to indicate whether any entry except
the
- * first one is writable.
- * @any_young: Optional pointer to indicate whether any entry except the
- * first one is young.
- * @any_dirty: Optional pointer to indicate whether any entry except the
- * first one is dirty.
- *
- * Detect a PTE batch: consecutive (present) PTEs that map consecutive
- * pages of the same large folio.
- *
- * All PTEs inside a PTE batch have the same PTE bits set, excluding
the PFN,
- * the accessed bit, writable bit, dirty bit (with FPB_IGNORE_DIRTY) and
- * soft-dirty bit (with FPB_IGNORE_SOFT_DIRTY).
- *
- * start_ptep must map any page of the folio. max_nr must be at least
one and
- * must be limited by the caller so scanning cannot exceed a single
page table.
- *
- * Return: the number of table entries in the batch.
- */
-static inline int folio_pte_batch(struct folio *folio, unsigned long addr,
- pte_t *start_ptep, pte_t pte, int max_nr, fpb_t flags,
- bool *any_writable, bool *any_young, bool *any_dirty)
+static inline int pte_batch(pte_t *start_ptep, pte_t pte, int max_nr,
+ fpb_t flags, bool *any_writable, bool *any_young, bool *any_dirty)
{
pte_t expected_pte, *ptep;
bool writable, young, dirty;
@@ -259,14 +231,6 @@ static inline int folio_pte_batch(struct folio
*folio, unsigned long addr,
if (any_dirty)
*any_dirty = false;
- VM_WARN_ON_FOLIO(!pte_present(pte), folio);
- VM_WARN_ON_FOLIO(!folio_test_large(folio) || max_nr < 1, folio);
- VM_WARN_ON_FOLIO(page_folio(pfn_to_page(pte_pfn(pte))) != folio,
folio);
-
- /* Limit max_nr to the actual remaining PFNs in the folio we could
batch. */
- max_nr = min_t(unsigned long, max_nr,
- folio_pfn(folio) + folio_nr_pages(folio) - pte_pfn(pte));
-
nr = pte_batch_hint(start_ptep, pte);
expected_pte = __pte_batch_clear_ignored(pte_advance_pfn(pte, nr),
flags);
ptep = start_ptep + nr;
@@ -300,6 +264,50 @@ static inline int folio_pte_batch(struct folio
*folio, unsigned long addr,
return min(nr, max_nr);
}
+/**
+ * folio_pte_batch - detect a PTE batch for a large folio
+ * @folio: The large folio to detect a PTE batch for.
+ * @addr: The user virtual address the first page is mapped at.
+ * @start_ptep: Page table pointer for the first entry.
+ * @pte: Page table entry for the first page.
+ * @max_nr: The maximum number of table entries to consider.
+ * @flags: Flags to modify the PTE batch semantics.
+ * @any_writable: Optional pointer to indicate whether any entry except
the
+ * first one is writable.
+ * @any_young: Optional pointer to indicate whether any entry except the
+ * first one is young.
+ * @any_dirty: Optional pointer to indicate whether any entry except the
+ * first one is dirty.
+ *
+ * Detect a PTE batch: consecutive (present) PTEs that map consecutive
+ * pages of the same large folio.
+ *
+ * All PTEs inside a PTE batch have the same PTE bits set, excluding
the PFN,
+ * the accessed bit, writable bit, dirty bit (with FPB_IGNORE_DIRTY) and
+ * soft-dirty bit (with FPB_IGNORE_SOFT_DIRTY).
+ *
+ * start_ptep must map any page of the folio. max_nr must be at least
one and
+ * must be limited by the caller so scanning cannot exceed a single
page table.
+ *
+ * Return: the number of table entries in the batch.
+ */
+static inline int folio_pte_batch(struct folio *folio, unsigned long addr,
+ pte_t *start_ptep, pte_t pte, int max_nr, fpb_t flags,
+ bool *any_writable, bool *any_young, bool *any_dirty)
+{
+
+ VM_WARN_ON_FOLIO(!pte_present(pte), folio);
+ VM_WARN_ON_FOLIO(!folio_test_large(folio) || max_nr < 1, folio);
+ VM_WARN_ON_FOLIO(page_folio(pfn_to_page(pte_pfn(pte))) != folio,
folio);
+
+ /* Limit max_nr to the actual remaining PFNs in the folio we could
batch. */
+ max_nr = min_t(unsigned long, max_nr,
+ folio_pfn(folio) + folio_nr_pages(folio) - pte_pfn(pte));
+
+ return pte_batch(start_ptep, pte, max_nr, flags, any_writable,
any_young,
+ any_dirty);
+}
+
/**
* pte_move_swp_offset - Move the swap entry offset field of a swap pte
* forward or backward by delta