[PATCH 6.6.y] mm/damon/paddr: drop last same folio access check reuse optimization

[PATCH 6.6.y] mm/damon/paddr: drop last same folio access check reuse optimization

[SJ Park]

It can race when multiple kdamonds are being used.  The problem from the
race is doubtful, but the gain from the optimization is also doubtful.
Simply drop the optimization in favor of code simplicity.

The user impact is doubtfully trivial.  After all, this kind of
interference can happen only by intentional user setup.  Even if it
happens, it will be rare, and the consequence is degradation of the
best-effort monitoring results.  No critical consequences like kernel
panic or memory corruption happen.

The race was discovered [1] by Sashiko.

Link: https://lore.kernel.org/20260715031002.108504-5-sj@kernel.org
Link: https://lore.kernel.org/20260621204050.10993-1-sj@kernel.org [1]
Fixes: a28397beb55b ("mm/damon: implement primitives for physical address space monitoring")
Signed-off-by: SJ Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 5.16.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit f23f0aa62b2f32c2b12f95959fc4603ef81678b2)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/paddr.c | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 909db25efb35e..fe481ae64f37e 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -111,7 +111,7 @@ static bool __damon_pa_young(struct folio *folio, struct vm_area_struct *vma,
 	return *accessed == false;
 }
 
-static bool damon_pa_young(unsigned long paddr, unsigned long *folio_sz)
+static bool damon_pa_young(unsigned long paddr)
 {
 	struct folio *folio = damon_get_folio(PHYS_PFN(paddr));
 	bool accessed = false;
@@ -143,30 +143,17 @@ static bool damon_pa_young(unsigned long paddr, unsigned long *folio_sz)
 		folio_unlock(folio);
 
 out:
-	*folio_sz = folio_size(folio);
 	folio_put(folio);
 	return accessed;
 }
 
 static void __damon_pa_check_access(struct damon_region *r)
 {
-	static unsigned long last_addr;
-	static unsigned long last_folio_sz = PAGE_SIZE;
-	static bool last_accessed;
-
-	/* If the region is in the last checked page, reuse the result */
-	if (ALIGN_DOWN(last_addr, last_folio_sz) ==
-				ALIGN_DOWN(r->sampling_addr, last_folio_sz)) {
-		if (last_accessed)
-			r->nr_accesses++;
-		return;
-	}
+	bool accessed;
 
-	last_accessed = damon_pa_young(r->sampling_addr, &last_folio_sz);
-	if (last_accessed)
+	accessed = damon_pa_young(r->sampling_addr);
+	if (accessed)
 		r->nr_accesses++;
-
-	last_addr = r->sampling_addr;
 }
 
 static unsigned int damon_pa_check_accesses(struct damon_ctx *ctx)
-- 
2.47.3

Re: [PATCH 6.6.y] mm/damon/paddr: drop last same folio access check reuse optimization

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Physical address sampling and PFN iterations fail for large folios because `damon_get_folio` explicitly rejects tail pages.
--

commit cb37cb361a4cb244731d8cba3a6b765eff4dcbaf
Author: SJ Park <sj@kernel.org>

mm/damon/paddr: drop last same folio access check reuse optimization

This commit drops the optimization that reuses the previous physical address
access check if the region falls within the last checked folio. The author
removes it in favor of code simplicity, noting it can race when multiple
kdamonds are used.

Link: https://lore.kernel.org/20260715031002.108504-5-sj@kernel.org
Link: https://lore.kernel.org/20260621204050.10993-1-sj@kernel.org [1]

> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index 909db25efb35e..fe481ae64f37e 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
[ ... ]
> @@ -111,7 +111,7 @@ static bool __damon_pa_young(struct folio *folio, struct vm_area_struct *vma,
>  	return *accessed == false;
>  }
>  
> -static bool damon_pa_young(unsigned long paddr, unsigned long *folio_sz)
> +static bool damon_pa_young(unsigned long paddr)
>  {
>  	struct folio *folio = damon_get_folio(PHYS_PFN(paddr));
>  	bool accessed = false;

[Severity: High]
This isn't a bug introduced by this patch, but does physical address sampling
fail for large folios because damon_get_folio() rejects tail pages?

In damon_pa_young(), the sampled physical address is passed to
damon_get_folio() via PHYS_PFN(paddr). For large folios, this randomly
sampled address is extremely likely to fall on a tail page. Since
damon_get_folio() explicitly returns NULL for tail pages:

mm/damon/ops-common.c:damon_get_folio() {
    ...
    if (!page || PageTail(page))
        return NULL;
    ...
}

Does this mean DAMON silently ignores large folios for physical address
monitoring? This same tail page rejection seems to cause the unconditional
PAGE_SIZE stepping in damon_pa_pageout() and
damon_pa_mark_accessed_or_deactivate() to skip entire large folios if the
region boundaries start on a tail page.

While this wasn't introduced by this patch, dropping the last-folio-size
cache here removes the only mechanism that might have partially masked this
flaw for randomly sampled addresses that happened to hit a head page.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909054545.256743-1-sj@kernel.org?part=1

Re: [PATCH 6.6.y] mm/damon/paddr: drop last same folio access check reuse optimization

[Sasha Levin]

> It can race when multiple kdamonds are being used.  The problem from the
> race is doubtful, but the gain from the optimization is also doubtful.
> Simply drop the optimization in favor of code simplicity.

Queued for 6.6, thanks.

-- 
Thanks,
Sasha