Re: [PATCH 01/11] cgroup: move rstat pointers into struct of their own

[Shakeel Butt <shakeel.butt@linux.dev>]

Thanks JP for awesome work. I am doing a quick first iteration and later
will do the deep review.

On Mon, Feb 17, 2025 at 07:14:38PM -0800, JP Kobryn wrote:
>  struct cgroup_freezer_state {
>  	/* Should the cgroup and its descendants be frozen. */
>  	bool freeze;
> @@ -517,23 +445,9 @@ struct cgroup {
>  	struct cgroup *old_dom_cgrp;		/* used while enabling threaded */
>  
>  	/* per-cpu recursive resource statistics */
> -	struct cgroup_rstat_cpu __percpu *rstat_cpu;
> +	struct cgroup_rstat rstat;
>  	struct list_head rstat_css_list;

You might want to place rstat after rstat_css_list just to keep
(hopefully) on the same cacheline as before other this will put
rstat_css_list with rstat_flush_next which the current padding is trying
to avoid. This is just to be safe. Later we might want to reevaluate the
padding and right cacheline alignments of the fields of struct cgroup.

>  
> -	/*
> -	 * Add padding to separate the read mostly rstat_cpu and
> -	 * rstat_css_list into a different cacheline from the following
> -	 * rstat_flush_next and *bstat fields which can have frequent updates.
> -	 */
> -	CACHELINE_PADDING(_pad_);
> -
> -	/*
> -	 * A singly-linked list of cgroup structures to be rstat flushed.
> -	 * This is a scratch field to be used exclusively by
> -	 * cgroup_rstat_flush_locked() and protected by cgroup_rstat_lock.
> -	 */
> -	struct cgroup	*rstat_flush_next;
> -
>  	/* cgroup basic resource statistics */
>  	struct cgroup_base_stat last_bstat;
>  	struct cgroup_base_stat bstat;
> diff --git a/include/linux/cgroup_rstat.h b/include/linux/cgroup_rstat.h
> new file mode 100644
> index 000000000000..f95474d6f8ab
> --- /dev/null
> +++ b/include/linux/cgroup_rstat.h
> @@ -0,0 +1,92 @@
[...]
> +struct cgroup_rstat {
> +	struct cgroup_rstat_cpu __percpu *rstat_cpu;
> +
> +	/*
> +	 * Add padding to separate the read mostly rstat_cpu and
> +	 * rstat_css_list into a different cacheline from the following
> +	 * rstat_flush_next and containing struct fields which can have
> +	 * frequent updates.
> +	 */
> +	CACHELINE_PADDING(_pad_);
> +	struct cgroup *rstat_flush_next;
> +};