Re: [RFC PATCH v2 1/5] workingset: simplify and use a more intuitive model

[Kairui Song <ryncsn@gmail.com>]

Johannes Weiner <hannes@cmpxchg.org> 于2023年9月13日周三 03:47写道：
>
> Hello Kairui,
>
> On Wed, Sep 13, 2023 at 02:45:07AM +0800, Kairui Song wrote:
> > @@ -226,8 +227,103 @@ static void unpack_shadow(void *shadow, int *memcgidp, pg_data_t **pgdat,
> >       *workingsetp = workingset;
> >  }
> >
> > -#ifdef CONFIG_LRU_GEN
> > +/*
> > + * Get the distance reading at eviction time.
> > + */
> > +static inline unsigned long lru_eviction(struct lruvec *lruvec,
> > +                                      int bits, int bucket_order)
> > +{
> > +     unsigned long eviction = atomic_long_read(&lruvec->nonresident_age);
> > +
> > +     eviction >>= bucket_order;
> > +     eviction &= ~0UL >> (BITS_PER_LONG - bits);
> > +
> > +     return eviction;
> > +}
> > +
> > +/*
> > + * Calculate and test refault distance
> > + */
> > +static inline bool lru_refault(struct mem_cgroup *memcg,
> > +                            struct lruvec *lruvec,
> > +                            unsigned long eviction, bool file,
> > +                            int bits, int bucket_order)
> > +{
>
> This patch changes the refault decision making at the same time as it
> moves the functions around in preparation for use by MGLRU. This makes
> it difficult to review the exact changes to the current algorithm.
>
> Can you please separate those two components?

Hi, thanks for the review.

>
> Step 1: Change the existing logic in its place to the new
> algorithm. Please also update the big comment on how everything works
> at the same time (IOW, merge patch #2 into this one).
>
> Step 2: Add a new patch to refactor for MGLRU right before the patch
> that will use those functions. As far as I can see, that's the last
> patch in the series. So the refactor should be second-to-last.
>

Good suggestion, I'll split these parts into individual patches and
merge the comments.

> Thanks