All of lore.kernel.org
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: sashiko-bot@kernel.org
Cc: SJ Park <sj@kernel.org>, damon@lists.linux.dev
Subject: Re: [PATCH 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum
Date: Fri, 10 Jul 2026 07:35:00 -0700	[thread overview]
Message-ID: <20260710143501.26630-1-sj@kernel.org> (raw)
In-Reply-To: <20260710140119.C919F1F000E9@smtp.kernel.org>

On Fri, 10 Jul 2026 14:01:19 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The apply_probes callback fails to calculate and return the maximum probe hits weighted sum, unconditionally returning 0.
> --
> 
> --- Patch [4]: [PATCH 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum ---
> commit bb862935a31b8d93d0e79b24c1d5bb92b5faf39a
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: ask apply_probe() to return max probe hits weighted sum
> 
> This commit updates the DAMON operations protocol to allow the apply_probes
> callback to calculate and return the maximum probe hits weighted sum. It
> modifies the function signature to take a return_max_wsum parameter and 
> updates the existing implementation in damon_pa_apply_probes() to return 
> an unsigned int.
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index aee28b05acc6c..e7acd8e610a64 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> [ ... ]
> > @@ -649,6 +649,8 @@ enum damon_ops_id {
> >   * @apply_probes should apply the data attribute probes to each region and
> >   * accordingly update the probe hits counter of the region.  It should also
> >   * set &damon_region->sampling_addr of each region if ``set_samples`` is true.
> > + * It should also return maximum probe hits weighted sum of regions if
> > + * ``return_max_wsum`` is true.
> >   * @get_scheme_score should return the priority score of a region for a scheme
> >   * as an integer in [0, &DAMOS_MAX_SCORE].
> >   * @apply_scheme is called from @kdamond when a region for user provided
> 
> [ ... ]
> 
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index ff4fcca949660..3eba3564a31fc 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> [ ... ]
> > @@ -166,7 +166,8 @@ static bool damon_pa_filter_pass(phys_addr_t pa, struct folio *folio,
> >  	return pass;
> >  }
> >  
> > -static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
> > +static unsigned int damon_pa_apply_probes(struct damon_ctx *ctx,
> > +		bool set_samples, bool return_max_wsum)
> >  {
> >  	struct damon_target *t;
> >  	struct damon_region *r;
> [ ... ]
> > @@ -194,6 +195,7 @@ static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
> >  				folio_put(folio);
> >  		}
> >  	}
> > +	return 0;
> >  }
> 
> [Severity: Medium]
> Will this unconditionally return 0 even when return_max_wsum is true?
> 
> The struct documentation states the callback should return the maximum probe
> hits weighted sum of regions if return_max_wsum is true. However, the
> implementation in damon_pa_apply_probes() simply returns 0 at the end
> without computing the weighted sum.
> 
> Should the maximum weighted sum calculation be implemented here to fulfill
> the updated protocol?

No issue.  No caller sets return_max_wsum true at this moment.

And a later patch in this series will make the argument be respected before
someone sets it true.

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


Thanks,
SJ

  reply	other threads:[~2026-07-10 14:35 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 13:46 [PATCH 00/19] mm/damon: introduce data attributes only monitoring SJ Park
2026-07-10 13:46 ` [PATCH 01/19] mm/damon/core: introduce damon_probe->weight SJ Park
2026-07-10 13:46 ` [PATCH 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address SJ Park
2026-07-10 13:56   ` sashiko-bot
2026-07-10 14:32     ` SJ Park
2026-07-10 13:46 ` [PATCH 03/19] mm/damon/paddr: set samples in apply_probes() if requested SJ Park
2026-07-10 14:01   ` sashiko-bot
2026-07-10 14:33     ` SJ Park
2026-07-10 13:46 ` [PATCH 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum SJ Park
2026-07-10 14:01   ` sashiko-bot
2026-07-10 14:35     ` SJ Park [this message]
2026-07-10 13:46 ` [PATCH 05/19] mm/damon/core: implement damon_probe_hits_wsum() SJ Park
2026-07-10 13:46 ` [PATCH 06/19] mm/damon/paddr: respect return_max_wsum SJ Park
2026-07-10 13:46 ` [PATCH 07/19] mm/damon/core: use abs_diff() instead of abs() SJ Park
2026-07-10 14:10   ` sashiko-bot
2026-07-10 14:43     ` SJ Park
2026-07-10 13:46 ` [PATCH 08/19] mm/damon/core: extend merge function to work with probe hits SJ Park
2026-07-10 14:05   ` sashiko-bot
2026-07-10 14:49     ` SJ Park
2026-07-10 13:46 ` [PATCH 09/19] mm/damon/core: disallow probe_hits overflow on attrs only monitoring SJ Park
2026-07-10 13:46 ` [PATCH 10/19] mm/damon/core: validate params for probe hits weighted sum overflow SJ Park
2026-07-10 14:07   ` sashiko-bot
2026-07-10 14:51     ` SJ Park
2026-07-10 13:46 ` [PATCH 11/19] mm/damon/core: disable access monitoring when probe weights are set SJ Park
2026-07-10 13:46 ` [PATCH 12/19] mm/damon/core: set samples in apply_probes() if " SJ Park
2026-07-10 14:02   ` sashiko-bot
2026-07-10 14:52     ` SJ Park
2026-07-10 13:46 ` [PATCH 13/19] mm/damon/core: s/max_nr_accesses/max_merge_score/ in kdamond_fn() SJ Park
2026-07-10 13:46 ` [PATCH 14/19] mm/damon/core: get merge threshold from probe hits when weights are set SJ Park
2026-07-10 14:05   ` sashiko-bot
2026-07-10 14:54     ` SJ Park
2026-07-10 13:46 ` [PATCH 15/19] mm/damon/core: implement damon_has_probe_weight() SJ Park
2026-07-10 14:11   ` sashiko-bot
2026-07-10 14:55     ` SJ Park
2026-07-10 13:46 ` [PATCH 16/19] mm/damon/sysfs: implement probe/weight file SJ Park
2026-07-10 13:46 ` [PATCH 17/19] Docs/mm/damon/design: document attrs-only monitoring SJ Park
2026-07-10 13:46 ` [PATCH 18/19] Docs/admin-guide/mm/damon/usage: document weight sysfs file SJ Park
2026-07-10 13:46 ` [PATCH 19/19] Docs/ABI/damon: document probe weight file SJ Park
2026-07-10 14:59 ` [PATCH 00/19] mm/damon: introduce data attributes only monitoring 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=20260710143501.26630-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-bot@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.