Re: [PATCH] mm/damon/reclaim: Use resource_size function on resource object

Re: [PATCH] mm/damon/reclaim: Use resource_size function on resource object

[SeongJae Park]

Hi Jiapeng,


Thank you for this patch!

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

Re: [PATCH] mm/damon/reclaim: Use resource_size function on resource object

[Boehme, Markus]

(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