promote mapped executable folios after first usage for MGLRU
Subsystems (4)
Baolin Wang
reviewerPATCH v3 1/2 mm: vmscan: convert folio_referenced() to use vma_flags_t
In reply to Baolin WangReplace use of the legacy vm_flags_t flags with vma_flags_t values for folio_referenced() and related logic. This is also a preparation for the following changes.
No functional changes.
--- include/linux/rmap.h | 7 +++---- mm/rmap.c | 19 +++++++++++-------- mm/vmscan.c | 14 +++++++------- 3 files changed, 21 insertions(+), 19 deletions(-)
include/linux/rmap.h +3 -4
@@ -843,7 +843,7 @@ static inline int folio_try_share_anon_rmap_pmd(struct folio *folio,* Called from mm/vmscan.c to handle paging out */ int folio_referenced (struct folio *,int is_locked, -struct mem_cgroup *memcg,vm_flags_t *vm_flags); +struct mem_cgroup *memcg,vma_flags_t *vma_flags);void try_to_migrate (struct folio *folio,enum ttu_flags flags);void try_to_unmap (struct folio *,enum ttu_flags flags);
@@ -975,10 +975,9 @@ struct anon_vma *folio_lock_anon_vma_read(const struct folio *folio,#define anon_vma_prepare (vma) (0)static inline int folio_referenced (struct folio *folio,int is_locked, -struct mem_cgroup *memcg, -vm_flags_t *vm_flags) +struct mem_cgroup *memcg,vma_flags_t *vma_flags) { - *vm_flags =0 ; +vma_flags_clear_all (vma_flags);return 0 ; }
mm/rmap.c +11 -8
@@ -907,7 +907,7 @@ pmd_t *mm_find_pmd(struct mm_struct *mm, unsigned long address)struct folio_referenced_arg {int mapcount;int referenced; -vm_flags_t vm_flags; +vma_flags_t vma_flags;struct mem_cgroup *memcg; };
@@ -926,7 +926,7 @@ static bool folio_referenced_one(struct folio *folio, address = pvmw.address; nr =1 ; -if (vma->vm_flags &VM_LOCKED ) { +if (vma_test (vma,VMA_LOCKED_BIT )) { ptes++; pra->mapcount--;
@@ -947,7 +947,7 @@ static bool folio_referenced_one(struct folio *folio,/* Restore the mlock which got missed */ mlock_vma_folio (folio, vma);page_vma_mapped_walk_done (&pvmw); - pra->vm_flags |=VM_LOCKED ; +vma_flags_set (&pra->vma_flags,VMA_LOCKED_BIT );return false;/* To break the loop */ }
@@ -1015,8 +1015,11 @@ static bool folio_referenced_one(struct folio *folio, referenced++;if (referenced) { +vma_flags_t vma_flags = vma->flags; + pra->referenced++; - pra->vm_flags |= vma->vm_flags & ~VM_LOCKED ; +vma_flags_clear (&vma_flags,VMA_LOCKED_BIT ); +vma_flags_set_mask (&pra->vma_flags, vma_flags); }if (!pra->mapcount)
@@ -1054,7 +1057,7 @@ static bool invalid_folio_referenced_vma(struct vm_area_struct *vma, void *arg)* @folio: The folio to test. * @is_locked: Caller holds lock on the folio. * @memcg: target memory cgroup -* @vm_flags: A combination of all the vma->vm_flags which referenced the folio. +* @vma_flags: A combination of all the vma->flags which referenced the folio. * * Quick test_and_clear_referenced for all mappings of a folio, *
@@ -1062,7 +1065,7 @@ static bool invalid_folio_referenced_vma(struct vm_area_struct *vma, void *arg)* the function bailed out due to rmap lock contention. */ int folio_referenced (struct folio *folio,int is_locked, -struct mem_cgroup *memcg,vm_flags_t *vm_flags) +struct mem_cgroup *memcg,vma_flags_t *vma_flags) {bool we_locked = false;struct folio_referenced_arg pra = {
@@ -1078,7 +1081,7 @@ int folio_referenced(struct folio *folio, int is_locked, };VM_WARN_ON_ONCE_FOLIO (folio_is_zone_device (folio), folio); - *vm_flags =0 ; +vma_flags_clear_all (vma_flags);if (!pra.mapcount)return 0 ;
@@ -1092,7 +1095,7 @@ int folio_referenced(struct folio *folio, int is_locked, }rmap_walk (folio, &rwc); - *vm_flags = pra.vm_flags; +vma_flags_set_mask (vma_flags, pra.vma_flags);if (we_locked)folio_unlock (folio);
mm/vmscan.c +7 -7
@@ -861,16 +861,16 @@ static enum folio_references folio_check_references(struct folio *folio, struct scan_control *sc) {int referenced_ptes, referenced_folio; -vm_flags_t vm_flags; +vma_flags_t vma_flags; referenced_ptes =folio_referenced (folio,1 , sc->target_mem_cgroup, - &vm_flags); + &vma_flags);/* * The supposedly reclaimable folio was found to be in a VM_LOCKED vma. * Let the folio, now marked Mlocked, be moved to the unevictable list. */ -if (vm_flags &VM_LOCKED ) +if (vma_flags_test (&vma_flags,VMA_LOCKED_BIT ))return FOLIOREF_ACTIVATE ;/*
@@ -914,7 +914,7 @@ static enum folio_references folio_check_references(struct folio *folio,/* * Activate file-backed executable folios after first usage. */ -if ((vm_flags &VM_EXEC ) &&folio_is_file_lru (folio)) +if (vma_flags_test (&vma_flags,VMA_EXEC_BIT ) &&folio_is_file_lru (folio))return FOLIOREF_ACTIVATE ;return FOLIOREF_KEEP ;
@@ -2065,7 +2065,7 @@ static void shrink_active_list(unsigned long nr_to_scan, {unsigned long nr_taken;unsigned long nr_scanned; -vm_flags_t vm_flags; +vma_flags_t vma_flags;LIST_HEAD (l_hold);/* The folios which were snipped off */ LIST_HEAD (l_active);LIST_HEAD (l_inactive);
@@ -2109,7 +2109,7 @@ static void shrink_active_list(unsigned long nr_to_scan,/* Referenced or rmap lock contention: rotate */ if (folio_referenced (folio,0 , sc->target_mem_cgroup, - &vm_flags) !=0 ) { + &vma_flags) !=0 ) {/* * Identify referenced, file-backed active folios and * give them one more trip around the active list. So
@@ -2119,7 +2119,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 ((vm_flags &VM_EXEC ) &&folio_is_file_lru (folio)) { +if (vma_flags_test (&vma_flags,VMA_EXEC_BIT ) &&folio_is_file_lru (folio)) { nr_rotated +=folio_nr_pages (folio);list_add (&folio->lru, &l_active);continue ;
Signature · 1 line
2.47.3
Baolin Wang
reviewerPATCH v3 2/2 mm: mglru: promote mapped executable folios after first usage
In reply to Baolin WangClassical 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.
--- mm/vmscan.c | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-)
mm/vmscan.c +31 -16
@@ -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 ();
Signature · 1 line
2.47.3
Baolin Wang
reviewerv3 cover promote mapped executable folios after first usage for MGLRU
Now MGLRU's protection of mapped executable file folios is less reliable. Follow the classical LRU's logic, promoting mapped executable file folios after their first usage to give executable code a better chance to stay in memory and improve workload performance (See patch 2 for more details).
Changes from v2:
https://lore.kernel.org/all/[email protected]/
- Add acked tag from David. Thanks.
- Rename the exec-file-folio helper and move it into vmscan.c (per David).
Changes from v1:
https://lore.kernel.org/all/4b921ed528c483e13c9e22d1ae44ba58b4a15b0b.1784096432.git.baolin.wang@linux.alibaba.com/
- Add a new patch to clean up vma flags as preparation.
- Add a new helper to check exec file folios (per Kaisui).
- Promote exec file folios in lru_gen_set_refs (Per Sashiko).
Baolin Wang (2):
mm: vmscan: convert folio_referenced() to use vma_flags_t
mm: mglru: promote mapped executable folios after first usage
include/linux/rmap.h | 7 +++--- mm/rmap.c | 19 ++++++++------- mm/vmscan.c | 57 ++++++++++++++++++++++++++++---------------- 3 files changed, 50 insertions(+), 33 deletions(-)
Signature · 1 line
2.47.3
Re: [PATCH v3 1/2] mm: vmscan: convert folio_referenced() to use vma_flags_t
In reply to Baolin WangQuoted 8 lines · Baolin Wang
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues forfolio_referenced()and related logic. This is also a preparation for the following changes.No functional changes.
Signed-off-by Baolin Wang <[email protected]>
Had to double take on the ors and ands and nots conversions, but looks correct to me.
Re: [PATCH v3 2/2] mm: mglru: promote mapped executable folios after first usage
In reply to Baolin WangQuoted 22 lines · Baolin Wang
Classical LRU protects mapped executable file folios through commit
8cab4754d24a0("vmscan: make mapped executable pages the first class citizen") and commitc909e99364c8("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 bylru_gen_look_around()orwalk_mm(). Additionally,folio_update_gen()orlru_gen_set_refs()only sets the 'PG_referenced' flag for mapped executable file folios, which causesshrink_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()andlru_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.
Quoted 7 lines
base patched
9248.543s 7861.579sWhile we are at it, introduce a new helper to check mapped executable file folios.
Signed-off-by Baolin Wang <[email protected]>
Both patches contained herein look good:
[ 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. ]
Quoted 180 lines
--- mm/vmscan.c | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-)mm/vmscan.c +31
-16@@ -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 ();Signature · 1 line
2.47.3
Baolin Wang
reviewerRe: [PATCH v3 2/2] mm: mglru: promote mapped executable folios after first usage
Quoted 29 lines · Johannes Weiner
Quoted 28 lines · Baolin Wang
Classical LRU protects mapped executable file folios through commit
8cab4754d24a0("vmscan: make mapped executable pages the first class citizen") and commitc909e99364c8("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 bylru_gen_look_around()orwalk_mm(). Additionally,folio_update_gen()orlru_gen_set_refs()only sets the 'PG_referenced' flag for mapped executable file folios, which causesshrink_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()andlru_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.579sWhile we are at it, introduce a new helper to check mapped executable file folios.
Quoted 6 lines
Quoted 1 line
Signed-off-by Baolin Wang <[email protected]>Both patches contained herein look good:
Acked-by Johannes Weiner <[email protected]>Acked-by Johannes Weiner <[email protected]>
Thanks for reviewing.
Quoted 3 lines
[ 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. ]
Ha, sure. Sorry for mixing in a helper replacement. I'll split it out into a separate patch next time. :)
Quoted 5 lines · Baolin Wang
Now MGLRU's protection of mapped executable file folios is less reliable. Follow the classical LRU's logic, promoting mapped executable file folios after their first usage to give executable code a better chance to stay in memory and improve workload performance (See patch 2 for more details).
Both patches look correct to me, and the premise of the functional change makes sense. For what it's worth feel free to take (on both):
Quoted 24 lines
Changes from v2:
https://lore.kernel.org/all/[email protected]/
- Add acked tag from David. Thanks.
- Rename the exec-file-folio helper and move it intovmscan.c(per David).Changes from v1:
https://lore.kernel.org/all/4b921ed528c483e13c9e22d1ae44ba58b4a15b0b.1784096432.git.baolin.wang@linux.alibaba.com/
- Add a new patch to clean up vma flags as preparation.
- Add a new helper to check exec file folios (per Kaisui).
- Promote exec file folios inlru_gen_set_refs(Per Sashiko).Baolin Wang (2):
mm: vmscan: convertfolio_referenced()to usevma_flags_t
mm: mglru: promote mapped executable folios after first usageinclude/linux/rmap.h | 7 +++--- mm/rmap.c | 19 ++++++++------- mm/vmscan.c | 57 ++++++++++++++++++++++++++++---------------- 3 files changed, 50 insertions(+), 33 deletions(-)--
2.47.3
Re: [PATCH v3 1/2] mm: vmscan: convert folio_referenced() to use vma_flags_t
In reply to Baolin WangQuoted 9 lines · Baolin Wang
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues forfolio_referenced()and related logic. This is also a preparation for the following changes.No functional changes.
Signed-off-by Baolin Wang <[email protected]>
LGTM,
[...]
mm/rmap.c static bool folio_referenced_one(struct folio *folio, · line 1022
Quoted 10 lines
mm/rmap.c +4
-1@@ -1015,8 +1015,11 @@ static bool folio_referenced_one(struct folio *folio, referenced++;if (referenced) { +vma_flags_t vma_flags = vma->flags; + pra->referenced++; - pra->vm_flags |= vma->vm_flags & ~VM_LOCKED ; +vma_flags_clear (&vma_flags,VMA_LOCKED_BIT ); +vma_flags_set_mask (&pra->vma_flags, vma_flags);
I spent a couple of minutes tracing how vma->vm_flags ends up in pra, and it seems to be correct :-)
Best Regards
Barry
Re: [PATCH v3 1/2] mm: vmscan: convert folio_referenced() to use vma_flags_t
In reply to Baolin WangQuoted 7 lines · Baolin Wang
Replace use of the legacy
vm_flags_tflags withvma_flags_tvalues forfolio_referenced()and related logic. This is also a preparation for the following changes.No functional changes.
Quoted 8 lines
Signed-off-by Baolin Wang <[email protected]>--- include/linux/rmap.h | 7 +++---- mm/rmap.c | 19 +++++++++++-------- mm/vmscan.c | 14 +++++++------- 3 files changed, 21 insertions(+), 19 deletions(-)
Re: [PATCH v3 2/2] mm: mglru: promote mapped executable folios after first usage
In reply to Baolin WangQuoted 26 lines · Baolin Wang
Classical LRU protects mapped executable file folios through commit
8cab4754d24a0("vmscan: make mapped executable pages the first class citizen") and commitc909e99364c8("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 bylru_gen_look_around()orwalk_mm(). Additionally,folio_update_gen()orlru_gen_set_refs()only sets the 'PG_referenced' flag for mapped executable file folios, which causesshrink_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()andlru_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
Quoted 7 lines
While we are at it, introduce a new helper to check mapped executable file folios.
Signed-off-by Baolin Wang <[email protected]>--- mm/vmscan.c | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-)
Hi Baolin, thanks for the update, looks good to me with two nit picks:
Quoted 22 lines
mm/vmscan.c +6
-0@@ -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(). */
mm/vmscan.c enum folio_references { · line 852
Quoted 10 lines
mm/vmscan.c +5
-1-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; +
This somehow missed the PG_workingset below?
Quoted 49 lines
mm/vmscan.c +5
-5return 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);
mm/vmscan.c static int folio_update_gen(struct folio *folio, int gen) · line 3212
Quoted 7 lines
mm/vmscan.c +4
-0@@ -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; +
Will it be cleaner if we just:
/*
* See the comment on LRU_REFS_FLAGS, and we protect the executable
* parts to avoid typical IO thrashing from reclaiming.
*/
if (!folio_test_referenced(folio) && !folio_test_workingset(folio) &&
!is_exec_file_folio(folio, vma_flags)) {
set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced));
return -1;
}If we use same check above we also get the PG_workingset?
Baolin Wang
reviewerRe: [PATCH v3 2/2] mm: mglru: promote mapped executable folios after first usage
Quoted 63 lines · Kairui Song
Quoted 26 lines · Baolin Wang
Classical LRU protects mapped executable file folios through commit
8cab4754d24a0("vmscan: make mapped executable pages the first class citizen") and commitc909e99364c8("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 bylru_gen_look_around()orwalk_mm(). Additionally,folio_update_gen()orlru_gen_set_refs()only sets the 'PG_referenced' flag for mapped executable file folios, which causesshrink_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()andlru_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.579sQuoted 7 lines
While we are at it, introduce a new helper to check mapped executable file folios.
Signed-off-by Baolin Wang <[email protected]>--- mm/vmscan.c | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-)Hi Baolin, thanks for the update, looks good to me with two nit picks:
Quoted 26 lines
mm/vmscan.c +7
-1@@ -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 */
Quoted 8 lines
mm/vmscan.c enum folio_references { · line 852
Quoted 6 lines
mm/vmscan.c +4
-0if (!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; +This somehow missed the
PG_workingsetbelow?
I was following the original logic of this function, which calls folio_mark_accessed() before promoting. But after re-reading the comment on LRU_REFS_FLAGS:
"
* For folios accessed multiple times through page tables, folio_update_gen()
* from a page table walk or lru_gen_set_refs() from a rmap walk sets
* PG_referenced after the accessed bit is cleared for the first time.
* Thereafter, those two paths set PG_workingset and promote folios to the
* youngest generation. Like folio_inc_gen(), folio_update_gen() also
clears
* PG_referenced. Note that for this case, LRU_REFS_MASK is not used.
"
I agree that I should set PG_workingset and clear LRU_REFS_FLAGS before promoting the executable file folios. And sashiko[1] also pointed this out. So I'll change it.
[1] https://sashiko.dev/#/patchset/cover.1784197559.git.baolin.wang%40linux.alibaba.com
Additionally, I think Barry's earlier patch[2] also has an issue: when lru_gen_set_refs() returns true to promote the folio, we should also clear LRU_REFS_FLAGS, rather than calling folio_mark_accessed(). So in my opinion, when we call folio_mark_accessed(), we should return false and should not promote the folio, so the logic shoule be:
static bool lru_gen_set_refs(struct folio *folio)
{
/* 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));
return false;
}
/* Promote on second access */
if (folio_lru_refs(folio) > 1) {
set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS, BIT(PG_workingset));
return true;
}
folio_mark_accessed(folio);
return false;
}What do you think? I'd like to send a fix first to correct the logic here.
[2] https://lore.kernel.org/linux-mm/[email protected]/
Quoted 64 lines
Quoted 49 lines
mm/vmscan.c +5
-5return 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);mm/vmscan.c static int folio_update_gen(struct folio *folio, int gen) · line 3212
Quoted 7 lines
mm/vmscan.c +4
-0@@ -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; +Will it be cleaner if we just:
/*
* See the comment onLRU_REFS_FLAGS, and we protect the executable
* parts to avoid typical IO thrashing from reclaiming.
*/
if (!folio_test_referenced(folio) && !folio_test_workingset(folio) &&
Quoted 4 lines
!is_exec_file_folio(folio, vma_flags)) {
set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced));
return -1;
}Yes. Much cleaner. Will do. Thanks for reviewing.
Baolin Wang
reviewerRe: [PATCH v3 2/2] mm: mglru: promote mapped executable folios after first usage
Quoted 76 lines · Baolin Wang
Quoted 73 lines · Kairui Song
Quoted 26 lines · Baolin Wang
Classical LRU protects mapped executable file folios through commit
8cab4754d24a0("vmscan: make mapped executable pages the first class citizen") and commitc909e99364c8("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 bylru_gen_look_around()orwalk_mm(). Additionally,folio_update_gen()orlru_gen_set_refs()only sets the 'PG_referenced' flag for mapped executable file folios, which causesshrink_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()andlru_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.579sQuoted 7 lines
While we are at it, introduce a new helper to check mapped executable file folios.
Signed-off-by Baolin Wang <[email protected]>---
mm/vmscan.c| 47 +++++++++++++++++++++++++++++++----------------1 file changed, 31 insertions(+), 16 deletions(-)Hi Baolin, thanks for the update, looks good to me with two nit picks:
Quoted 36 lines
mm/vmscan.c +6
-0@@ -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) {mm/vmscan.c +2
-1@@ -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 firstusage. */
+ if (is_exec_file_folio(folio, vma_flags)) + return true;
+
[snip]
Quoted 17 lines
Additionally, I think Barry's earlier patch[2] also has an issue: when
lru_gen_set_refs()returns true to promote the folio, we should also clearLRU_REFS_FLAGS, rather than callingfolio_mark_accessed(). So in my opinion, when we callfolio_mark_accessed(), we should return false and should not promote the folio, so the logic shoule be:static bool
lru_gen_set_refs(struct folio *folio)
{
/* see the comment onLRU_REFS_FLAGS*/
if (!folio_test_referenced(folio) && !folio_test_workingset(folio)) {set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced)); return false;
}/* Promote on second access */
if (folio_lru_refs(folio)> 1) { set_mask_bits(&folio->flags.f,LRU_REFS_FLAGS,BIT(PG_workingset));
return true;
}
Quoted 5 lines
folio_mark_accessed(folio);
return false;
}What do you think? I'd like to send a fix first to correct the logic here.
I tested this fix for Barry's patch and see some performance regression. I'll think more about how to fix it.