* [RFC PATCH v2 1/6] mm/damon/vaddr: advance migration attempt in folio level for filter out
2026-11-01 17:16 [RFC PATCH v2 0/6] mm/damon: misc improvements/cleanups for core, vaddr, sysfs and tests SJ Park
@ 2026-11-01 17:16 ` SJ Park
2026-11-01 17:16 ` [RFC PATCH v2 2/6] mm/damon/vaddr: use damos->last_applied for migrate action SJ Park
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: SJ Park @ 2026-11-01 17:16 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
damos_va_migrate_pmd_entry() finds folios to migrate under the given
pmd, using a loop for the pmd address range. The loop advances in page
size if a given folio is excluded by DAMOS ops filters. DAMOS ops
filters work in folio granularity. Hence it is more efficient and makes
sense to advance in the folio size.
Advance the loop in folio size for the ops filter out decision. For
that, do the folio size based loop advancement offset update before
applying DAMOS ops filters.
Closes: https://sashiko.dev/#/bug/linux-67c34df7-6dfb-4e18-9ce0-0d26915e5785
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/vaddr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index b549496ea8e2c..c271b6d63c027 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -881,12 +881,12 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
folio = vm_normal_folio(walk->vma, addr, ptent);
if (!folio)
continue;
+ page_idx = folio_page_idx(folio, pte_page(ptent));
+ nr = folio_nr_pages(folio) - page_idx;
if (damos_va_filter_out(s, folio, walk->vma, addr, pte, NULL))
continue;
damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
migration_lists);
- page_idx = folio_page_idx(folio, pte_page(ptent));
- nr = folio_nr_pages(folio) - page_idx;
}
pte_unmap_unlock(start_pte, ptl);
return 0;
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [RFC PATCH v2 2/6] mm/damon/vaddr: use damos->last_applied for migrate action
2026-11-01 17:16 [RFC PATCH v2 0/6] mm/damon: misc improvements/cleanups for core, vaddr, sysfs and tests SJ Park
2026-11-01 17:16 ` [RFC PATCH v2 1/6] mm/damon/vaddr: advance migration attempt in folio level for filter out SJ Park
@ 2026-11-01 17:16 ` SJ Park
2026-11-01 17:27 ` sashiko-bot
2026-11-01 17:16 ` [RFC PATCH v2 3/6] mm/damon/vaddr: implement sz_filter_passed for migrate_{hot,cold} action SJ Park
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: SJ Park @ 2026-11-01 17:16 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
To avoid applying the same DAMOS action multiple times to folios that
are located across adjacent regions, damos->last_applied was introduced.
DAMOS_MIGRATE_{HOT,COLD} action support in vaddr is not utilizing it.
Use it to avoid unnecessarily applying the same action to the same folio
multiple times.
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/vaddr.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index c271b6d63c027..b1d7bcf7305bb 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -834,6 +834,12 @@ static void damos_va_migrate_dests_add(struct folio *folio,
list_add(&folio->lru, &migration_lists[i]);
}
+static inline bool damos_va_invalid_folio(struct folio *folio,
+ struct damos *s)
+{
+ return !folio || folio == s->last_applied;
+}
+
static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
unsigned long next, struct mm_walk *walk)
{
@@ -854,10 +860,11 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
if (!pmd_present(pmde))
goto huge_out;
folio = vm_normal_folio_pmd(walk->vma, addr, pmde);
- if (!folio)
+ if (damos_va_invalid_folio(folio, s))
goto huge_out;
if (damos_va_filter_out(s, folio, walk->vma, addr, NULL, pmd))
goto huge_out;
+ s->last_applied = folio;
damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
migration_lists);
huge_out:
@@ -879,12 +886,13 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
if (pte_none(ptent) || !pte_present(ptent))
continue;
folio = vm_normal_folio(walk->vma, addr, ptent);
- if (!folio)
+ if (damos_va_invalid_folio(folio, s))
continue;
page_idx = folio_page_idx(folio, pte_page(ptent));
nr = folio_nr_pages(folio) - page_idx;
if (damos_va_filter_out(s, folio, walk->vma, addr, pte, NULL))
continue;
+ s->last_applied = folio;
damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
migration_lists);
}
@@ -991,12 +999,6 @@ struct damos_va_stat_private {
unsigned long *sz_filter_passed;
};
-static inline bool damos_va_invalid_folio(struct folio *folio,
- struct damos *s)
-{
- return !folio || folio == s->last_applied;
-}
-
static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
unsigned long next, struct mm_walk *walk)
{
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 2/6] mm/damon/vaddr: use damos->last_applied for migrate action
2026-11-01 17:16 ` [RFC PATCH v2 2/6] mm/damon/vaddr: use damos->last_applied for migrate action SJ Park
@ 2026-11-01 17:27 ` sashiko-bot
2026-11-01 17:33 ` SJ Park
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-11-01 17:27 UTC (permalink / raw)
To: SJ Park; +Cc: Andrew Morton, damon, linux-mm
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] mm/damon/vaddr: last_applied not updated when damos_va_filter_out() rejects
- [Medium] mm/damon/vaddr: stale folio in last_applied after damos_va_migrate()
- [Low] mm/damon/vaddr: single-page PTE step in damos_va_migrate_pmd_entry()
--
commit af5ba00f84f8ca12cd15e44abbd02f8e8e38b95b
Author: SJ Park <sj@kernel.org>
mm/damon/vaddr: use damos->last_applied for migrate action
This patch updates damos_va_migrate_pmd_entry() to check and record
s->last_applied via damos_va_invalid_folio() so that DAMOS_MIGRATE_HOT and
DAMOS_MIGRATE_COLD actions are not applied multiple times to folios spanning
adjacent virtual address regions.
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index c271b6d63c027..b1d7bcf7305bb 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -834,6 +834,12 @@ static void damos_va_migrate_dests_add(struct folio *folio,
> list_add(&folio->lru, &migration_lists[i]);
> }
>
> +static inline bool damos_va_invalid_folio(struct folio *folio,
> + struct damos *s)
> +{
> + return !folio || folio == s->last_applied;
> +}
> +
> static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> unsigned long next, struct mm_walk *walk)
> {
> @@ -854,10 +860,11 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> if (!pmd_present(pmde))
> goto huge_out;
> folio = vm_normal_folio_pmd(walk->vma, addr, pmde);
> - if (!folio)
> + if (damos_va_invalid_folio(folio, s))
> goto huge_out;
> if (damos_va_filter_out(s, folio, walk->vma, addr, NULL, pmd))
> goto huge_out;
> + s->last_applied = folio;
> damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> migration_lists);
> huge_out:
> @@ -879,12 +886,13 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> if (pte_none(ptent) || !pte_present(ptent))
> continue;
> folio = vm_normal_folio(walk->vma, addr, ptent);
> - if (!folio)
> + if (damos_va_invalid_folio(folio, s))
> continue;
[Severity: Low]
In damos_va_migrate_pmd_entry(), when damos_va_invalid_folio(folio, s) returns
true because folio == s->last_applied for a PTE-mapped large folio at the
start of the next region, the loop continues before computing page_idx and nr:
nr = 1;
ptent = ptep_get(pte);
if (pte_none(ptent) || !pte_present(ptent))
continue;
folio = vm_normal_folio(walk->vma, addr, ptent);
if (damos_va_invalid_folio(folio, s))
continue;
page_idx = folio_page_idx(folio, pte_page(ptent));
nr = folio_nr_pages(folio) - page_idx;
Doesn't this leave nr at 1 and step through all remaining tail PTEs of that
large folio one page at a time instead of advancing by folio_nr_pages(folio) -
page_idx?
> page_idx = folio_page_idx(folio, pte_page(ptent));
> nr = folio_nr_pages(folio) - page_idx;
> if (damos_va_filter_out(s, folio, walk->vma, addr, pte, NULL))
> continue;
> + s->last_applied = folio;
> damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> migration_lists);
> }
[Severity: Medium]
Should s->last_applied be updated even when damos_va_filter_out() rejects the
folio in both the PMD and PTE paths of damos_va_migrate_pmd_entry(), matching
damos_va_stat_pmd_entry() and damon_pa_migrate()?
When a PMD-mapped THP or PTE-mapped large folio spans two adjacent regions and
is rejected by damos_va_filter_out() in the first region, jumping to huge_out
or continuing before setting s->last_applied = folio leaves s->last_applied
unchanged. When the second region is walked, damos_va_filter_out() is called
a second time on the same folio.
If an ops filter such as DAMOS_FILTER_TYPE_YOUNG is used to exclude young
folios (for example with DAMOS_MIGRATE_COLD), evaluating
damos_va_filter_young_match() in the first region clears the page table young
bit and marks the folio idle as a side effect:
mm/damon/vaddr.c:damos_va_filter_young_match() {
...
if (young && ptep)
damon_ptep_mkold(ptep, vma, addr);
else if (young && pmdp)
damon_pmdp_mkold(pmdp, vma, addr);
...
}
Wouldn't re-running damos_va_filter_out() on the same folio in the adjacent
region then observe the folio as old, pass the filter, and migrate the young
folio that was just rejected in the first region?
[Severity: Medium]
Also, how does s->last_applied deduplicate a folio across adjacent regions
once damos_va_migrate() has migrated it in the first region?
In damos_va_migrate(), damon_migrate_pages() runs at the end of each region
before the next region is walked:
mm/damon/vaddr.c:damos_va_migrate() {
...
damon_va_walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
mmput(mm);
for (int i = 0; i < nr_dests; i++) {
nid = use_target_nid ? s->target_nid : dests->node_id_arr[i];
applied += damon_migrate_pages(&priv.migration_lists[i], nid);
cond_resched();
}
...
}
When a large folio spanning two adjacent regions is migrated during the first
region, migrate_pages() allocates a new destination folio, remaps all PTEs or
PMDs across the VMA (including those in the second region) to the new folio,
and frees the source folio.
When damos_va_migrate_pmd_entry() then walks the second region,
vm_normal_folio() or vm_normal_folio_pmd() returns the newly allocated
destination folio while s->last_applied still holds the old source folio
pointer. Since the pointers do not match, won't damos_va_invalid_folio()
return false in the second region, causing damos_va_filter_out() and
damos_va_migrate_dests_add() to be evaluated again on the already-migrated
folio?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261101171630.81477-1-sj@kernel.org?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 2/6] mm/damon/vaddr: use damos->last_applied for migrate action
2026-11-01 17:27 ` sashiko-bot
@ 2026-11-01 17:33 ` SJ Park
0 siblings, 0 replies; 13+ messages in thread
From: SJ Park @ 2026-11-01 17:33 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, Andrew Morton, damon, linux-mm
On Sun, 01 Nov 2026 17:27:03 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] mm/damon/vaddr: last_applied not updated when damos_va_filter_out() rejects
> - [Medium] mm/damon/vaddr: stale folio in last_applied after damos_va_migrate()
> - [Low] mm/damon/vaddr: single-page PTE step in damos_va_migrate_pmd_entry()
> --
>
> commit af5ba00f84f8ca12cd15e44abbd02f8e8e38b95b
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/vaddr: use damos->last_applied for migrate action
>
> This patch updates damos_va_migrate_pmd_entry() to check and record
> s->last_applied via damos_va_invalid_folio() so that DAMOS_MIGRATE_HOT and
> DAMOS_MIGRATE_COLD actions are not applied multiple times to folios spanning
> adjacent virtual address regions.
>
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index c271b6d63c027..b1d7bcf7305bb 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> > @@ -834,6 +834,12 @@ static void damos_va_migrate_dests_add(struct folio *folio,
> > list_add(&folio->lru, &migration_lists[i]);
> > }
> >
> > +static inline bool damos_va_invalid_folio(struct folio *folio,
> > + struct damos *s)
> > +{
> > + return !folio || folio == s->last_applied;
> > +}
> > +
> > static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> > unsigned long next, struct mm_walk *walk)
> > {
> > @@ -854,10 +860,11 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> > if (!pmd_present(pmde))
> > goto huge_out;
> > folio = vm_normal_folio_pmd(walk->vma, addr, pmde);
> > - if (!folio)
> > + if (damos_va_invalid_folio(folio, s))
> > goto huge_out;
> > if (damos_va_filter_out(s, folio, walk->vma, addr, NULL, pmd))
> > goto huge_out;
> > + s->last_applied = folio;
> > damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> > migration_lists);
> > huge_out:
> > @@ -879,12 +886,13 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> > if (pte_none(ptent) || !pte_present(ptent))
> > continue;
> > folio = vm_normal_folio(walk->vma, addr, ptent);
> > - if (!folio)
> > + if (damos_va_invalid_folio(folio, s))
> > continue;
>
> [Severity: Low]
> In damos_va_migrate_pmd_entry(), when damos_va_invalid_folio(folio, s) returns
> true because folio == s->last_applied for a PTE-mapped large folio at the
> start of the next region, the loop continues before computing page_idx and nr:
>
> nr = 1;
> ptent = ptep_get(pte);
>
> if (pte_none(ptent) || !pte_present(ptent))
> continue;
> folio = vm_normal_folio(walk->vma, addr, ptent);
> if (damos_va_invalid_folio(folio, s))
> continue;
> page_idx = folio_page_idx(folio, pte_page(ptent));
> nr = folio_nr_pages(folio) - page_idx;
>
> Doesn't this leave nr at 1 and step through all remaining tail PTEs of that
> large folio one page at a time instead of advancing by folio_nr_pages(folio) -
> page_idx?
Good catch. I will setup nr and page_idx before calling
damos_va_invalid_folio().
>
> > page_idx = folio_page_idx(folio, pte_page(ptent));
> > nr = folio_nr_pages(folio) - page_idx;
> > if (damos_va_filter_out(s, folio, walk->vma, addr, pte, NULL))
> > continue;
> > + s->last_applied = folio;
> > damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> > migration_lists);
> > }
>
> [Severity: Medium]
> Should s->last_applied be updated even when damos_va_filter_out() rejects the
> folio in both the PMD and PTE paths of damos_va_migrate_pmd_entry(), matching
> damos_va_stat_pmd_entry() and damon_pa_migrate()?
>
> When a PMD-mapped THP or PTE-mapped large folio spans two adjacent regions and
> is rejected by damos_va_filter_out() in the first region, jumping to huge_out
> or continuing before setting s->last_applied = folio leaves s->last_applied
> unchanged. When the second region is walked, damos_va_filter_out() is called
> a second time on the same folio.
>
> If an ops filter such as DAMOS_FILTER_TYPE_YOUNG is used to exclude young
> folios (for example with DAMOS_MIGRATE_COLD), evaluating
> damos_va_filter_young_match() in the first region clears the page table young
> bit and marks the folio idle as a side effect:
>
> mm/damon/vaddr.c:damos_va_filter_young_match() {
> ...
> if (young && ptep)
> damon_ptep_mkold(ptep, vma, addr);
> else if (young && pmdp)
> damon_pmdp_mkold(pmdp, vma, addr);
> ...
> }
>
> Wouldn't re-running damos_va_filter_out() on the same folio in the adjacent
> region then observe the folio as old, pass the filter, and migrate the young
> folio that was just rejected in the first region?
Makes sense. I will do so in the next version.
>
> [Severity: Medium]
> Also, how does s->last_applied deduplicate a folio across adjacent regions
> once damos_va_migrate() has migrated it in the first region?
>
> In damos_va_migrate(), damon_migrate_pages() runs at the end of each region
> before the next region is walked:
>
> mm/damon/vaddr.c:damos_va_migrate() {
> ...
> damon_va_walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
> mmput(mm);
>
> for (int i = 0; i < nr_dests; i++) {
> nid = use_target_nid ? s->target_nid : dests->node_id_arr[i];
> applied += damon_migrate_pages(&priv.migration_lists[i], nid);
> cond_resched();
> }
> ...
> }
>
> When a large folio spanning two adjacent regions is migrated during the first
> region, migrate_pages() allocates a new destination folio, remaps all PTEs or
> PMDs across the VMA (including those in the second region) to the new folio,
> and frees the source folio.
>
> When damos_va_migrate_pmd_entry() then walks the second region,
> vm_normal_folio() or vm_normal_folio_pmd() returns the newly allocated
> destination folio while s->last_applied still holds the old source folio
> pointer. Since the pointers do not match, won't damos_va_invalid_folio()
> return false in the second region, causing damos_va_filter_out() and
> damos_va_migrate_dests_add() to be evaluated again on the already-migrated
> folio?
That's a good room to improve. Not a blocker of this patch in my opinion,
though.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261101171630.81477-1-sj@kernel.org?part=2
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH v2 3/6] mm/damon/vaddr: implement sz_filter_passed for migrate_{hot,cold} action
2026-11-01 17:16 [RFC PATCH v2 0/6] mm/damon: misc improvements/cleanups for core, vaddr, sysfs and tests SJ Park
2026-11-01 17:16 ` [RFC PATCH v2 1/6] mm/damon/vaddr: advance migration attempt in folio level for filter out SJ Park
2026-11-01 17:16 ` [RFC PATCH v2 2/6] mm/damon/vaddr: use damos->last_applied for migrate action SJ Park
@ 2026-11-01 17:16 ` SJ Park
2026-11-01 17:21 ` sashiko-bot
2026-11-01 17:16 ` [RFC PATCH v2 4/6] mm/damon/sysfs-schemes: drop redundant and racy zero target_value check SJ Park
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: SJ Park @ 2026-11-01 17:16 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
Support of DAMOS ops filter and sz_filter_passed support is up to each
operation set. DAMON_VADDR supports both for only damos_stat action.
It intentionally does not support those for madvise-based actions
because applying filters could make it unnecessarily complicated and
expensive. The vaddr implementation of migrate_{hot,cold} action does
support DAMOS ops filters but sz_filter_passed. There is no real reason
to not support sz_filter_passed. Add the support.
Pass the sz_filter_passed pointer to the filter applying logic and
accumulate the number.
Closes: https://sashiko.dev/#/bug/linux-986e8b64-8e65-4e32-970f-4d3ebdf8206e
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/vaddr.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index b1d7bcf7305bb..6fd0e07b60705 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -776,6 +776,7 @@ static bool damos_va_filter_out(struct damos *scheme, struct folio *folio,
struct damos_va_migrate_private {
struct list_head *migration_lists;
struct damos *scheme;
+ unsigned long *sz_filter_passed;
};
/*
@@ -846,7 +847,7 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
struct damos_va_migrate_private *priv = walk->private;
struct list_head *migration_lists = priv->migration_lists;
struct damos *s = priv->scheme;
- struct damos_migrate_dests *dests = &s->migrate_dests;
+ unsigned long *sz_filter_passed = priv->sz_filter_passed;
struct folio *folio;
spinlock_t *ptl;
pte_t *start_pte, *pte, ptent;
@@ -865,6 +866,7 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
if (damos_va_filter_out(s, folio, walk->vma, addr, NULL, pmd))
goto huge_out;
s->last_applied = folio;
+ *sz_filter_passed += folio_size(folio);
damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
migration_lists);
huge_out:
@@ -893,6 +895,7 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
if (damos_va_filter_out(s, folio, walk->vma, addr, pte, NULL))
continue;
s->last_applied = folio;
+ *sz_filter_passed += folio_size(folio);
damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
migration_lists);
}
@@ -971,6 +974,7 @@ static unsigned long damos_va_migrate(struct damon_target *target,
priv.migration_lists = kmalloc_objs(*priv.migration_lists, nr_dests);
if (!priv.migration_lists)
return 0;
+ priv.sz_filter_passed = sz_filter_passed;
for (int i = 0; i < nr_dests; i++)
INIT_LIST_HEAD(&priv.migration_lists[i]);
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 3/6] mm/damon/vaddr: implement sz_filter_passed for migrate_{hot,cold} action
2026-11-01 17:16 ` [RFC PATCH v2 3/6] mm/damon/vaddr: implement sz_filter_passed for migrate_{hot,cold} action SJ Park
@ 2026-11-01 17:21 ` sashiko-bot
2026-11-01 17:37 ` SJ Park
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-11-01 17:21 UTC (permalink / raw)
To: SJ Park; +Cc: Andrew Morton, damon, linux-mm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] mm/damon/vaddr: undeclared dests in damos_va_migrate_pmd_entry()
--
commit 7670e03c4d866e7e831b34d6066cefe5b658b73a
Author: SJ Park <sj@kernel.org>
mm/damon/vaddr: implement sz_filter_passed for migrate_{hot,cold} action
This commit adds sz_filter_passed accounting to the DAMON_VADDR
migrate_hot and migrate_cold actions by passing the counter through
damos_va_migrate_private and accumulating the size of folios that pass
ops filters in damos_va_migrate_pmd_entry().
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index b1d7bcf7305bb..6fd0e07b60705 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
[ ... ]
> @@ -846,7 +847,7 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> struct damos_va_migrate_private *priv = walk->private;
> struct list_head *migration_lists = priv->migration_lists;
> struct damos *s = priv->scheme;
> - struct damos_migrate_dests *dests = &s->migrate_dests;
> + unsigned long *sz_filter_passed = priv->sz_filter_passed;
> struct folio *folio;
> spinlock_t *ptl;
> pte_t *start_pte, *pte, ptent;
> @@ -865,6 +866,7 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> if (damos_va_filter_out(s, folio, walk->vma, addr, NULL, pmd))
> goto huge_out;
> s->last_applied = folio;
> + *sz_filter_passed += folio_size(folio);
> damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> migration_lists);
[Severity: Low]
Was the declaration of dests in damos_va_migrate_pmd_entry() removed by
accident when adding sz_filter_passed?
Both the transparent hugepage path above and the PTE loop below in
damos_va_migrate_pmd_entry() still pass dests to
damos_va_migrate_dests_add(), which causes mm/damon/vaddr.c to fail to
compile with an undeclared identifier error when CONFIG_DAMON_VADDR is
enabled.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261101171630.81477-1-sj@kernel.org?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 3/6] mm/damon/vaddr: implement sz_filter_passed for migrate_{hot,cold} action
2026-11-01 17:21 ` sashiko-bot
@ 2026-11-01 17:37 ` SJ Park
0 siblings, 0 replies; 13+ messages in thread
From: SJ Park @ 2026-11-01 17:37 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, Andrew Morton, damon, linux-mm
On Sun, 01 Nov 2026 17:21:39 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] mm/damon/vaddr: undeclared dests in damos_va_migrate_pmd_entry()
> --
>
> commit 7670e03c4d866e7e831b34d6066cefe5b658b73a
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/vaddr: implement sz_filter_passed for migrate_{hot,cold} action
>
> This commit adds sz_filter_passed accounting to the DAMON_VADDR
> migrate_hot and migrate_cold actions by passing the counter through
> damos_va_migrate_private and accumulating the size of folios that pass
> ops filters in damos_va_migrate_pmd_entry().
>
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index b1d7bcf7305bb..6fd0e07b60705 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> [ ... ]
> > @@ -846,7 +847,7 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> > struct damos_va_migrate_private *priv = walk->private;
> > struct list_head *migration_lists = priv->migration_lists;
> > struct damos *s = priv->scheme;
> > - struct damos_migrate_dests *dests = &s->migrate_dests;
> > + unsigned long *sz_filter_passed = priv->sz_filter_passed;
> > struct folio *folio;
> > spinlock_t *ptl;
> > pte_t *start_pte, *pte, ptent;
> > @@ -865,6 +866,7 @@ static int damos_va_migrate_pmd_entry(pmd_t *pmd, unsigned long addr,
> > if (damos_va_filter_out(s, folio, walk->vma, addr, NULL, pmd))
> > goto huge_out;
> > s->last_applied = folio;
> > + *sz_filter_passed += folio_size(folio);
> > damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
> > migration_lists);
>
> [Severity: Low]
> Was the declaration of dests in damos_va_migrate_pmd_entry() removed by
> accident when adding sz_filter_passed?
>
> Both the transparent hugepage path above and the PTE loop below in
> damos_va_migrate_pmd_entry() still pass dests to
> damos_va_migrate_dests_add(), which causes mm/damon/vaddr.c to fail to
> compile with an undeclared identifier error when CONFIG_DAMON_VADDR is
> enabled.
Yes, it is a mistake I made while formatting this patch. I will fix it in the
next spin.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261101171630.81477-1-sj@kernel.org?part=3
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH v2 4/6] mm/damon/sysfs-schemes: drop redundant and racy zero target_value check
2026-11-01 17:16 [RFC PATCH v2 0/6] mm/damon: misc improvements/cleanups for core, vaddr, sysfs and tests SJ Park
` (2 preceding siblings ...)
2026-11-01 17:16 ` [RFC PATCH v2 3/6] mm/damon/vaddr: implement sz_filter_passed for migrate_{hot,cold} action SJ Park
@ 2026-11-01 17:16 ` SJ Park
2026-11-01 17:16 ` [RFC PATCH v2 5/6] selftests/damon/damos_apply_interval: remove unused variables SJ Park
2026-11-01 17:16 ` [RFC PATCH v2 6/6] selftests/damon: remove unused imports of os and sys modules SJ Park
5 siblings, 0 replies; 13+ messages in thread
From: SJ Park @ 2026-11-01 17:16 UTC (permalink / raw)
Cc: SJ Park, Andrew Morton, damon, linux-kernel, linux-mm
DAMON sysfs interface disallowed zero target_value because it could
result in divide-by-zero in damos_quota_store(). Because similar checks
were duplicated to other DAMON API callers, a patch series that starts
with commit 610c4438fb66 ("mm/damon/core: error
damos_commit_quota_goal() for zero target_value") added a same check in
the core layer central parameters, and dropped redundant checks in
DAMON_LRU_SORT and DAMON_SAMPLIE_MTIER. It forgot to drop that in the
DAMON sysfs interface.
Now the zero target_value check in DAMON sysfs interface is only
redundant. Further, it has a time-of-check to time-of-use (toctou) race
with target_value_store(). It can confuse readers to believe [1] the
division-by-zero can still happen. Drop the redundant and racy check.
[1] https://lore.kernel.org/20261021005234.94107-1-sj@kernel.org
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/sysfs-schemes.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 637f1bd0c950b..38efdc258ef3f 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -2898,9 +2898,6 @@ static int damos_sysfs_add_quota_score(
struct damos_sysfs_quota_goal *sysfs_goal =
sysfs_goals->goals_arr[i];
- if (!sysfs_goal->target_value)
- continue;
-
goal = damos_new_quota_goal(sysfs_goal->metric,
sysfs_goal->complement,
sysfs_goal->target_value);
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [RFC PATCH v2 5/6] selftests/damon/damos_apply_interval: remove unused variables
2026-11-01 17:16 [RFC PATCH v2 0/6] mm/damon: misc improvements/cleanups for core, vaddr, sysfs and tests SJ Park
` (3 preceding siblings ...)
2026-11-01 17:16 ` [RFC PATCH v2 4/6] mm/damon/sysfs-schemes: drop redundant and racy zero target_value check SJ Park
@ 2026-11-01 17:16 ` SJ Park
2026-11-01 17:16 ` [RFC PATCH v2 6/6] selftests/damon: remove unused imports of os and sys modules SJ Park
5 siblings, 0 replies; 13+ messages in thread
From: SJ Park @ 2026-11-01 17:16 UTC (permalink / raw)
Cc: SJ Park, Shuah Khan, damon, linux-kernel, linux-kselftest,
linux-mm
wss_collected and nr_quota_exceeds in damos_apply_interval.py are
assigned to but never used. Remove.
The issue is found using flake8.
Signed-off-by: SJ Park <sj@kernel.org>
---
tools/testing/selftests/damon/damos_apply_interval.py | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/testing/selftests/damon/damos_apply_interval.py b/tools/testing/selftests/damon/damos_apply_interval.py
index 0bf7768b2006a..0ad110be0fb94 100755
--- a/tools/testing/selftests/damon/damos_apply_interval.py
+++ b/tools/testing/selftests/damon/damos_apply_interval.py
@@ -42,8 +42,6 @@ def main():
print('kdamond start failed: %s' % err)
exit(1)
- wss_collected = []
- nr_quota_exceeds = 0
while proc.poll() == None:
time.sleep(0.1)
err = kdamonds.kdamonds[0].update_schemes_stats()
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [RFC PATCH v2 6/6] selftests/damon: remove unused imports of os and sys modules
2026-11-01 17:16 [RFC PATCH v2 0/6] mm/damon: misc improvements/cleanups for core, vaddr, sysfs and tests SJ Park
` (4 preceding siblings ...)
2026-11-01 17:16 ` [RFC PATCH v2 5/6] selftests/damon/damos_apply_interval: remove unused variables SJ Park
@ 2026-11-01 17:16 ` SJ Park
2026-11-01 17:20 ` sashiko-bot
5 siblings, 1 reply; 13+ messages in thread
From: SJ Park @ 2026-11-01 17:16 UTC (permalink / raw)
Cc: SJ Park, Shuah Khan, damon, linux-kernel, linux-kselftest,
linux-mm
sysfs_no_op_commit_break.py imports 'sys' module but never uses it.
sysfs_update_schemes_tried_regions_hang imports 'time' module but never
uses it. Remove the unnecessary imports.
The issue is found using flake8.
Signed-off-by: SJ Park <sj@kernel.org>
---
tools/testing/selftests/damon/sysfs_no_op_commit_break.py | 1 -
.../selftests/damon/sysfs_update_schemes_tried_regions_hang.py | 1 -
2 files changed, 2 deletions(-)
diff --git a/tools/testing/selftests/damon/sysfs_no_op_commit_break.py b/tools/testing/selftests/damon/sysfs_no_op_commit_break.py
index 81774ddc55ac4..5664f75186bac 100755
--- a/tools/testing/selftests/damon/sysfs_no_op_commit_break.py
+++ b/tools/testing/selftests/damon/sysfs_no_op_commit_break.py
@@ -4,7 +4,6 @@
import json
import os
import subprocess
-import sys
import _damon_sysfs
diff --git a/tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_hang.py b/tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_hang.py
index 625761c243b58..326a14f040534 100755
--- a/tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_hang.py
+++ b/tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_hang.py
@@ -4,7 +4,6 @@
import os
import subprocess
import sys
-import time
sys.path.append(os.path.dirname(os.path.abspath(__file__)))
import _damon_sysfs
--
2.47.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 6/6] selftests/damon: remove unused imports of os and sys modules
2026-11-01 17:16 ` [RFC PATCH v2 6/6] selftests/damon: remove unused imports of os and sys modules SJ Park
@ 2026-11-01 17:20 ` sashiko-bot
2026-11-01 17:38 ` SJ Park
0 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-11-01 17:20 UTC (permalink / raw)
To: SJ Park; +Cc: Shuah Khan, damon, linux-mm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/damon: commit subject names wrong removed modules
--
commit dfc99fb0f6f595e8bf824229e67f0b391cab2a8e
Author: SJ Park <sj@kernel.org>
selftests/damon: remove unused imports of os and sys modules
This commit removes unused Python module imports from
sysfs_no_op_commit_break.py and sysfs_update_schemes_tried_regions_hang.py
in the DAMON selftests.
> selftests/damon: remove unused imports of os and sys modules
>
> sysfs_no_op_commit_break.py imports 'sys' module but never uses it.
> sysfs_update_schemes_tried_regions_hang imports 'time' module but never
> uses it. Remove the unnecessary imports.
>
> The issue is found using flake8.
>
> Signed-off-by: SJ Park <sj@kernel.org>
[Severity: Low]
As reported by Sashiko on RFC [1], should the commit subject be updated to
refer to the sys and time modules instead of os and sys?
In tools/testing/selftests/damon/sysfs_no_op_commit_break.py, the patch
removes import sys while import os remains and is used in
dump_damon_status_dict():
file_dir = os.path.dirname(os.path.abspath(__file__))
dump_script = os.path.join(file_dir, 'drgn_dump_damon_status.py')
In tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_hang.py,
the patch removes import time while both import os and import sys remain and
are used at module scope:
sys.path.append(os.path.dirname(os.path.abspath(__file__)))
[1] https://lore.kernel.org/r/sashiko-outbox-192115@kernel.org
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261101171630.81477-1-sj@kernel.org?part=6
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [RFC PATCH v2 6/6] selftests/damon: remove unused imports of os and sys modules
2026-11-01 17:20 ` sashiko-bot
@ 2026-11-01 17:38 ` SJ Park
0 siblings, 0 replies; 13+ messages in thread
From: SJ Park @ 2026-11-01 17:38 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, Shuah Khan, damon, linux-mm
On Sun, 01 Nov 2026 17:20:33 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] selftests/damon: commit subject names wrong removed modules
> --
>
> commit dfc99fb0f6f595e8bf824229e67f0b391cab2a8e
> Author: SJ Park <sj@kernel.org>
>
> selftests/damon: remove unused imports of os and sys modules
>
> This commit removes unused Python module imports from
> sysfs_no_op_commit_break.py and sysfs_update_schemes_tried_regions_hang.py
> in the DAMON selftests.
>
> > selftests/damon: remove unused imports of os and sys modules
> >
> > sysfs_no_op_commit_break.py imports 'sys' module but never uses it.
> > sysfs_update_schemes_tried_regions_hang imports 'time' module but never
> > uses it. Remove the unnecessary imports.
> >
> > The issue is found using flake8.
> >
> > Signed-off-by: SJ Park <sj@kernel.org>
>
> [Severity: Low]
> As reported by Sashiko on RFC [1], should the commit subject be updated to
> refer to the sys and time modules instead of os and sys?
Yes, I forgot updating the subject. I will do in the next spin.
>
> In tools/testing/selftests/damon/sysfs_no_op_commit_break.py, the patch
> removes import sys while import os remains and is used in
> dump_damon_status_dict():
>
> file_dir = os.path.dirname(os.path.abspath(__file__))
> dump_script = os.path.join(file_dir, 'drgn_dump_damon_status.py')
>
> In tools/testing/selftests/damon/sysfs_update_schemes_tried_regions_hang.py,
> the patch removes import time while both import os and import sys remain and
> are used at module scope:
>
> sys.path.append(os.path.dirname(os.path.abspath(__file__)))
>
> [1] https://lore.kernel.org/r/sashiko-outbox-192115@kernel.org
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261101171630.81477-1-sj@kernel.org?part=6
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 13+ messages in thread