Thread (66 messages) 66 messages, 8 authors, 2023-07-28

RE: [PATCH net-next 1/2] net: Keep sk->sk_forward_alloc as a proper size

From: Zhang, Cathy <hidden>
Date: 2023-05-10 13:54:41
Also in: cgroups, linux-mm 

-----Original Message-----
From: Eric Dumazet <redacted>
Sent: Wednesday, May 10, 2023 7:25 PM
To: Zhang, Cathy <redacted>
Cc: Shakeel Butt <redacted>; Linux MM <redacted>;
Cgroups [off-list ref]; Paolo Abeni [off-list ref];
davem@davemloft.net; kuba@kernel.org; Brandeburg, Jesse
[off-list ref]; Srinivas, Suresh
[off-list ref]; Chen, Tim C [off-list ref]; You,
Lizhen [off-list ref]; eric.dumazet@gmail.com;
netdev@vger.kernel.org
Subject: Re: [PATCH net-next 1/2] net: Keep sk->sk_forward_alloc as a proper
size

On Wed, May 10, 2023 at 1:11 PM Zhang, Cathy [off-list ref]
wrote:
quoted
Hi Shakeel, Eric and all,

How about adding memory pressure checking in sk_mem_uncharge() to
decide if keep part of memory or not, which can help avoid the issue
you fixed and the problem we find on the system with more CPUs.

The code draft is like this:

static inline void sk_mem_uncharge(struct sock *sk, int size) {
        int reclaimable;
        int reclaim_threshold = SK_RECLAIM_THRESHOLD;

        if (!sk_has_account(sk))
                return;
        sk->sk_forward_alloc += size;

        if (mem_cgroup_sockets_enabled && sk->sk_memcg &&
            mem_cgroup_under_socket_pressure(sk->sk_memcg)) {
                sk_mem_reclaim(sk);
                return;
        }

        reclaimable = sk->sk_forward_alloc -
sk_unused_reserved_mem(sk);

        if (reclaimable > reclaim_threshold) {
                reclaimable -= reclaim_threshold;
                __sk_mem_reclaim(sk, reclaimable);
        }
}

I've run a test with the new code, the result looks good, it does not
introduce latency, RPS is the same.
It will not work for sockets that are idle, after a burst.
If we restore per socket caches, we will need a shrinker.
Trust me, we do not want that kind of big hammer, crushing latencies.

Have you tried to increase batch sizes ?
I jus picked up 256 and 1024 for a try, but no help, the overhead still exists.

quoted hunk ↗ jump to hunk
Any kind of cache (even per-cpu) might need some adjustment when core
count or expected traffic is increasing.
This was somehow hinted in
commit 1813e51eece0ad6f4aacaeb738e7cced46feb470
Author: Shakeel Butt [off-list ref]
Date:   Thu Aug 25 00:05:06 2022 +0000

    memcg: increase MEMCG_CHARGE_BATCH to 64



diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index
222d7370134c73e59fdbdf598ed8d66897dbbf1d..0418229d30c25d114132a1e
d46ac01358cf21424
100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -334,7 +334,7 @@ struct mem_cgroup {
  * TODO: maybe necessary to use big numbers in big irons or dynamic based
of the
  * workload.
  */
-#define MEMCG_CHARGE_BATCH 64U
+#define MEMCG_CHARGE_BATCH 128U

 extern struct mem_cgroup *root_mem_cgroup;

diff --git a/include/net/sock.h b/include/net/sock.h index
656ea89f60ff90d600d16f40302000db64057c64..82f6a288be650f886b207e6a
5e62a1d5dda808b0
100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1433,8 +1433,8 @@ sk_memory_allocated(const struct sock *sk)
        return proto_memory_allocated(sk->sk_prot);
 }

-/* 1 MB per cpu, in page units */
-#define SK_MEMORY_PCPU_RESERVE (1 << (20 - PAGE_SHIFT))
+/* 2 MB per cpu, in page units */
+#define SK_MEMORY_PCPU_RESERVE (1 << (21 - PAGE_SHIFT))

 static inline void
 sk_memory_allocated_add(struct sock *sk, int amt)






quoted
quoted
-----Original Message-----
From: Shakeel Butt <redacted>
Sent: Wednesday, May 10, 2023 12:10 AM
To: Eric Dumazet <redacted>; Linux MM <linux-
mm@kvack.org>; Cgroups [off-list ref]
Cc: Zhang, Cathy <redacted>; Paolo Abeni
[off-list ref]; davem@davemloft.net; kuba@kernel.org;
Brandeburg, Jesse [off-list ref]; Srinivas, Suresh
[off-list ref]; Chen, Tim C [off-list ref];
You, Lizhen [off-list ref]; eric.dumazet@gmail.com;
netdev@vger.kernel.org
Subject: Re: [PATCH net-next 1/2] net: Keep sk->sk_forward_alloc as
a proper size

+linux-mm & cgroup

Thread: https://lore.kernel.org/all/20230508020801.10702-1-
cathy.zhang@intel.com/

On Tue, May 9, 2023 at 8:43 AM Eric Dumazet [off-list ref]
wrote:
quoted
[...]
quoted
Some mm experts should chime in, this is not a networking issue.
Most of the MM folks are busy in LSFMM this week. I will take a look
at this soon.
  



    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help