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

[Johannes Weiner <hannes@cmpxchg.org>]

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?

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.

Thanks