From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-113.freemail.mail.aliyun.com (out30-113.freemail.mail.aliyun.com [115.124.30.113]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B6DE243F08A for ; Sun, 19 Jul 2026 01:46:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.113 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784425565; cv=none; b=Fw9XQ+pCeGqFWhRpsOFfGxws120Tj3jpxfmexU9FpcCuhD+z1BBwkgXSoyXYr7Z2hGoShkqEroWhX3UiFaNj8tTEfQ5m8FFQlWA5VpTmgMCrSVer5okj0Ob9CAiOlL8QydaRwjoidEv5Yb2smn7XJQmCI/7aAA6zd/By8HqG2IM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784425565; c=relaxed/simple; bh=XqVmfHvs0Qswdln9DSJxeQH+YicFavl9XIrSs44YCmM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=NJPKYcxNf+2qwJWhgufYHLEFbikx/Uto76komwNfDUFy7vRADKjgKCmJecsB2ITEtjhPebRFYFgk+DvgyD+cQlNuHqIPVetCk7wkzqxrwuLUYf14/QBA6I0Kpif9+JClK7/s+UhPQddNf/LKehQpfY/TaYZJLryIuwFFGVlEGPU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=wM85+XG/; arc=none smtp.client-ip=115.124.30.113 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="wM85+XG/" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1784425553; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=npjWI4sepkXG3SfpWUN7QMm7qm2VcJzaC7SQ+AT6TP4=; b=wM85+XG/E+8+gGmYq8/a6vw1wA4R8ajT4X9LbjyQiOm5F0ha3tuDFXaGWKcJotmnaaCEL+dHrvSZSWqYKV8YyxVLL5cSvLXqomVfDDo4EJhkeCBeb45S8i2KVQt4elj4d4Mpi1r/0TlmIX4caMYePfpbwGjqxsZH1gawz6JFoqc= X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R641e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=maildocker-contentspam011083073210;MF=baolin.wang@linux.alibaba.com;NM=1;PH=DS;RN=20;SR=0;TI=SMTPD_---0X7KvgGE_1784425550; Received: from 30.120.38.205(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0X7KvgGE_1784425550 cluster:ay36) by smtp.aliyun-inc.com; Sun, 19 Jul 2026 09:45:52 +0800 Message-ID: Date: Sun, 19 Jul 2026 09:45:50 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 2/2] mm: mglru: promote mapped executable folios after first usage To: Kairui Song Cc: akpm@linux-foundation.org, david@kernel.org, ljs@kernel.org, hannes@cmpxchg.org, riel@surriel.com, liam@infradead.org, vbabka@kernel.org, harry@kernel.org, jannh@google.com, lance.yang@linux.dev, 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 References: <7db4d1975cf04e3e48157f98bdf398565ee189a4.1784268206.git.baolin.wang@linux.alibaba.com> From: Baolin Wang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 7/19/26 3:23 AM, Kairui Song wrote: > On Fri, Jul 17, 2026 at 6:06 PM 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 >> --- >> 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; >> + > > This somehow missed the PG_workingset below? 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/20260526130938.66253-1-baohua@kernel.org/ >> 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; >> + > > 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; > } Yes. Much cleaner. Will do. Thanks for reviewing.