Re: [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total
SJ Park <[email protected]> Sun, 2 Aug 2026 10:41:44 -0700
| Newsgroups | dev.linux.lists.damon |
|---|---|
| Message-ID | <[email protected]> |
On Sun, 02 Aug 2026 16:43:14 +0000 [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > Pre-existing issues: > - [Medium] The patch incompletely fixes the uninitialized `last_psi_total` variable usage. While it initializes `last_psi_total` when the `kdamond` thread starts, it fails to initialize it for schemes or quota goals added dynamically at runtime via sysfs. > -- > > --- Patch [2]: [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total --- > commit ca3c4694fe111f676d0de56148cf986c02532428 > Author: SJ Park <[email protected]> > > mm/damon/core: initialize damos_quota_goal->last_psi_total > > When DAMOS_QUOTA_SOME_MEM_PSI_US metric damos quota goal is set, the PSI > delta for the feedback loop is calculated using > damos_quota_goal->last_psi_total. It is not initialized at the beginning. > This commit initializes the value at the beginning of kdamond. > > Link: https://lore.kernel.org/[email protected] > > > diff --git a/mm/damon/core.c b/mm/damon/core.c > > index e2900d0c984c9..3bdbf4fbf7147 100644 > > --- a/mm/damon/core.c > > +++ b/mm/damon/core.c > > [ ... ] > > > @@ -3744,6 +3755,7 @@ static void kdamond_init_ctx(struct damon_ctx *ctx) > > damon_for_each_scheme(scheme, ctx) { > > damos_set_next_apply_sis(scheme, ctx); > > damos_set_filters_default_reject(scheme); > > + damos_init_quota_goal_last_psi(scheme); > > [Severity: Medium] > This is a pre-existing issue, but does this completely fix the uninitialized > variable usage? > > While this initializes last_psi_total when the kdamond thread starts, it > appears goals added dynamically at runtime via sysfs will still be > uninitialized. > > When a goal is parsed from sysfs, memory is allocated without zeroing the > structure, leaving last_psi_total filled with slab garbage: > > mm/damon/sysfs-schemes.c:damos_sysfs_add_quota_score() { > ... > goal = damos_new_quota_goal(sysfs_goal->metric, > sysfs_goal->target_value); > ... > } > > During the context commit phase, the goal is dynamically allocated but > last_psi_total is intentionally bypassed: > > mm/damon/core.c:damos_commit_quota_goals() { > ... > new_goal = damos_new_quota_goal( > src_goal->metric, src_goal->target_value); > ... > } > > When the feedback loop executes, this uninitialized memory is consumed: > > mm/damon/core.c:damos_set_quota_goal_current_value() { > ... > goal->current_value = now_psi_total - goal->last_psi_total; > ... > } > > Will this cause a random, extremely large or small initial quota adjustment > when a user adds a new DAMON scheme or quota goal with the > DAMOS_QUOTA_SOME_MEM_PSI_US metric to a running context via sysfs? Good catch. To handle this, the fix will be little bit complicated. It may not deserve stable@ backport. I will drop this from the next version of this series and work on more complete fix. > > > } > > } > > -- > Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2 > Thanks, SJ