linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx()
@ 2025-12-03  8:04 Jung-JaeJoon
  2025-12-05 10:15 ` Enze Li
       [not found] ` <6932b0cb.170a0220.2d16ee.eb89SMTPIN_ADDED_BROKEN@mx.google.com>
  0 siblings, 2 replies; 3+ messages in thread
From: Jung-JaeJoon @ 2025-12-03  8:04 UTC (permalink / raw)
  To: sj; +Cc: Jung-JaeJoon, damon, linux-mm, rgbi3307

The damon_stat_build_ctx() function sets the values
of attrs.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: Jung-JaeJoon <rgbi3307@gmail.com>
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] 3+ messages in thread

* Re: [PATCH v2] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx()
  2025-12-03  8:04 [PATCH v2] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx() Jung-JaeJoon
@ 2025-12-05 10:15 ` Enze Li
       [not found] ` <6932b0cb.170a0220.2d16ee.eb89SMTPIN_ADDED_BROKEN@mx.google.com>
  1 sibling, 0 replies; 3+ messages in thread
From: Enze Li @ 2025-12-05 10:15 UTC (permalink / raw)
  To: Jung-JaeJoon; +Cc: sj, damon, linux-mm, rgbi3307, enze.li

Hello JaeJoon,

Thanks for your patch!  I've a trivial comment inline, see below.

On Wed, Dec 03 2025 at 05:04:33 PM +0900, Jung-JaeJoon wrote:

> The damon_stat_build_ctx() function sets the values
> of attrs.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.

Would you mind reformatting the patch description?  The kernel community
documentation recommends wrapping lines at 75 columns [1].  While the
current length is within the limit, adjusting it for more balanced line
lengths would improve readability.  Also, I suggest using two spaces
after a period instead of a line break.

[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#subject-line

Thanks,
Enze

>
> Signed-off-by: Jung-JaeJoon <rgbi3307@gmail.com>
> 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;


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

* Re: [PATCH v2] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx()
       [not found] ` <6932b0cb.170a0220.2d16ee.eb89SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2025-12-06  0:39   ` JaeJoon Jung
  0 siblings, 0 replies; 3+ messages in thread
From: JaeJoon Jung @ 2025-12-06  0:39 UTC (permalink / raw)
  To: Enze Li; +Cc: sj, damon, linux-mm, rgbi3307, enze.li

Hello Enze,

Thank you for your kind help.  I will reflect it in v3 and send it again.

Thanks,
JaeJoon

On Fri, 5 Dec 2025 at 19:15, Enze Li <lienze@kylinos.cn> wrote:
>
> Hello JaeJoon,
>
> Thanks for your patch!  I've a trivial comment inline, see below.
>
> On Wed, Dec 03 2025 at 05:04:33 PM +0900, Jung-JaeJoon wrote:
>
> > The damon_stat_build_ctx() function sets the values
> > of attrs.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.
>
> Would you mind reformatting the patch description?  The kernel community
> documentation recommends wrapping lines at 75 columns [1].  While the
> current length is within the limit, adjusting it for more balanced line
> lengths would improve readability.  Also, I suggest using two spaces
> after a period instead of a line break.
>
> [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#subject-line
>
> Thanks,
> Enze
>
> >
> > Signed-off-by: Jung-JaeJoon <rgbi3307@gmail.com>
> > 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;


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

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

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-03  8:04 [PATCH v2] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx() Jung-JaeJoon
2025-12-05 10:15 ` Enze Li
     [not found] ` <6932b0cb.170a0220.2d16ee.eb89SMTPIN_ADDED_BROKEN@mx.google.com>
2025-12-06  0:39   ` JaeJoon Jung

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