linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Enze Li <lienze@kylinos.cn>
To: SeongJae Park <sj@kernel.org>
Cc: damon@lists.linux.dev,  linux-mm@kvack.org
Subject: Re: [PATCH] mm/damon: make region calculations more precise
Date: Thu, 22 May 2025 09:53:17 +0800	[thread overview]
Message-ID: <87bjrl9zsi.fsf@kylinos.cn> (raw)
In-Reply-To: <20250521171809.45618-1-sj@kernel.org> (SeongJae Park's message of "Wed, 21 May 2025 10:18:09 -0700")


Hi SeongJae,

On Wed, May 21 2025 at 10:18:09 AM -0700, SeongJae Park wrote:

> Hi Enze,
>
> On Wed, 21 May 2025 15:07:47 +0800 Enze Li <lienze@kylinos.cn> wrote:
>
>> The damon_sz_region() function misses counting one element when
>> calculating region size, which leads to inaccurate results.  This patch
>> corrects the size calculation by properly accounting for all elements.
>
> Thank you for this patch, but I don't think the current calculation is wrong.
> Please refer to the below comment.
>
>> 
>> Signed-off-by: Enze Li <lienze@kylinos.cn>
>> ---
>>  include/linux/damon.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>> index 47e36e6ea203..70473863f7fe 100644
>> --- a/include/linux/damon.h
>> +++ b/include/linux/damon.h
>> @@ -808,7 +808,7 @@ static inline struct damon_region *damon_first_region(struct damon_target *t)
>>  
>>  static inline unsigned long damon_sz_region(struct damon_region *r)
>>  {
>> -	return r->ar.end - r->ar.start;
>> +	return r->ar.end - r->ar.start + 1;
>>  }
>
> 'ar' here is 'struct damon_addr_range' which is for a half-open range.  Refer
> to the comment on 'struct damon_addr_range' definition on include/linux/damon.h
> for detail.  So I don't think the current calculation is wrong.

Thanks for pointing out this - I overlooked it :(

>
> If you think this function also deserves a short comment for clarifying this,
> your patch for that wil be welcomed :)

Since this is already documented in the definition, repeating it here
would be redundant.  If someone submits a similar patch for this in the
future, we can consider adding comments then.

Thanks,
Enze

[...]

>
> Please let me know if I'm missing something.
>
>
> Thanks,
> SJ
>
> [...]


  reply	other threads:[~2025-05-22  1:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-21  7:07 Enze Li
2025-05-21 17:18 ` SeongJae Park
2025-05-22  1:53   ` Enze Li [this message]
2025-05-22 17:16     ` SeongJae Park
2025-05-29  5:53 ` kernel test robot
2025-05-29 16:41   ` SeongJae Park

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=87bjrl9zsi.fsf@kylinos.cn \
    --to=lienze@kylinos.cn \
    --cc=damon@lists.linux.dev \
    --cc=linux-mm@kvack.org \
    --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