2023-08-07 10:46:17

by Hyeongtak Ji

[permalink] [raw]
Subject: [PATCH] mm/damon: Prevent unnecessary age reset for regions

DAMON resets the age of each region after applying each scheme,
regardless of whether the scheme has been successfully applied.

This patch adds a simple condition to prevent the age of regions from
being reset when schemes have not been actually applied.

Signed-off-by: Hyeongtak Ji <[email protected]>
---
mm/damon/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 91cff7f2997e..4044fcf18ac1 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -908,7 +908,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
quota->charge_addr_from = r->ar.end + 1;
}
}
- if (s->action != DAMOS_STAT)
+ if (s->action != DAMOS_STAT && sz_applied > 0)
r->age = 0;

update_stat:
--
2.7.4



2023-08-07 18:41:20

by SeongJae Park

[permalink] [raw]
Subject: Re: [PATCH] mm/damon: Prevent unnecessary age reset for regions

Hi Hyeongtak,


Thank you for this patch!

On Mon, 7 Aug 2023 18:44:35 +0900 Hyeongtak Ji <[email protected]> wrote:

> DAMON resets the age of each region after applying each scheme,
> regardless of whether the scheme has been successfully applied.
>
> This patch adds a simple condition to prevent the age of regions from
> being reset when schemes have not been actually applied.

We consider applying the action as making a change to the region, and hence
reset the age to zero. Even if the action was not completely applied,
that might be enough to make some change to the region. The behavior is also
to limit a scheme too repeatedly and frequently applied to a region.

So, this is not a bug but an intended behavior, and I think this change might
not what really necessary.

Is there a specific use case that this change is needed? If so, I think we can
think about extending the interface to support the case.


Thanks,
SJ

>
> Signed-off-by: Hyeongtak Ji <[email protected]>
> ---
> mm/damon/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 91cff7f2997e..4044fcf18ac1 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -908,7 +908,7 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
> quota->charge_addr_from = r->ar.end + 1;
> }
> }
> - if (s->action != DAMOS_STAT)
> + if (s->action != DAMOS_STAT && sz_applied > 0)
> r->age = 0;
>
> update_stat:
> --
> 2.7.4
>

2023-08-08 17:33:43

by Hyeongtak Ji

[permalink] [raw]
Subject: Re: [PATCH] mm/damon: Prevent unnecessary age reset for regions

Hello,

Thank you for your review. I really appreciate it.

On Tue, Aug 8, 2023 at 3:14 AM SeongJae Park <[email protected]> wrote:
>
> Hi Hyeongtak,
>
>
> Thank you for this patch!
>
> On Mon, 7 Aug 2023 18:44:35 +0900 Hyeongtak Ji <[email protected]> wrote:
>
> > DAMON resets the age of each region after applying each scheme,
> > regardless of whether the scheme has been successfully applied.
> >
> > This patch adds a simple condition to prevent the age of regions from
> > being reset when schemes have not been actually applied.
>
> We consider applying the action as making a change to the region, and hence
> reset the age to zero. Even if the action was not completely applied,
> that might be enough to make some change to the region. The behavior is also
> to limit a scheme too repeatedly and frequently applied to a region.

This is what I have totally overlooked.

>
> So, this is not a bug but an intended behavior, and I think this change might
> not what really necessary.

Now I understand that this patch is not necessary.

>
> Is there a specific use case that this change is needed? If so, I think we can
> think about extending the interface to support the case.

Not for now, but if I find any use cases for this situation, I will
let you know.

>
>
> Thanks,
> SJ
>