linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx()
@ 2025-12-06  1:17 JaeJoon Jung
  2025-12-06  1:50 ` SeongJae Park
  0 siblings, 1 reply; 5+ messages in thread
From: JaeJoon Jung @ 2025-12-06  1:17 UTC (permalink / raw)
  To: sj; +Cc: JaeJoon Jung, damon, linux-mm, lienze

The damon_stat_build_ctx() function sets the values of intervals_goal
structure members.  These values are applied to damon_ctx in
damon_set_attrs().  However, It is resetting the values that were already
applied previously to the same values.  I suggest removing this code as it
constitutes duplicate execution.

Signed-off-by: JaeJoon Jung <rgbi3307@gmail.com>
Reviewed-by: Enze Li <lienze@kylinos.cn>
Reviewed-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/stat.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index bf8626859902..51dd29685915 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -173,14 +173,6 @@ static struct damon_ctx *damon_stat_build_ctx(void)
 	if (damon_set_attrs(ctx, &attrs))
 		goto free_out;
 
-	/*
-	 * auto-tune sampling and aggregation interval aiming 4% DAMON-observed
-	 * accesses ratio, keeping sampling interval in [5ms, 10s] range.
-	 */
-	ctx->attrs.intervals_goal = (struct damon_intervals_goal) {
-		.access_bp = 400, .aggrs = 3,
-		.min_sample_us = 5000, .max_sample_us = 10000000,
-	};
 	if (damon_select_ops(ctx, DAMON_OPS_PADDR))
 		goto free_out;
 
-- 
2.43.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx()
  2025-12-06  1:17 [PATCH v3] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx() JaeJoon Jung
@ 2025-12-06  1:50 ` SeongJae Park
  2025-12-06  4:17   ` JaeJoon Jung
  0 siblings, 1 reply; 5+ messages in thread
From: SeongJae Park @ 2025-12-06  1:50 UTC (permalink / raw)
  To: JaeJoon Jung; +Cc: SeongJae Park, damon, linux-mm, lienze

On Sat,  6 Dec 2025 10:17:13 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:

> The damon_stat_build_ctx() function sets the values of intervals_goal
> structure members.  These values are applied to damon_ctx in
> damon_set_attrs().  However, It is resetting the values that were already
> applied previously to the same values.  I suggest removing this code as it
> constitutes duplicate execution.
> 
> Signed-off-by: JaeJoon Jung <rgbi3307@gmail.com>
> Reviewed-by: Enze Li <lienze@kylinos.cn>

I show Enze did suggest changes [1] to v2 of this patch.  But I don't show Enze
gave Reviewed-by: tag to the patch.  Am I missing something?

[1] https://lore.kernel.org/875xal44lf.fsf@

> Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx()
  2025-12-06  1:50 ` SeongJae Park
@ 2025-12-06  4:17   ` JaeJoon Jung
  2025-12-06  7:58     ` Enze Li
  0 siblings, 1 reply; 5+ messages in thread
From: JaeJoon Jung @ 2025-12-06  4:17 UTC (permalink / raw)
  To: SeongJae Park; +Cc: damon, linux-mm, lienze

On Sat, 6 Dec 2025 at 10:50, SeongJae Park <sj@kernel.org> wrote:
>
> On Sat,  6 Dec 2025 10:17:13 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:
>
> > The damon_stat_build_ctx() function sets the values of intervals_goal
> > structure members.  These values are applied to damon_ctx in
> > damon_set_attrs().  However, It is resetting the values that were already
> > applied previously to the same values.  I suggest removing this code as it
> > constitutes duplicate execution.
> >
> > Signed-off-by: JaeJoon Jung <rgbi3307@gmail.com>
> > Reviewed-by: Enze Li <lienze@kylinos.cn>
>
> I show Enze did suggest changes [1] to v2 of this patch.  But I don't show Enze
> gave Reviewed-by: tag to the patch.  Am I missing something?

I added "Reviewed-by: Enze".
It is just minor commit message format suggestion.
Am I putting too much emphasis on this?
Should I delete "Reviewed-by: Enze"?

