All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: akpm@linux-foundation.org, david@kernel.org, ljs@kernel.org,
	riel@surriel.com, liam@infradead.org, vbabka@kernel.org,
	harry@kernel.org, jannh@google.com, lance.yang@linux.dev,
	kasong@tencent.com, qi.zheng@linux.dev, shakeel.butt@linux.dev,
	baohua@kernel.org, axelrasmussen@google.com, yuanchu@google.com,
	weixugc@google.com, mhocko@kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/2] mm: mglru: promote mapped executable folios after first usage
Date: Fri, 17 Jul 2026 14:44:03 +0200	[thread overview]
Message-ID: <20260717124403.GG6843@cmpxchg.org> (raw)
In-Reply-To: <7db4d1975cf04e3e48157f98bdf398565ee189a4.1784268206.git.baolin.wang@linux.alibaba.com>

On Fri, Jul 17, 2026 at 06:05:34PM +0800, Baolin Wang wrote:
> Classical LRU protects mapped executable file folios through commit
> 8cab4754d24a0 ("vmscan: make mapped executable pages the first class
> citizen") and commit c909e99364c8 ("vmscan: activate executable pages
> after first usage"), giving executable code a better chance to stay in
> memory, avoiding IO thrashing and improving workload performance.
> 
> However, MGLRU's protection of mapped executable file folios is less
> reliable. Although shrink_folio_list() checks references, the access flag
> of mapped executable file folios may have already been checked and
> cleared by lru_gen_look_around() or walk_mm(). Additionally,
> folio_update_gen() or lru_gen_set_refs() only sets the 'PG_referenced'
> flag for mapped executable file folios, which causes shrink_folio_list()
> to ignore the first usage of these mapped executable file folios and
> reclaim them easily.
> 
> Follow the classical LRU's logic, promoting mapped executable file folios
> after their first usage in folio_update_gen() and lru_gen_set_refs(),
> giving executable code a better chance to stay in memory.
> 
> On my 32-core Arm machine, with the memcg limit set to 2G, running
> 'make -j32' to build kernel showed some improvement in sys time.
> 
> base			patched
> 9248.543s		7861.579s
> 
> While we are at it, introduce a new helper to check mapped executable
> file folios.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>

Both patches contained herein look good:

Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>

[ Sorry ;-) Please don't mix refactoring work with functional
  changes. It's difficult to look through a sequence of diff hunks
  with more than one review narrative interleaved. ]

