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

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

[Hyeongtak Ji]

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 <hyeongtak.ji@sk.com>
---
 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

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

[SeongJae Park]

Hi Hyeongtak,


Thank you for this patch!

On Mon, 7 Aug 2023 18:44:35 +0900 Hyeongtak Ji <hyeongtak.ji@gmail.com> 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 <hyeongtak.ji@sk.com>
> ---
>  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
> 

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 <sj@kernel.org> wrote:
>
> Hi Hyeongtak,
>
>
> Thank you for this patch!
>
> On Mon, 7 Aug 2023 18:44:35 +0900 Hyeongtak Ji <hyeongtak.ji@gmail.com> 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
>