linux-kernel - Re: [RFC PATCH v3 1/7] mm/damon/paddr: refactor DAMOS_PAGEOUT with migration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240405191907.66958-1-sj@kernel.org>
Date: Fri,  5 Apr 2024 12:19:07 -0700
From: SeongJae Park <sj@...nel.org>
To: Honggyu Kim <honggyu.kim@...com>
Cc: SeongJae Park <sj@...nel.org>,
	damon@...ts.linux.dev,
	linux-mm@...ck.org,
	akpm@...ux-foundation.org,
	apopple@...dia.com,
	baolin.wang@...ux.alibaba.com,
	dave.jiang@...el.com,
	hyeongtak.ji@...com,
	kernel_team@...ynix.com,
	linmiaohe@...wei.com,
	linux-kernel@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org,
	mathieu.desnoyers@...icios.com,
	mhiramat@...nel.org,
	rakie.kim@...com,
	rostedt@...dmis.org,
	surenb@...gle.com,
	yangx.jy@...itsu.com,
	ying.huang@...el.com,
	ziy@...dia.com,
	42.hyeyoo@...il.com,
	art.jeongseob@...il.com
Subject: Re: [RFC PATCH v3 1/7] mm/damon/paddr: refactor DAMOS_PAGEOUT with migration_mode

On Fri,  5 Apr 2024 15:08:50 +0900 Honggyu Kim <honggyu.kim@...com> wrote:

> This is a preparation patch that introduces migration modes.
> 
> The damon_pa_pageout is renamed to damon_pa_migrate and it receives an
> extra argument for migration_mode.

I personally think keeping damon_pa_pageout() as is and adding a new function
(damon_pa_migrate()) with some duplicated code is also ok, but this approach
also looks fine to me.  So I have no strong opinion here, but just letting you
know I would have no objection at both approaches.

> 
> No functional changes applied.
> 
> Signed-off-by: Honggyu Kim <honggyu.kim@...com>
> ---
>  mm/damon/paddr.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index 081e2a325778..277a1c4d833c 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -224,7 +224,12 @@ static bool damos_pa_filter_out(struct damos *scheme, struct folio *folio)
>  	return false;
>  }
>  
> -static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s)
> +enum migration_mode {
> +	MIG_PAGEOUT,
> +};

To avoid name conflicts, what about renaming to 'damos_migration_mode' and
'DAMOS_MIG_PAGEOUT'?

> +
> +static unsigned long damon_pa_migrate(struct damon_region *r, struct damos *s,
> +				      enum migration_mode mm)

My poor brain has a bit confused with the name.  What about calling it 'mode'?

>  {
>  	unsigned long addr, applied;
>  	LIST_HEAD(folio_list);
> @@ -249,7 +254,14 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s)
>  put_folio:
>  		folio_put(folio);
>  	}
> -	applied = reclaim_pages(&folio_list);
> +	switch (mm) {
> +	case MIG_PAGEOUT:
> +		applied = reclaim_pages(&folio_list);
> +		break;
> +	default:
> +		/* Unexpected migration mode. */
> +		return 0;
> +	}
>  	cond_resched();
>  	return applied * PAGE_SIZE;
>  }
> @@ -297,7 +309,7 @@ static unsigned long damon_pa_apply_scheme(struct damon_ctx *ctx,
>  {
>  	switch (scheme->action) {
>  	case DAMOS_PAGEOUT:
> -		return damon_pa_pageout(r, scheme);
> +		return damon_pa_migrate(r, scheme, MIG_PAGEOUT);
>  	case DAMOS_LRU_PRIO:
>  		return damon_pa_mark_accessed(r, scheme);
>  	case DAMOS_LRU_DEPRIO:
> -- 
> 2.34.1


Thanks,
SJ