All of lore.kernel.org
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: SJ Park <sj@kernel.org>
Cc: sashiko-bot@kernel.org, damon@lists.linux.dev
Subject: Re: [RFC PATCH 3/8] mm/damon/ops-common: support hugepage_size damon filter matching
Date: Sun, 13 Sep 2026 10:41:41 -0700	[thread overview]
Message-ID: <20260913174142.105047-1-sj@kernel.org> (raw)
In-Reply-To: <20260913173502.104385-1-sj@kernel.org>

On Sun, 13 Sep 2026 10:35:09 -0700 SJ Park <sj@kernel.org> wrote:

> On Sun, 13 Sep 2026 17:24:42 +0000 sashiko-bot@kernel.org wrote:
> 
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [High] damon_commit_filters() leaves range_min and range_max uninitialized for appended hugepage_size filters.
> > --
> > 
> > commit 377608c283881250a794879309e80b931825b0d3
> > Author: SJ Park <sj@kernel.org>
> > 
> > mm/damon/ops-common: support hugepage_size damon filter matching
> > 
> > Update ops-common data attribute filter matching logic to support
> > hugepage_size filter type.
> > 
> > > diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> > > index c36cc39cd2c70..77366f42b3e5b 100644
> > > --- a/mm/damon/ops-common.c
> > > +++ b/mm/damon/ops-common.c
> > 
> > [ ... ]
> > 
> > > @@ -558,6 +559,15 @@ bool damon_ops_filter_match(struct damon_filter *filter, struct folio *folio)
> > >  			matched = filter->memcg_id == mem_cgroup_id(memcg);
> > >  		rcu_read_unlock();
> > >  		break;
> > > +	case DAMON_FILTER_TYPE_HUGEPAGE_SIZE:
> > > +		if (!folio) {
> > > +			matched = false;
> > > +			break;
> > > +		}
> > > +		folio_sz = folio_size(folio);
> > > +		matched = filter->range_min <= folio_sz &&
> > > +			folio_sz <= filter->range_max;
> > > +		break;
> > 
> > [Severity: High]
> > Does this new filter type need to be explicitly copied when a newly appended
> > filter is committed?
> > 
> > When userspace adds a new filter to an existing DAMON probe via sysfs,
> > damon_commit_filters() in mm/damon/core.c invokes damon_new_filter() which
> > allocates memory without zeroing it. The attributes are then copied in a
> > switch block:
> > 
> > mm/damon/core.c:damon_commit_filters() {
> > 	...
> > 	new_filter = damon_new_filter(src_filter->type,
> > 			src_filter->matching, src_filter->allow);
> > 	if (!new_filter)
> > 		return -ENOMEM;
> > 	switch (src_filter->type) {
> > 	case DAMON_FILTER_TYPE_MEMCG:
> > 		new_filter->memcg_id = src_filter->memcg_id;
> > 		break;
> > 	default:
> > 		break;
> > 	}
> > 	damon_add_filter(dst, new_filter);
> > 	...
> > }
> > 
> > Because DAMON_FILTER_TYPE_HUGEPAGE_SIZE is missing from this switch block,
> > range_min and range_max will be left uninitialized for appended filters. This
> > could result in the filtering logic using garbage memory values.
> > 
> > Should DAMON_FILTER_TYPE_HUGEPAGE_SIZE be added to this switch statement
> > so the range boundaries are properly copied to the new filter?
> 
> No issue.  DAMON sysfs is not allowing hugepage_size probe filter at this
> moment.  Later, it will add the support with proper range setup.

I was wrong.  I will fix this in the next spin.

> 
> > 
> > >  	default:
> > >  		break;
> > >  	}
> > 
> > -- 
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260913171133.102366-1-sj@kernel.org?part=3
> 
> 
> Thanks,
> SJ
> 


Thanks,
SJ

  reply	other threads:[~2026-09-13 17:41 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-13 17:11 [RFC PATCH 0/8] mm/damon: introduce hugepage_size probe filter SJ Park
2026-09-13 17:11 ` [RFC PATCH 1/8] mm/damon/api: introduce DAMON_FILTER_TYPE_HUGEPAGE_SIZE SJ Park
2026-09-13 17:22   ` sashiko-bot
2026-09-13 17:31     ` SJ Park
2026-09-13 17:40       ` SJ Park
2026-09-13 17:11 ` [RFC PATCH 2/8] mm/damon/core: commit hugepage_size type damon filter SJ Park
2026-09-13 17:24   ` sashiko-bot
2026-09-13 17:33     ` SJ Park
2026-09-13 17:41       ` SJ Park
2026-09-13 17:11 ` [RFC PATCH 3/8] mm/damon/ops-common: support hugepage_size damon filter matching SJ Park
2026-09-13 17:24   ` sashiko-bot
2026-09-13 17:35     ` SJ Park
2026-09-13 17:41       ` SJ Park [this message]
2026-09-13 17:11 ` [RFC PATCH 4/8] mm/damon/sysfs: add min,max files under probe filter directory SJ Park
2026-09-13 17:16   ` sashiko-bot
2026-09-13 17:11 ` [RFC PATCH 5/8] mm/damon/sysfs: support hugepage_size probe filter SJ Park
2026-09-13 17:30   ` sashiko-bot
2026-09-13 17:38     ` SJ Park
2026-09-13 17:11 ` [RFC PATCH 6/8] Docs/mm/damon/design: update for " SJ Park
2026-09-13 17:24   ` sashiko-bot
2026-09-13 17:42     ` SJ Park
2026-09-13 17:11 ` [RFC PATCH 7/8] Docs/admin-guide/mm/damon/usage: update for hugepage_size SJ Park
2026-09-13 17:13   ` sashiko-bot
2026-09-13 17:11 ` [RFC PATCH 8/8] Docs/ABI/damon: update for hugepage_size probe filter SJ Park
2026-09-13 17:13   ` sashiko-bot

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=20260913174142.105047-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.