Re: [PATCH] mm, memcg: reduce size of struct mem_cgroup by using bit field

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 27 Dec 2019 07:43:52 -0500 Yafang Shao <laoar.shao@gmail.com> wrote:

> There are some members in struct mem_group can be either 0(false) or
> 1(true), so we can define them using bit field to reduce size. With this
> patch, the size of struct mem_cgroup can be reduced by 64 bytes in theory,
> but as there're some MEMCG_PADDING()s, the real number may be different,
> which is relate with the cacheline size. Anyway, this patch could reduce
> the size of struct mem_cgroup more or less.
> 
> ...
>
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -229,20 +229,26 @@ struct mem_cgroup {
>  	/*
>  	 * Should the accounting and control be hierarchical, per subtree?
>  	 */
> -	bool use_hierarchy;
> +	unsigned int use_hierarchy : 1;
> +
> +	/* Legacy tcp memory accounting */
> +	unsigned int tcpmem_active : 1;
> +	unsigned int tcpmem_pressure : 1;

Kernel coding style for this is normally no-spaces:

	bool foo:1;



More significantly...  Now that these fields share the same word of
memory, what prevents races when two CPUs do read-modify-write
operations on adjacent bitfields?

This:

struct foo {
	int a;
	int b;
};

doesn't need locking to prevent modifications of `a' from scribbling on
`b'.  But with this:

struct foo {
	int a:1;
	int b:1;
}

a simple `a = 1' on CPU1 could race with a `b = 1' on CPU2.

I think.  Maybe the compiler can take care of this in some fashion, but
it would require atomic bitops and I doubt if gcc does that for us?