On Mon, Oct 12, 2020 at 9:59 PM Johannes Weiner <hannes@xxxxxxxxxxx> wrote: > > On Sat, Oct 10, 2020 at 06:45:21PM +0800, Muchun Song wrote: > > Move the mem_cgroup_sockets_enabled() checks into memcg socket charge > > or uncharge functions, so the users don't have to explicitly check that > > condition. > > > > This is purely code cleanup patch without any functional change. But > > move the sk_memcg member of the sock structure to the CONFIG_MEMCG > > scope. > > > > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx> > > --- > > include/linux/memcontrol.h | 78 ++++++++++++++++++++++++++------- > > include/net/sock.h | 5 ++- > > include/net/tcp.h | 3 +- > > mm/memcontrol.c | 43 +++++++++++++----- > > net/core/sock.c | 15 +++---- > > net/ipv4/inet_connection_sock.c | 6 +-- > > net/ipv4/tcp_output.c | 3 +- > > 7 files changed, 111 insertions(+), 42 deletions(-) > > Hm, this is almost 3 times as much code. > > The sk_memcg saving on !CONFIG_MEMCG is somewhat nice, but it's not > clear how many users would benefit here. And it adds ifdefs in code. The 'ifdefs in code' means the initialization of sk_memcg in the sk_clone_lock? If yes, we can add a new inline initialization function to avoid this. > > Also memcg code now has to know about struct sock. Without this patch, the memcg code also has to know about struct sock. You can see the code of mem_cgroup_sk_alloc and mem_cgroup_sk_free. Thanks. > > I'm not quite sure that this is an overall improvement. -- Yours, Muchun