On Mon, Oct 12, 2020 at 9:59 PM Johannes Weiner [off-list ref] wrote:

On Sat, Oct 10, 2020 at 06:45:21PM +0800, Muchun Song wrote:

quoted

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 <redacted>
---
 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