Re: [PATCH v3 1/2] memcg: reduce cacheline contention between lruvec stat updates and LRU lists

[Michal Hocko <mhocko@suse.com>]

On Fri 23-10-26 12:54:16, Joanne Koong wrote:
> Every memcg lruvec (per-node) stat update reads the memcg and
> lruvec_stats_percpu pointers in the first 64 bytes of struct
> mem_cgroup_per_node. These updates are extremely frequent (eg these
> updates happen several times for every page that's faulted in, freed,
> dirtied, or written back).
> 
> The memcg_pn_read_mostly cacheline group keeps those read-mostly fields
> on a different cacheline from the lruvec, but Intel CPUs do spatial
> prefetching [1] and fetch cachelines in 128-byte aligned pairs.
> Depending on where struct mem_cgroup_per_node lands in its slab, that
> cacheline can be paired with the cacheline holding the LRU list heads.
> 
> When this happens, every stat update also pulls in a copy of the LRU
> list heads cacheline, which is written to constantly by other CPUs under
> the lru_lock. The stat update has to fetch it from another CPU, which is
> unnecessary overhead since the stat update doesn't use the LRU list
> heads at all. But even worse, the lru_lock holder then has to invalidate
> those copies and wait for the acknowledgements to come back before it
> can do its next write, which makes the lru_lock critical section longer
> for everyone waiting on the lock.
> 
> This was found when analyzing the output of perf c2c for a benchmark
> where many threads fault in pages in one memcg. It showed the first 64
> bytes of mem_cgroup_per_node as the most contended cacheline (over a
> third of all HITMs and 5x more than the lru_lock cacheline) even though
> there are no writes to it. A quick experiment adding extra padding so
> that the cacheline no longer shared a pairing with the LRU list heads
> showed that this removed most of the contention, which confirmed the
> cause.
> 
> Fix this by moving the memcg_pn_cold group, which is only touched when
> the memcg is reparented or freed, right after memcg_pn_read_mostly, so
> that the read-mostly cacheline is paired with the cold cacheline
> instead of the LRU list heads.
> 
> Results from a multi-threaded fault/free benchmark in a single memcg,
> run on an Intel Skylake machine (80 threads, 2 NUMA nodes) with threads
> pinned to CPUs showed average pages faulted in (and freed) per second
> across all threads, over 7 freshly created memcgs per run:
> 
>                      before    after
>   shmem, 20 threads  4.74M     5.59M   (+18%)
>   shmem, 80 threads  7.94M     8.66M   (+9%)
>   anon,  20 threads  3.04M     3.08M   (+1%)
> 
> [1] https://www.intel.com/content/dam/doc/manual/64-ia-32-architectures-optimization-manual.pdf
> (bottom of page 62/800)
> 
> Link: https://lore.kernel.org/linux-fsdevel/CAJnrk1ZezmmzrYdf8KP6re_a76izMS=qGPeC8ZZXiteHyZZC-Q@mail.gmail.com/
> Suggested-by: Shakeel Butt <shakeel.butt@linux.dev>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>

Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!

> ---
>  include/linux/memcontrol.h | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 69e1bcc67c58..6139195b7fa4 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -95,6 +95,24 @@ struct mem_cgroup_per_node {
>  
>  	__cacheline_group_end_aligned(memcg_pn_read_mostly);
>  
> +	/*
> +	 * Touched only when the memcg is reparented or freed. Placed right
> +	 * after memcg_pn_read_mostly because CPUs with spatial prefetching
> +	 * (eg most Intel CPUs) fetch cachelines in 128-byte aligned pairs.
> +	 * This keeps the read-mostly line, which every lruvec stat update
> +	 * reads, from being paired with the LRU list heads, which are written
> +	 * constantly under lru_lock.
> +	 */
> +	__cacheline_group_begin_aligned(memcg_pn_cold);
> +	/*
> +	 * orig_objcg preserves a pointer (and a reference) to the original
> +	 * objcg until the end of life of memcg.
> +	 */
> +	struct obj_cgroup	*orig_objcg;
> +	/* list of inherited objcgs, protected by objcg_lock */
> +	struct list_head objcg_list;
> +	__cacheline_group_end_aligned(memcg_pn_cold);
> +
>  	/*
>  	 * Keep lruvec on its own lines. Sharing them with lru_zone_size[]
>  	 * regressed, see commit f59adcf59332 ("mm: memcg: add cacheline
> @@ -114,17 +132,6 @@ struct mem_cgroup_per_node {
>  	atomic_t		slab_unreclaimable;
>  #endif
>  	__cacheline_group_end_aligned(memcg_pn_write_hot);
> -
> -	/* Touched only when the memcg is reparented or freed. */
> -	__cacheline_group_begin_aligned(memcg_pn_cold);
> -	/*
> -	 * orig_objcg preserves a pointer (and a reference) to the original
> -	 * objcg until the end of life of memcg.
> -	 */
> -	struct obj_cgroup	*orig_objcg;
> -	/* list of inherited objcgs, protected by objcg_lock */
> -	struct list_head objcg_list;
> -	__cacheline_group_end_aligned(memcg_pn_cold);
>  };
>  
>  struct mem_cgroup_threshold {
> -- 
> 2.52.0

-- 
Michal Hocko
SUSE Labs