Re: [PATCH v3 2/2] mm: mglru: promote mapped executable folios after first usage

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 7/19/26 9:45 AM, Baolin Wang wrote:
> 
> 
> On 7/19/26 3:23 AM, Kairui Song wrote:
>> On Fri, Jul 17, 2026 at 6:06 PM Baolin Wang
>> <baolin.wang@linux.alibaba.com> 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>
>>> ---
>>>   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:
>>>
>>> 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;
>>> +

[snip]

> 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.
I tested this fix for Barry's patch and see some performance regression. 
I'll think more about how to fix it.