> ---
>  mm/vmscan.c | 47 +++++++++++++++++++++++++++++++----------------
>  1 file changed, 31 insertions(+), 16 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index de62899c108d..1040bf9f96e8 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -268,6 +268,12 @@ static int sc_swappiness(struct scan_control *sc, struct mem_cgroup *memcg)
>  }
>  #endif
>  
> +static inline bool is_exec_file_folio(const struct folio *folio,
> +		const vma_flags_t *vma_flags)
> +{
> +	return vma_flags_test(vma_flags, VMA_EXEC_BIT) && folio_is_file_lru(folio);
> +}
> +
>  static void set_task_reclaim_state(struct task_struct *task,
>  				   struct reclaim_state *rs)
>  {
> @@ -835,11 +841,15 @@ enum folio_references {
>   * with PG_active set. In contrast, the aging (page table walk) path uses
>   * folio_update_gen().
>   */
> -static bool lru_gen_set_refs(struct folio *folio)
> +static bool lru_gen_set_refs(struct folio *folio, const vma_flags_t *vma_flags)
>  {
>  	/* see the comment on LRU_REFS_FLAGS */
>  	if (!folio_test_referenced(folio) && !folio_test_workingset(folio)) {
>  		set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced));
> +		/* Activate file-backed executable folios after first usage. */
> +		if (is_exec_file_folio(folio, vma_flags))
> +			return true;
> +
>  		return false;
>  	}
>  
> @@ -851,7 +861,7 @@ static bool lru_gen_set_refs(struct folio *folio)
>  	return true;
>  }
>  #else
> -static bool lru_gen_set_refs(struct folio *folio)
> +static bool lru_gen_set_refs(struct folio *folio, const vma_flags_t *vma_flags)
>  {
>  	return false;
>  }
> @@ -886,7 +896,7 @@ static enum folio_references folio_check_references(struct folio *folio,
>  		if (!referenced_ptes)
>  			return FOLIOREF_RECLAIM;
>  
> -		return lru_gen_set_refs(folio) ? FOLIOREF_ACTIVATE : FOLIOREF_KEEP;
> +		return lru_gen_set_refs(folio, &vma_flags) ? FOLIOREF_ACTIVATE : FOLIOREF_KEEP;
>  	}
>  
>  	referenced_folio = folio_test_clear_referenced(folio);
> @@ -914,7 +924,7 @@ static enum folio_references folio_check_references(struct folio *folio,
>  		/*
>  		 * Activate file-backed executable folios after first usage.
>  		 */
> -		if (vma_flags_test(&vma_flags, VMA_EXEC_BIT) && folio_is_file_lru(folio))
> +		if (is_exec_file_folio(folio, &vma_flags))
>  			return FOLIOREF_ACTIVATE;
>  
>  		return FOLIOREF_KEEP;
> @@ -2119,7 +2129,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
>  			 * IO, plus JVM can create lots of anon VM_EXEC folios,
>  			 * so we ignore them here.
>  			 */
> -			if (vma_flags_test(&vma_flags, VMA_EXEC_BIT) && folio_is_file_lru(folio)) {
> +			if (is_exec_file_folio(folio, &vma_flags)) {
>  				nr_rotated += folio_nr_pages(folio);
>  				list_add(&folio->lru, &l_active);
>  				continue;
> @@ -3188,7 +3198,7 @@ static bool positive_ctrl_err(struct ctrl_pos *sp, struct ctrl_pos *pv)
>   ******************************************************************************/
>  
>  /* promote pages accessed through page tables */
> -static int folio_update_gen(struct folio *folio, int gen)
> +static int folio_update_gen(struct folio *folio, int gen, const vma_flags_t *vma_flags)
>  {
>  	unsigned long new_flags, old_flags = READ_ONCE(folio->flags.f);
>  
> @@ -3196,10 +3206,15 @@ static int folio_update_gen(struct folio *folio, int gen)
>  
>  	/* see the comment on LRU_REFS_FLAGS */
>  	if (!folio_test_referenced(folio) && !folio_test_workingset(folio)) {
> +		/* Activate file-backed executable folios after first usage. */
> +		if (is_exec_file_folio(folio, vma_flags))
> +			goto promote;
> +
>  		set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced));
>  		return -1;
>  	}
>  
> +promote:
>  	do {
>  		/* lru_gen_del_folio() has isolated this page? */
>  		if (!(old_flags & LRU_GEN_MASK))
> @@ -3428,8 +3443,8 @@ static bool suitable_to_scan(int total, int young)
>  	return young * n >= total;
>  }
>  
> -static void walk_update_folio(struct lru_gen_mm_walk *walk, struct folio *folio,
> -			      int new_gen, bool dirty)
> +static void walk_update_folio(struct lru_gen_mm_walk *walk, struct vm_area_struct *vma,
> +		struct folio *folio, int new_gen, bool dirty)
>  {
>  	int old_gen;
>  
> @@ -3442,10 +3457,10 @@ static void walk_update_folio(struct lru_gen_mm_walk *walk, struct folio *folio,
>  		folio_mark_dirty(folio);
>  
>  	if (walk) {
> -		old_gen = folio_update_gen(folio, new_gen);
> +		old_gen = folio_update_gen(folio, new_gen, &vma->flags);
>  		if (old_gen >= 0 && old_gen != new_gen)
>  			update_batch_size(walk, folio, old_gen, new_gen);
> -	} else if (lru_gen_set_refs(folio)) {
> +	} else if (lru_gen_set_refs(folio, &vma->flags)) {
>  		old_gen = folio_lru_gen(folio);
>  		if (old_gen >= 0 && old_gen != new_gen)
>  			folio_activate(folio);
> @@ -3518,7 +3533,7 @@ static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end,
>  			continue;
>  
>  		if (last != folio) {
> -			walk_update_folio(walk, last, gen, dirty);
> +			walk_update_folio(walk, args->vma, last, gen, dirty);
>  
>  			last = folio;
>  			dirty = false;
> @@ -3531,7 +3546,7 @@ static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end,
>  		walk->mm_stats[MM_LEAF_YOUNG] += nr;
>  	}
>  
> -	walk_update_folio(walk, last, gen, dirty);
> +	walk_update_folio(walk, args->vma, last, gen, dirty);
>  	last = NULL;
>  
>  	if (i < PTRS_PER_PTE && get_next_vma(PMD_MASK, PAGE_SIZE, args, &start, &end))
> @@ -3609,7 +3624,7 @@ static void walk_pmd_range_locked(pud_t *pud, unsigned long addr, struct vm_area
>  			goto next;
>  
>  		if (last != folio) {
> -			walk_update_folio(walk, last, gen, dirty);
> +			walk_update_folio(walk, vma, last, gen, dirty);
>  
>  			last = folio;
>  			dirty = false;
> @@ -3623,7 +3638,7 @@ static void walk_pmd_range_locked(pud_t *pud, unsigned long addr, struct vm_area
>  		i = i > MIN_LRU_BATCH ? 0 : find_next_bit(bitmap, MIN_LRU_BATCH, i) + 1;
>  	} while (i <= MIN_LRU_BATCH);
>  
> -	walk_update_folio(walk, last, gen, dirty);
> +	walk_update_folio(walk, vma, last, gen, dirty);
>  
>  	lazy_mmu_mode_disable();
>  	spin_unlock(ptl);
> @@ -4258,7 +4273,7 @@ bool lru_gen_look_around(struct page_vma_mapped_walk *pvmw, unsigned int nr)
>  			continue;
>  
>  		if (last != folio) {
> -			walk_update_folio(walk, last, gen, dirty);
> +			walk_update_folio(walk, vma, last, gen, dirty);
>  
>  			last = folio;
>  			dirty = false;
> @@ -4270,7 +4285,7 @@ bool lru_gen_look_around(struct page_vma_mapped_walk *pvmw, unsigned int nr)
>  		young += nr;
>  	}
>  
> -	walk_update_folio(walk, last, gen, dirty);
> +	walk_update_folio(walk, vma, last, gen, dirty);
>  
>  	lazy_mmu_mode_disable();
>  
> -- 
> 2.47.3
> 

  reply	other threads:[~2026-07-17 12:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-17 10:05 [PATCH v3 0/2] promote mapped executable folios after first usage for MGLRU Baolin Wang
2026-07-17 10:05 ` [PATCH v3 1/2] mm: vmscan: convert folio_referenced() to use vma_flags_t Baolin Wang
2026-07-17 12:34   ` Johannes Weiner
2026-07-17 10:05 ` [PATCH v3 2/2] mm: mglru: promote mapped executable folios after first usage Baolin Wang
2026-07-17 12:44   ` Johannes Weiner [this message]
2026-07-17 12:58     ` Baolin Wang
2026-07-17 17:27 ` [PATCH v3 0/2] promote mapped executable folios after first usage for MGLRU Axel Rasmussen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260717124403.GG6843@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=harry@kernel.org \
    --cc=jannh@google.com \
    --cc=kasong@tencent.com \
    --cc=lance.yang@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=qi.zheng@linux.dev \
    --cc=riel@surriel.com \
    --cc=shakeel.butt@linux.dev \
    --cc=vbabka@kernel.org \
    --cc=weixugc@google.com \
    --cc=yuanchu@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.