Re: [RFC PATCH v1.1 3/5] mm/damon/vaddr: drop last same folio access check optimization
SJ Park <[email protected]>
| Newsgroups | dev.linux.lists.damon |
|---|---|
| Message-ID | <[email protected]> |
On Sun, 12 Jul 2026 17:10:09 +0000 [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Low] Unused parameter and dead code tracking logic left behind after removing optimization. > -- > > --- Patch [3]: [RFC PATCH v1.1 3/5] mm/damon/vaddr: drop last same folio access check optimization --- > commit 40e12defdea3b013644cc026283aa34cdd3ae2fb > Author: SJ Park <[email protected]> > mm/damon/vaddr: drop last same folio access check optimization > > This commit removes the last same folio access check optimization from DAMON's > virtual address space primitives. The optimization was found to cause race > conditions when multiple kdamonds are running. > > > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c > > index d10b8042adb5b..16fe210d2042a 100644 > > --- a/mm/damon/vaddr.c > > +++ b/mm/damon/vaddr.c > > [ ... ] > > > @@ -496,26 +489,15 @@ static bool damon_va_young(struct mm_struct *mm, unsigned long addr, > > static void __damon_va_check_access(struct mm_struct *mm, > > struct damon_region *r, bool same_target) > > { > > [Severity: Low] > Does this code still need the same_target parameter? > > With the optimization removed below, the parameter is no longer used in > __damon_va_check_access(). > > The caller, damon_va_check_accesses(), also still computes and tracks the local > same_target variable on every iteration just to pass it here: > > damon_for_each_target(t, ctx) { > mm = damon_get_mm(t); > same_target = false; > damon_for_each_region(r, t) { > __damon_va_check_access(mm, r, same_target); > max_nr_accesses = max(r->nr_accesses, max_nr_accesses); > same_target = true; > } > if (mm) > mmput(mm); > } > > Could we remove the parameter and the dead tracking code in the caller as well? Good finding. I will drop that in the next revision. Thanks, SJ [...]