linux-kernel - Re: [RFC PATCH v3 2/7] mm: make alloc_demote_folio externally invokable for 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 for Android: free password hash cracker in your pocket

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

Date: Fri,  5 Apr 2024 12:20:10 -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 2/7] mm: make alloc_demote_folio externally invokable for migration

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

> The alloc_demote_folio can be used out of vmscan.c so it'd be better to
> remove static keyword from it.
> 
> This function can also be used for both demotion and promotion so it'd
> be better to rename it from alloc_demote_folio to alloc_migrate_folio.

I'm not sure if renaming is really needed, but has no strong opinion.

> 
> Signed-off-by: Honggyu Kim <honggyu.kim@...com>

I have one more trivial comment below, but finds no blocker for me.

Reviewed-by: SeongJae Park <sj@...nel.org>

> ---
>  mm/internal.h |  1 +
>  mm/vmscan.c   | 10 +++++++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index f309a010d50f..c96ff9bc82d0 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -866,6 +866,7 @@ extern unsigned long  __must_check vm_mmap_pgoff(struct file *, unsigned long,
>          unsigned long, unsigned long);
>  
>  extern void set_pageblock_order(void);
> +struct folio *alloc_migrate_folio(struct folio *src, unsigned long private);
>  unsigned long reclaim_pages(struct list_head *folio_list);
>  unsigned int reclaim_clean_pages_from_list(struct zone *zone,
>  					    struct list_head *folio_list);
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 4255619a1a31..9e456cac03b4 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -910,8 +910,7 @@ static void folio_check_dirty_writeback(struct folio *folio,
>  		mapping->a_ops->is_dirty_writeback(folio, dirty, writeback);
>  }
>  
> -static struct folio *alloc_demote_folio(struct folio *src,
> -		unsigned long private)
> +struct folio *alloc_migrate_folio(struct folio *src, unsigned long private)
>  {
>  	struct folio *dst;
>  	nodemask_t *allowed_mask;
> @@ -935,6 +934,11 @@ static struct folio *alloc_demote_folio(struct folio *src,
>  	if (dst)
>  		return dst;
>  
> +	/*
> +	 * Allocation failed from the target node so try to allocate from
> +	 * fallback nodes based on allowed_mask.
> +	 * See fallback_alloc() at mm/slab.c.
> +	 */

I think this might better to be a separate cleanup patch, but given its small
size, I have no strong opinion.

>  	mtc->gfp_mask &= ~__GFP_THISNODE;
>  	mtc->nmask = allowed_mask;
>  
> @@ -973,7 +977,7 @@ static unsigned int demote_folio_list(struct list_head *demote_folios,
>  	node_get_allowed_targets(pgdat, &allowed_mask);
>  
>  	/* Demotion ignores all cpuset and mempolicy settings */
> -	migrate_pages(demote_folios, alloc_demote_folio, NULL,
> +	migrate_pages(demote_folios, alloc_migrate_folio, NULL,
>  		      (unsigned long)&mtc, MIGRATE_ASYNC, MR_DEMOTION,
>  		      &nr_succeeded);
>  
> -- 
> 2.34.1


Thanks,
SJ