From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-110.freemail.mail.aliyun.com (out30-110.freemail.mail.aliyun.com [115.124.30.110]) (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 797173D5253 for ; Tue, 31 Mar 2026 09:52:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.110 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774950768; cv=none; b=onnZSCVTHdFcyCmTcR/IL+BcwByoCpQUXcgUPzUVOMOW5uXuGMmS+qkD0hnpOLNOYZSTxTx3nCxUfeSTlilBgvemdBt41lae3i0tVzgGDnq5BBc3kz5+kt0EpEsWFAkzXJT4Ynt3ZwIldaQmLdxBnWhUd2ZdtTn753df6i6DsVI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774950768; c=relaxed/simple; bh=rFkxLCSIhkWE0ea6ByQDeVrjQQAmDowtJ6z7b/YFwWs=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=SPPFdhP+6Ag15NolYLgPys9pWld/w0koVNwdILQ45PPI7AGJ5fIG77rNAX1LUBP6a719FB+b3S7pbvZoejgdL2i4pWMu9FXCUSmnfENgW5HADqh0HL+S2JClPFMNBHnsblCL57ESYHGuBgw/1kLI1FmGP/LsJR7hwcbe6Kx9/pg= 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=F2lJtOQ8; arc=none smtp.client-ip=115.124.30.110 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="F2lJtOQ8" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1774950763; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=6C51cv3GmInVDRSzJ0o1SrLSNCpx+iw3NqPxrMXCQW0=; b=F2lJtOQ8IaWU1UZUZSeZk0Fiu2amRwKI8SBl4xZ/tc3SFFrpEUok6PU7Dgqzt3/j5g78cyXaWz7EMGkIKjGTNaYrdeLC0ApJxqzuydmCzTV5/2a2X99O4x5+7Y9z7AEI5w30Sdp0QOx0mT/ts1T9yhA7Vl9MCSCoNMiL73/Ikeg= X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R191e4;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=26;SR=0;TI=SMTPD_---0X03ykpe_1774950760; Received: from 30.74.144.129(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0X03ykpe_1774950760 cluster:ay36) by smtp.aliyun-inc.com; Tue, 31 Mar 2026 17:52:41 +0800 Message-ID: <51d646bf-87f0-4f56-892e-4c62940458a5@linux.alibaba.com> Date: Tue, 31 Mar 2026 17:52:40 +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 v2 05/12] mm/mglru: scan and count the exact number of folios To: Kairui Song Cc: kasong@tencent.com, linux-mm@kvack.org, Andrew Morton , Axel Rasmussen , Yuanchu Xie , Wei Xu , Johannes Weiner , David Hildenbrand , Michal Hocko , Qi Zheng , Shakeel Butt , Lorenzo Stoakes , Barry Song , David Stevens , Chen Ridong , Leno Hou , Yafang Shao , Yu Zhao , Zicheng Wang , Kalesh Singh , Suren Baghdasaryan , Chris Li , Vernon Yang , linux-kernel@vger.kernel.org, Qi Zheng References: <20260329-mglru-reclaim-v2-0-b53a3678513c@tencent.com> <20260329-mglru-reclaim-v2-5-b53a3678513c@tencent.com> <3ef1cb78-5110-4326-bde1-d929f638b8f6@linux.alibaba.com> From: Baolin Wang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 3/31/26 5:01 PM, Kairui Song wrote: > On Tue, Mar 31, 2026 at 04:04:30PM +0800, Baolin Wang wrote: >> >> >> On 3/29/26 3:52 AM, Kairui Song via B4 Relay wrote: >>> From: Kairui Song >>> >>> Make the scan helpers return the exact number of folios being scanned >>> or isolated. Since the reclaim loop now has a natural scan budget that >>> controls the scan progress, returning the scan number directly should >>> make the scan more accurate and easier to follow. >>> >>> The number of scanned folios for each iteration is always positive and >>> larger than 0, unless the reclaim must stop for a forced aging, so >>> there is no more need for any special handling when there is no >>> progress made: >>> >>> - `return isolated || !remaining ? scanned : 0` in scan_folios: both >>> the function and the call now just return the exact scan count, >>> combined with the scan budget introduced in the previous commit to >>> avoid livelock or under scan. >> >> Make sense to me. >> >>> >>> - `scanned += try_to_inc_min_seq` in evict_folios: adding a bool as a >>> scan count was kind of confusing and no longer needed too, as scan >>> number will never be zero even if none of the folio in oldest >>> generation is isolated. >> >> Yes, agree. >> >>> >>> - `evictable_min_seq + MIN_NR_GENS > max_seq` guard in evict_folios: >>> the per-type get_nr_gens == MIN_NR_GENS check in scan_folios >>> naturally returns 0 when only two gens remain and breaks the loop. >>> >>> Also move try_to_inc_min_seq before isolate_folios, so that any empty >>> gens created by external folio freeing are also skipped. >> >> This part is somewhat confusing. You probably mean the case where the list >> of that gen becomes empty via isolate_folio(), right? >> >> If that's the case, the original logic would remove the empty gens produced >> by isolate_folio() after calling try_to_inc_min_seq(). >> >> However, with your changes, this removal won't happen until the next >> eviction. Does this provide any additional benefits? Or could you describe >> how this change impacts your testing? > > Hi Baolin, thanks for the review. > > Yeah, I also notices this issue after sending this while doing more > self review. > > So I did some test with the patch below: > > static bool inc_max_seq(struct lruvec *lruvec, unsigned long seq, int swappiness) > @@ -4818,11 +4814,15 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec, > > lruvec_lock_irq(lruvec); > > + /* In case folio deletion created empty gen, flush them */ > try_to_inc_min_seq(lruvec, swappiness); > > scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness, > &list, &isolated, &type, &type_scanned); > > + /* Isolation might created empty gen, flush them */ > + try_to_inc_min_seq(lruvec, swappiness); > + > lruvec_unlock_irq(lruvec); > > if (list_empty(&list)) > > The return value of try_to_inc_min_seq can also be dropped > since it's no longer used, and the function call should be cheap. > > After system time of build kernel using 3G memory and make -j96 > with ZRAM as swap, system time in seconds average of 12 test run each: > > mm-new: > 9136.055833 > > After V2: > 8819.932222 > > After V2, with above patch: > 8783.944444 > > After V2, without above patch but move try_to_inc_min_seq > back to after isolate_folios: > 8807.874444 > > This series is looking good, this inc_min change seems trivial > but in theory it does have have real effect. > > - Moving the try_to_inc_min_seq after isolate_folios may result in a > wasted isolate_folios call and early abort of reclaim loop if there > is a stalled oldest gen created by folio deletion. Indeed. > - Moving the try_to_inc_min_seq before isolate_folios may leave a > empty gen after isolation. Usually it's fine because next eviction > will still reclaim them. But before next eviction, during that period, > new file folios could be added the oldest gen and get reclaim too > early. That looks a real problem. > > This maybe trivial since MGLRU itself also may suffer the same > problem when the oldest gen is just too short, that's a much more > common case (For this short oldest gen issue we can solve later). > > - Having try_to_inc_min_seq both before and after isolate_folios > seems the best choice here and somehow matches the benchmark > result above, very close to the noise level though. > > Well I only tested one cases, the cover letter described a > larger matrix, still all good with this series and I'm not > 100% sure how this particular change effects them, I guess > it's still trivial. > > The try_to_inc_min_seq call should be cheap enough since it's > called only for one batch of 64 folios, and it's only reading > a few lists for the non inc path. > > How do you think that we just call it twice here? Sounds reasonable to me. I'm not sure if we need to split out a new patch with adding above message, as this patch mainly focuses on optimizing the number of folios being scanned.