From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mta1.migadu.com (out-83.mta1.migadu.com [95.215.58.83]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B566319D07E for ; Wed, 26 Aug 2026 08:23:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.83 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787732631; cv=none; b=bj11eRWfmWEJG6PFjZ/KzrtKaQ1+0MH8im+qIe7VuWyWJFHPbCWCh0wDQ70mG5LksFiny3D6UhVg7nQQrZTq4kuOltjHkm1JPjsJiPSaeftH+YxH2rNCSt5B51cp91HlV6CeWFteWai3pLM1Dv2InBCrRwQaxahhHYr9KU9yIOc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787732631; c=relaxed/simple; bh=phYg0IFEYWVrpDdbQ98Z9cebBiSYeTfZKyJMdv6V2gs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=atyLQwazo3ABRzsLguqRw1Axl9r78epTWzl6oZ1oczJBIPmd+BQjzvsBOq+ppHLdcuyZxySrXvMJOFeaEu1jKBNNlJO3so0pBTgcHAX4LBWJLwvjPST5RzB8Ij3ZtWumu/O5CqVWkFu2i09jB3lQwIh8gA9Q++xgpLgLP2oAwkI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=pGVrpfAx; arc=none smtp.client-ip=95.215.58.83 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="pGVrpfAx" X-Envelope-To: linux-kernel@vger.kernel.org DKIM-Signature: a=rsa-sha256; bh=phYg0IFEYWVrpDdbQ98Z9cebBiSYeTfZKyJMdv6V2gs=; c=simple/simple; d=linux.dev; h=from:to:subject:date:message-id:mime-version:content-type; s=key1; t=1787732626; v=1; x=1788337426; b=pGVrpfAxNuTaD/Om83RMN3gQMIzXMtUE5r0zLTCW12HTM26NYyzdoR8r7Dd8R8nCDtw6Dffn 1fFHUkNOnLzVEHN5r7ryDVxw+rhznt+umM2eb1kYy61q1jJ2h30sXo7VofPS9PzV/LzqHqbJUFj Koj8tOVF++TuNRdQg0siWUGc= X-Envelope-To: linux-kernel@vger.kernel.org Received: from localhost (223.70.159.239) by mta12.migadu.com with ESMTPS id c55063d47d246f4e; Wed, 26 Aug 2026 08:23:46 +0000 X-Mizu-Trace-ID: c55063d47d246f4e X-Migadu-Flow: FLOW_OUT Date: Wed, 26 Aug 2026 16:23:34 +0800 From: Baoquan He To: "Barry Song (Xiaomi)" Cc: akpm@linux-foundation.org, linux-mm@kvack.org, axelrasmussen@google.com, baolin.wang@linux.alibaba.com, chenridong@xiaomi.com, david@kernel.org, hannes@cmpxchg.org, kasong@tencent.com, lianux.mm@gmail.com, linux-kernel@vger.kernel.org, ljs@kernel.org, lyugaofei@xiaomi.com, mhocko@kernel.org, qi.zheng@linux.dev, shakeel.butt@linux.dev, stevensd@chromium.org, wangzicheng@honor.com, weixugc@google.com, yuanchu@google.com, zhangbo56@xiaomi.com Subject: Re: [PATCH 1/6] mm/mglru: batch update lrugen->nr_pages in inc_min_seq() Message-ID: References: <20260821102538.22642-1-baohua@kernel.org> <20260821102538.22642-2-baohua@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260821102538.22642-2-baohua@kernel.org> Hi Barry, On 08/21/26 at 06:25pm, Barry Song (Xiaomi) wrote: > Currently, folio_inc_gen() updates lrugen->nr_pages for every folio > as it advances generations. Instead, accumulate the size changes > and update lrugen->nr_pages in a batch after scanning the entire > oldest generation, or when the scan stops because remaining reaches > zero. This patch refactor code to split out __folio_inc_gen() and introduce gen_increased, this is the base of later patches. You seem to only mention the minor optimization of lrugen->nr_pages. IMHO, this refactoring can be split out to an independent patch. The lrugen->nr_pages can be integrated with other patch, e.g patch 2 or patch 6. > > Since we only move folios from the oldest generation to the second > oldest generation, the active/inactive state cannot change. We can > therefore skip __lru_update_size(). For the justification of skipping __lru_update_size(), there's a precondition: get_nr_gens(lruvec, type) == MAX_NR_GENS; With this, the oldest gen and 2nd oldest gen are both inactive. Calling __lru_update_size() may waste tiny cpu, while skipping it may cause issue in future if the precondition is changed. Do you think adding a VM_WARN_ON_ONCE is necessary? VM_WARN_ON_ONCE(lru_gen_is_active(lruvec, old_gen) || lru_gen_is_active(lruvec, target_gen)); Thanks Baoquan > > Signed-off-by: Barry Song (Xiaomi) > --- > mm/vmscan.c | 46 +++++++++++++++++++++++++++++++++++----------- > 1 file changed, 35 insertions(+), 11 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index c1404a59523d..0d74fc00abd3 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -3296,20 +3296,21 @@ static int folio_update_gen(struct folio *folio, int gen, const vma_flags_t *vma > } > > /* protect pages accessed multiple times through file descriptors */ > -static int folio_inc_gen(struct lruvec *lruvec, struct folio *folio) > +static int __folio_inc_gen(struct folio *folio, int old_gen, bool *increased) > { > - int type = folio_is_file_lru(folio); > - struct lru_gen_folio *lrugen = &lruvec->lrugen; > - int new_gen, old_gen = lru_gen_from_seq(lrugen->min_seq[type]); > unsigned long new_flags, old_flags = READ_ONCE(folio->flags.f); > + int new_gen; > > VM_WARN_ON_ONCE_FOLIO(!(old_flags & LRU_GEN_MASK), folio); > > do { > new_gen = ((old_flags & LRU_GEN_MASK) >> LRU_GEN_PGOFF) - 1; > /* folio_update_gen() has promoted this page? */ > - if (new_gen >= 0 && new_gen != old_gen) > + if (new_gen >= 0 && new_gen != old_gen) { > + if (increased) > + *increased = false; > return new_gen; > + } > > new_gen = (old_gen + 1) % MAX_NR_GENS; > > @@ -3317,8 +3318,21 @@ static int folio_inc_gen(struct lruvec *lruvec, struct folio *folio) > new_flags |= (new_gen + 1UL) << LRU_GEN_PGOFF; > } while (!try_cmpxchg(&folio->flags.f, &old_flags, new_flags)); > > - lru_gen_update_size(lruvec, folio, old_gen, new_gen); > + if (increased) > + *increased = true; > + return new_gen; > +} > > +static int folio_inc_gen(struct lruvec *lruvec, struct folio *folio) > +{ > + int type = folio_is_file_lru(folio); > + struct lru_gen_folio *lrugen = &lruvec->lrugen; > + int new_gen, old_gen = lru_gen_from_seq(lrugen->min_seq[type]); > + bool gen_increased; > + > + new_gen = __folio_inc_gen(folio, old_gen, &gen_increased); > + if (gen_increased) > + lru_gen_update_size(lruvec, folio, old_gen, new_gen); > return new_gen; > } > > @@ -3904,6 +3918,7 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness) > struct lru_gen_folio *lrugen = &lruvec->lrugen; > int hist = lru_hist_from_seq(lrugen->min_seq[type]); > int new_gen, old_gen = lru_gen_from_seq(lrugen->min_seq[type]); > + int target_gen = (old_gen + 1) % MAX_NR_GENS; > > /* For file type, skip the check if swappiness is anon only */ > if (type && (swappiness == SWAPPINESS_ANON_ONLY)) > @@ -3916,32 +3931,41 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness) > /* prevent cold/hot inversion if the type is evictable */ > for (zone = 0; zone < MAX_NR_ZONES; zone++) { > struct list_head *head = &lrugen->folios[old_gen][type][zone]; > + unsigned long delta = 0; > > while (!list_empty(head)) { > struct folio *folio = lru_to_folio(head); > + long nr_pages = folio_nr_pages(folio); > int refs = folio_lru_refs(folio); > bool workingset = folio_test_workingset(folio); > + bool gen_increased; > > VM_WARN_ON_ONCE_FOLIO(folio_test_unevictable(folio), folio); > VM_WARN_ON_ONCE_FOLIO(folio_test_active(folio), folio); > VM_WARN_ON_ONCE_FOLIO(folio_is_file_lru(folio) != type, folio); > VM_WARN_ON_ONCE_FOLIO(folio_zonenum(folio) != zone, folio); > > - new_gen = folio_inc_gen(lruvec, folio); > + new_gen = __folio_inc_gen(folio, old_gen, &gen_increased); > list_move_tail(&folio->lru, &lrugen->folios[new_gen][type][zone]); > - > + if (gen_increased) > + delta += nr_pages; > /* don't count the workingset being lazily promoted */ > if (refs + workingset != BIT(LRU_REFS_WIDTH) + 1) { > int tier = lru_tier_from_refs(refs, workingset); > - int delta = folio_nr_pages(folio); > > WRITE_ONCE(lrugen->protected[hist][type][tier], > - lrugen->protected[hist][type][tier] + delta); > + lrugen->protected[hist][type][tier] + nr_pages); > } > > if (!--remaining) > - return false; > + break; > } > + WRITE_ONCE(lrugen->nr_pages[old_gen][type][zone], > + lrugen->nr_pages[old_gen][type][zone] - delta); > + WRITE_ONCE(lrugen->nr_pages[target_gen][type][zone], > + lrugen->nr_pages[target_gen][type][zone] + delta); > + if (!remaining) > + return false; > } > done: > reset_ctrl_pos(lruvec, type, true); > -- > 2.34.1 >