>
> [1] https://lore.kernel.org/875xal44lf.fsf@
>
> > Reviewed-by: SeongJae Park <sj@kernel.org>
>
>
> Thanks,
> SJ
>
> [...]


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx()
  2025-12-06  4:17   ` JaeJoon Jung
@ 2025-12-06  7:58     ` Enze Li
  2025-12-06 14:52       ` SeongJae Park
  0 siblings, 1 reply; 5+ messages in thread
From: Enze Li @ 2025-12-06  7:58 UTC (permalink / raw)
  To: JaeJoon Jung, SeongJae Park; +Cc: damon, linux-mm, enze.li

On 2025/12/6 12:17, JaeJoon Jung wrote:
> On Sat, 6 Dec 2025 at 10:50, SeongJae Park <sj@kernel.org> wrote:
>>
>> On Sat,  6 Dec 2025 10:17:13 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:
>>
>>> The damon_stat_build_ctx() function sets the values of intervals_goal
>>> structure members.  These values are applied to damon_ctx in
>>> damon_set_attrs().  However, It is resetting the values that were already
>>> applied previously to the same values.  I suggest removing this code as it
>>> constitutes duplicate execution.
>>>
>>> Signed-off-by: JaeJoon Jung <rgbi3307@gmail.com>
>>> Reviewed-by: Enze Li <lienze@kylinos.cn>
>>
>> I show Enze did suggest changes [1] to v2 of this patch.  But I don't show Enze
>> gave Reviewed-by: tag to the patch.  Am I missing something?
> 
> I added "Reviewed-by: Enze".
> It is just minor commit message format suggestion.
> Am I putting too much emphasis on this?
> Should I delete "Reviewed-by: Enze"?

I have carefully reviewed both the commit message and the code changes
in the patch, it looks good to me.

Feel free to add,

Reviewed-by: Enze Li <lienze@kylinos.cn>

Thanks,
Enze

<...>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx()
  2025-12-06  7:58     ` Enze Li
@ 2025-12-06 14:52       ` SeongJae Park
  0 siblings, 0 replies; 5+ messages in thread
From: SeongJae Park @ 2025-12-06 14:52 UTC (permalink / raw)
  To: Enze Li; +Cc: SeongJae Park, JaeJoon Jung, damon, linux-mm, enze.li

On Sat, 6 Dec 2025 15:58:45 +0800 Enze Li <lienze@kylinos.cn> wrote:

> On 2025/12/6 12:17, JaeJoon Jung wrote:
> > On Sat, 6 Dec 2025 at 10:50, SeongJae Park <sj@kernel.org> wrote:
> >>
> >> On Sat,  6 Dec 2025 10:17:13 +0900 JaeJoon Jung <rgbi3307@gmail.com> wrote:
> >>
> >>> The damon_stat_build_ctx() function sets the values of intervals_goal
> >>> structure members.  These values are applied to damon_ctx in
> >>> damon_set_attrs().  However, It is resetting the values that were already
> >>> applied previously to the same values.  I suggest removing this code as it
> >>> constitutes duplicate execution.
> >>>
> >>> Signed-off-by: JaeJoon Jung <rgbi3307@gmail.com>
> >>> Reviewed-by: Enze Li <lienze@kylinos.cn>
> >>
> >> I show Enze did suggest changes [1] to v2 of this patch.  But I don't show Enze
> >> gave Reviewed-by: tag to the patch.  Am I missing something?
> > 
> > I added "Reviewed-by: Enze".
> > It is just minor commit message format suggestion.
> > Am I putting too much emphasis on this?

Reviewed-by: tags are kind of approval.  Those should be offered by the
reviewer.  Please refert to the doc [1] for more details.

[1] https://origin.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

> > Should I delete "Reviewed-by: Enze"?

Because Enze offered the tag below, no change is needed.

> 
> I have carefully reviewed both the commit message and the code changes
> in the patch, it looks good to me.
> 
> Feel free to add,
> 
> Reviewed-by: Enze Li <lienze@kylinos.cn>

Thank you for your review, Enze.


Thanks,
SJ

[...]


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-12-06 14:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-06  1:17 [PATCH v3] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx() JaeJoon Jung
2025-12-06  1:50 ` SeongJae Park
2025-12-06  4:17   ` JaeJoon Jung
2025-12-06  7:58     ` Enze Li
2025-12-06 14:52       ` SeongJae Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox