From: "Boehme, Markus" <markubo@amazon.de>
To: "jiapeng.chong@linux.alibaba.com"
<jiapeng.chong@linux.alibaba.com>,
"sj@kernel.org" <sj@kernel.org>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
"damon@lists.linux.dev" <damon@lists.linux.dev>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"abaci@linux.alibaba.com" <abaci@linux.alibaba.com>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Subject: Re: [PATCH] mm/damon/reclaim: Use resource_size function on resource object
Date: Mon, 9 May 2022 10:13:48 +0000 [thread overview]
Message-ID: <5a9268fcd7bb829414da20aecd06d9e1d47a3489.camel@amazon.de> (raw)
In-Reply-To: <20220507153833.45600-1-sj@kernel.org>
(Keeping SJ's mail for context, since original patch seems to not have
gone to the DAMON list.)
On Sat, 2022-05-07 at 15:38 +0000, SeongJae Park wrote:
> On Sat, 7 May 2022 11:25:12 +0800 Jiapeng Chong <jiapeng.chong@linux.alibaba.com> wrote:
>
> > Fix the following coccicheck warnings:
> >
> > ./mm/damon/reclaim.c:241:30-33: WARNING: Suspicious code. resource_size
> > is maybe missing with res.
>
> Nit. I'd prefer having this kind of program outputs in commit message be
> indented and not broken, like below:
>
> ./mm/damon/reclaim.c:241:30-33: WARNING: Suspicious code. resource_size is maybe missing with res.
>
> > Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> > Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
>
> Other than the nit,
>
> Reviewed-by: SeongJae Park <sj@kernel.org>
>
>
> Thanks,
> SJ
>
> > ---
> > mm/damon/reclaim.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> > index f37c5d4b27fa..8efbfb24f3a1 100644
> > --- a/mm/damon/reclaim.c
> > +++ b/mm/damon/reclaim.c
> > @@ -238,7 +238,7 @@ static int walk_system_ram(struct resource *res, void *arg)
> > {
> > struct damon_reclaim_ram_walk_arg *a = arg;
> >
> > - if (a->end - a->start < res->end - res->start) {
> > + if (a->end - a->start < resource_size(res)) {
> > a->start = res->start;
> > a->end = res->end;
> > }
> > --
> > 2.20.1.7.g153144c
This is not just fixing the warning, but changing the calculation
slightly to correctly determine size (res->end not pointing one beyond
the end, but to the last address in the region). For finding the
biggest region it doesn't matter being off by one consistently, but
just fixing one side of the comparison changes behavior in the presence
of two equal regions. When fixing both sides, the "struct
damon_reclaim_ram_walk_arg" might just be replaced with a "struct
resource", too.
Markus
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
prev parent reply other threads:[~2022-05-09 10:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-07 3:25 Jiapeng Chong
2022-05-07 15:38 ` SeongJae Park
2022-05-09 10:13 ` Boehme, Markus [this message]
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=5a9268fcd7bb829414da20aecd06d9e1d47a3489.camel@amazon.de \
--to=markubo@amazon.de \
--cc=abaci@linux.alibaba.com \
--cc=akpm@linux-foundation.org \
--cc=damon@lists.linux.dev \
--cc=jiapeng.chong@linux.alibaba.com \
--cc=linux-kernel@vger.kernel.org \
--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