Re: [PATCH 4/8] mm/mglru: scan and count the exact number of folios

[Chen Ridong <chenridong@huaweicloud.com>]

On 2026/3/24 16:05, Kairui Song wrote:
> On Tue, Mar 24, 2026 at 3:22 PM Chen Ridong <chenridong@huaweicloud.com> wrote:
>> On 2026/3/23 0:20, Kairui Song wrote:
>>> On Sat, Mar 21, 2026 at 4:59 AM Axel Rasmussen <axelrasmussen@google.com> wrote:
>>>>
>>>> On Tue, Mar 17, 2026 at 12:11 PM Kairui Song via B4 Relay
>>>> <devnull+kasong.tencent.com@kernel.org> wrote:
>>>>>
>>>>> From: Kairui Song <kasong@tencent.com>
>>>>>
>>>>> Make the scan helpers return the exact number of folios being scanned
>>>>> or isolated. This should make the scan more accurate and easier to
>>>>> follow.
>>>>>
>>>>> Now there is no more need for special handling when there is no
>>>>> progress made. The old livelock prevention `(return isolated ||
>>>>> !remaining ? scanned : 0)` is replaced by the natural scan budget
>>>>> exhaustion in try_to_shrink_lruvec, and sort_folio moves ineligible
>>>>> folios to newer generations.
>>>>>
>>>>> Signed-off-by: Kairui Song <kasong@tencent.com>
> 
> ...
> 
>>>>>  static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>>>>> @@ -4852,7 +4851,6 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>>>>>         struct reclaim_stat stat;
>>>>>         struct lru_gen_mm_walk *walk;
>>>>>         bool skip_retry = false;
>>>>> -       struct lru_gen_folio *lrugen = &lruvec->lrugen;
>>>>>         struct mem_cgroup *memcg = lruvec_memcg(lruvec);
>>>>>         struct pglist_data *pgdat = lruvec_pgdat(lruvec);
>>>>>
>>>>> @@ -4860,10 +4858,7 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>>>>>
>>>>>         scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness, &type, &list);
>>>>>
>>>>> -       scanned += try_to_inc_min_seq(lruvec, swappiness);
>>>>> -
>>>>> -       if (evictable_min_seq(lrugen->min_seq, swappiness) + MIN_NR_GENS > lrugen->max_seq)
>>>>> -               scanned = 0;
>>>>> +       try_to_inc_min_seq(lruvec, swappiness);
>>>>
>>>> IIUC, this change is what introduces the issue patch 6 is trying to
>>>> resolve. Is it worth squashing patch 6 in to this one, so we don't
>>>> have this non-ideal intermediate state?
>>>
>>> Well it's not, patch 6 is fixing an existing problem, see the cover
>>> letter about the OOM issue.
>>>
>>> This part of changing is just cleanup the loop code. It looks really
>>> strange to me that increasing min_seq is considered as scanning one
>>> folio. Aborting the scan if there is only 2 gen kind of make sense but
>>> this doesn't seems the right place. These strange parts to avoid
>>> livelock can be dropped since we have an exact count of folios being
>>> scanned now. I'll add more words in the commit message.
>>
>> This change confused me too.
>>
>> IIUC, this change looks conceptually tied to patch 3. The following change means
>> that evict_folios should not be invoked if aging is needed. So the judge can be
>> dropped there, right?
>>
>>
>> ```
>>  static bool try_to_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
>>  {
>> ...
>> +               if (should_run_aging(lruvec, max_seq, sc, swappiness)) {
>> +                       if (try_to_inc_max_seq(lruvec, max_seq, swappiness, false))
>> +                               need_rotate = true;
>> +                       break;
>> +               }
>> ```
>>
> 
> Hi Ridong,
> 
> Ahh yes, as you pointed out, the explicit should_run_aging kind of
> guards the evict_folio. That's not everything, besides, previously
> isolate_folios may return 0 if there is no folio isolated. But now it
> always return the number of folios being scanned, unless there are
> only two genes left and hit the force protection, which also makes the
> judge here can be dropped.
> 
> But not invoking evict_folios if aging is needed is an existing
> behavior, that commit (patch 3) didn't change it, just made it cleaner
> so we can see it well.
> 

Thanks for the explanation.

Would it be better to combine this change with patch 3, rather than adding to
the commit message?

-- 
Best regards,
Ridong