All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "SJ Park" <sj@kernel.org>
Cc: damon@lists.linux.dev
Subject: Re: [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total
Date: Sun, 02 Aug 2026 16:43:14 +0000	[thread overview]
Message-ID: <20260802164314.73A571F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260802162631.90304-3-sj@kernel.org>

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 <sj@kernel.org>

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/20260718005316.89585-1-sj@kernel.org

> 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?

>  	}
>  }

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

  reply	other threads:[~2026-08-02 16:43 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-02 16:26 [RFC PATCH v1.1 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
2026-08-02 16:43   ` sashiko-bot [this message]
2026-08-02 17:41     ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
2026-08-02 16:34   ` sashiko-bot
2026-08-02 17:43     ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
2026-08-02 16:36   ` sashiko-bot
2026-08-02 17:45     ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD} SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
2026-08-02 16:35   ` sashiko-bot
2026-08-02 17:49     ` SJ Park
2026-08-02 17:59       ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
2026-08-02 16:38   ` sashiko-bot
2026-08-02 17:51     ` SJ Park
2026-08-02 16:26 ` [RFC PATCH v1.1 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() SJ Park
2026-08-02 16:51   ` sashiko-bot
2026-08-02 17:51     ` SJ Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260802164314.73A571F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.