linux-kernel - Re: [PATCH v2] mm/damon/core: skip needless update of damon_attrs in damon_commit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20250807162459.52683-1-sj@kernel.org>
Date: Thu,  7 Aug 2025 09:24:59 -0700
From: SeongJae Park <sj@...nel.org>
To: Bijan Tabatabai <bijan311@...il.com>
Cc: SeongJae Park <sj@...nel.org>,
	damon@...ts.linux.dev,
	linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Bijan Tabatabai <bijantabatab@...ron.com>
Subject: Re: [PATCH v2] mm/damon/core: skip needless update of damon_attrs in damon_commit_ctx()

On Wed, 6 Aug 2025 19:48:44 -0500 Bijan Tabatabai <bijan311@...il.com> wrote:

> On Wed, Aug 6, 2025 at 7:19 PM SeongJae Park <sj@...nel.org> wrote:
> >
> > On Wed,  6 Aug 2025 18:42:54 -0500 Bijan Tabatabai <bijan311@...il.com> wrote:
> >
> > > From: Bijan Tabatabai <bijantabatab@...ron.com>
> > >
> > > Currently, damon_commit_ctx() always calls damon_set_attrs() even if the
> > > attributes have not been changed. This can be problematic when the DAMON
> > > state is committed relatively frequently because damon_set_attrs() resets
> > > ctx->next_{aggregation,ops_update}_sis, causing aggregation and ops
> > > update operations to be needlessly delayed.
> > >
> > > This patch avoids this by only calling damon_set_attrs() in
> > > damon_commit_ctx when the attributes have been changed.
> > >
> > > Cc: Bijan Tabatabai <bijan311@...il.com>
> >
> > Maybe above line is added by a mistake?
> 
> Sorry about that. I added it because my internship ends this week and
> wanted to make sure I get notifications on the status of this patch
> (e.g. email notifications when the patch is merged in Andrew's tree).
> If it's inappropriate I will remove it in the next version (unless
> Andrew does it himself).

I was just thinking you might not added this for a purpose.  I don't mind
adding this Cc tag.  As you may already noticed, Andrew merged this patch as is
with this Cc line, so seems Andrew also doesn't really mind, so all is good.

I hope you had a nice internship and continue seeing on the mailing lists :)

[...]
> > > +static bool damon_attrs_equals(const struct damon_attrs *attrs1,
> > > +             const struct damon_attrs *attrs2)
> > > +{
> > > +     const struct damon_intervals_goal *ig1 = &attrs1->intervals_goal;
> > > +     const struct damon_intervals_goal *ig2 = &attrs2->intervals_goal;
> > > +
> > > +     return attrs1->sample_interval == attrs2->sample_interval &&
> > > +             attrs1->aggr_interval == attrs2->aggr_interval &&
> > > +             attrs1->ops_update_interval == attrs2->ops_update_interval &&
> > > +             attrs1->min_nr_regions == attrs2->min_nr_regions &&
> > > +             attrs1->max_nr_regions == attrs2->max_nr_regions &&
> > > +             ig1->access_bp == ig2->access_bp &&
> > > +             ig1->aggrs == ig2->aggrs &&
> > > +             ig1->min_sample_us == ig2->min_sample_us &&
> > > +             ig1->max_sample_us == ig2->max_sample_us;
> > > +
> >
> > Unnecessary blank line?
> 
> Sorry for missing this!

No worry, as you may already noticed, Andrew thankfully fixed this, with a
followup fix patch[1].

So all is well :)

[...]

[1] https://lore.kernel.org/20250807033443.BD30FC4CEEB@smtp.kernel.org


Thanks,
SJ