linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Enze Li <lienze@kylinos.cn>
To: Jung-JaeJoon <rgbi3307@gmail.com>
Cc: sj@kernel.org,  damon@lists.linux.dev,  linux-mm@kvack.org,
	rgbi3307@naver.com, enze.li@gmx.com
Subject: Re: [PATCH v2] mm/damon/stat: deduplicate intervals_goal setup in damon_stat_build_ctx()
Date: Fri, 05 Dec 2025 18:15:24 +0800	[thread overview]
Message-ID: <875xal44lf.fsf@> (raw)
In-Reply-To: <20251203080440.16096-1-rgbi3307@gmail.com> (Jung-JaeJoon's message of "Wed, 3 Dec 2025 17:04:33 +0900")

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;


  reply	other threads:[~2025-12-05 10:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-03  8:04 Jung-JaeJoon
2025-12-05 10:15 ` Enze Li [this message]
     [not found] ` <6932b0cb.170a0220.2d16ee.eb89SMTPIN_ADDED_BROKEN@mx.google.com>
2025-12-06  0:39   ` JaeJoon Jung

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=875xal44lf.fsf@ \
    --to=lienze@kylinos.cn \
    --cc=damon@lists.linux.dev \
    --cc=enze.li@gmx.com \
    --cc=linux-mm@kvack.org \
    --cc=rgbi3307@gmail.com \
    --cc=rgbi3307@naver.com \
    --cc=sj@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox