Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220309021230.721028-1-yuzhao@google.com> <20220309021230.721028-7-yuzhao@google.com>
 <877d8m7e1x.fsf@linux.ibm.com>
In-Reply-To: <877d8m7e1x.fsf@linux.ibm.com>
From:   Yu Zhao <yuzhao@google.com>
Date:   Mon, 21 Mar 2022 23:55:45 -0600
Message-ID: <CAOUHufbDfwgm8PgCGkhCjbhMbm=fekfjgRR56NL-j+5iUGfVuw@mail.gmail.com>
Subject: Re: [PATCH v9 06/14] mm: multi-gen LRU: minimal implementation
To:     "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Andi Kleen <ak@linux.intel.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Hillf Danton <hdanton@sina.com>, Jens Axboe <axboe@kernel.dk>,
        Jesse Barnes <jsbarnes@google.com>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Matthew Wilcox <willy@infradead.org>,
        Mel Gorman <mgorman@suse.de>,
        Michael Larabel <Michael@michaellarabel.com>,
        Michal Hocko <mhocko@kernel.org>,
        Mike Rapoport <rppt@kernel.org>,
        Rik van Riel <riel@surriel.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Will Deacon <will@kernel.org>,
        Ying Huang <ying.huang@intel.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Linux-MM <linux-mm@kvack.org>,
        Kernel Page Reclaim v2 <page-reclaim@google.com>,
        "the arch/x86 maintainers" <x86@kernel.org>,
        Brian Geffon <bgeffon@google.com>,
        Jan Alexander Steffens <heftig@archlinux.org>,
        Oleksandr Natalenko <oleksandr@natalenko.name>,
        Steven Barrett <steven@liquorix.net>,
        Suleiman Souhlal <suleiman@google.com>,
        Daniel Byrne <djbyrne@mtu.edu>,
        Donald Carr <d@chaos-reins.com>,
        =?UTF-8?Q?Holger_Hoffst=C3=A4tte?= <holger@applied-asynchrony.com>,
        Konstantin Kharlamov <Hi-Angel@yandex.ru>,
        Shuang Zhai <szhai2@cs.rochester.edu>,
        Sofia Trinh <sofia.trinh@edi.works>,
        Vaibhav Jain <vaibhav@linux.ibm.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Mar 21, 2022 at 11:27 PM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> Yu Zhao <yuzhao@google.com> writes:
>
> > +static void inc_max_seq(struct lruvec *lruvec, unsigned long max_seq)
> > +{
> > +     int prev, next;
> > +     int type, zone;
> > +     struct lru_gen_struct *lrugen = &lruvec->lrugen;
> > +
> > +     spin_lock_irq(&lruvec->lru_lock);
> > +
> > +     VM_BUG_ON(!seq_is_valid(lruvec));
> > +
> > +     if (max_seq != lrugen->max_seq)
> > +             goto unlock;
> > +
> > +     inc_min_seq(lruvec);
>
> Can this min seq update result in pages considered oldest become young.
> ie, if we had seq value of 0 - 3 and we need ageing, the new min seq and
> max_seq value will now become 1 - 4. What happens to pages in the
> generation value 0 which was oldest generation earlier and is youngest
> now.

If anon pages are not reclaimable, e.g., no swapfile, they won't be
scanned at all. So their coldness/hotness don't matter -- they don't
need to be on lrugen->lists[] at all.

If there is a swapfile but it's full, then yes, the inversion will
happen. This can be handled by moving pages from the oldest generation
to the tail of the second oldest generation, which maintains the LRU
order.

In fact, both were handled in the previous versions [1] [2]. They were
removed in v6 for simplicity.

[1] https://lore.kernel.org/linux-mm/20211111041510.402534-5-yuzhao@google.com/
[2] https://lore.kernel.org/linux-mm/20211111041510.402534-7-yuzhao@google.com/

> > +static int evict_folios(struct lruvec *lruvec, struct scan_control *sc, int swappiness)
> > +{
> > +     int type;
> > +     int scanned;
> > +     int reclaimed;
> > +     LIST_HEAD(list);
> > +     struct folio *folio;
> > +     enum vm_event_item item;
> > +     struct reclaim_stat stat;
> > +     struct mem_cgroup *memcg = lruvec_memcg(lruvec);
> > +     struct pglist_data *pgdat = lruvec_pgdat(lruvec);
> > +
> > +     spin_lock_irq(&lruvec->lru_lock);
> > +
> > +     scanned = isolate_folios(lruvec, sc, swappiness, &type, &list);
> > +
> > +     if (try_to_inc_min_seq(lruvec, swappiness))
> > +             scanned++;
>
> we are doing this before we shrink the page list. Any reason to do this before?

We have isolated pages from lrugen->lists[], and we might have
exhausted all pages in the oldest generations, i.e.,
lrugen->lists[min_seq] is now empty. Incrementing min_seq after
shrink_page_list() is not wrong. However, it's better we do it ASAP so
that concurrent reclaimers are less likely to see a stale min_seq and
come here under the false impression that they'd make some progress.
(Instead, they will go to the aging path and inc_max_seq() first
before coming here.)