| From foo@baz Thu Sep 14 23:20:08 PDT 2017 |
| From: Jesper Dangaard Brouer <brouer@redhat.com> |
| Date: Fri, 1 Sep 2017 11:26:08 +0200 |
| Subject: Revert "net: use lib/percpu_counter API for fragmentation mem accounting" |
| |
| From: Jesper Dangaard Brouer <brouer@redhat.com> |
| |
| |
| [ Upstream commit fb452a1aa3fd4034d7999e309c5466ff2d7005aa ] |
| |
| This reverts commit 6d7b857d541ecd1d9bd997c97242d4ef94b19de2. |
| |
| There is a bug in fragmentation codes use of the percpu_counter API, |
| that can cause issues on systems with many CPUs. |
| |
| The frag_mem_limit() just reads the global counter (fbc->count), |
| without considering other CPUs can have upto batch size (130K) that |
| haven't been subtracted yet. Due to the 3MBytes lower thresh limit, |
| this become dangerous at >=24 CPUs (3*1024*1024/130000=24). |
| |
| The correct API usage would be to use __percpu_counter_compare() which |
| does the right thing, and takes into account the number of (online) |
| CPUs and batch size, to account for this and call __percpu_counter_sum() |
| when needed. |
| |
| We choose to revert the use of the lib/percpu_counter API for frag |
| memory accounting for several reasons: |
| |
| 1) On systems with CPUs > 24, the heavier fully locked |
| __percpu_counter_sum() is always invoked, which will be more |
| expensive than the atomic_t that is reverted to. |
| |
| Given systems with more than 24 CPUs are becoming common this doesn't |
| seem like a good option. To mitigate this, the batch size could be |
| decreased and thresh be increased. |
| |
| 2) The add_frag_mem_limit+sub_frag_mem_limit pairs happen on the RX |
| CPU, before SKBs are pushed into sockets on remote CPUs. Given |
| NICs can only hash on L2 part of the IP-header, the NIC-RXq's will |
| likely be limited. Thus, a fair chance that atomic add+dec happen |
| on the same CPU. |
| |
| Revert note that commit 1d6119baf061 ("net: fix percpu memory leaks") |
| removed init_frag_mem_limit() and instead use inet_frags_init_net(). |
| After this revert, inet_frags_uninit_net() becomes empty. |
| |
| Fixes: 6d7b857d541e ("net: use lib/percpu_counter API for fragmentation mem accounting") |
| Fixes: 1d6119baf061 ("net: fix percpu memory leaks") |
| Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> |
| Acked-by: Florian Westphal <fw@strlen.de> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| include/net/inet_frag.h | 36 +++++++++--------------------------- |
| net/ipv4/inet_fragment.c | 4 +--- |
| 2 files changed, 10 insertions(+), 30 deletions(-) |
| |
| --- a/include/net/inet_frag.h |
| +++ b/include/net/inet_frag.h |
| @@ -1,14 +1,9 @@ |
| #ifndef __NET_FRAG_H__ |
| #define __NET_FRAG_H__ |
| |
| -#include <linux/percpu_counter.h> |
| - |
| struct netns_frags { |
| - /* The percpu_counter "mem" need to be cacheline aligned. |
| - * mem.count must not share cacheline with other writers |
| - */ |
| - struct percpu_counter mem ____cacheline_aligned_in_smp; |
| - |
| + /* Keep atomic mem on separate cachelines in structs that include it */ |
| + atomic_t mem ____cacheline_aligned_in_smp; |
| /* sysctls */ |
| int timeout; |
| int high_thresh; |
| @@ -110,11 +105,11 @@ void inet_frags_fini(struct inet_frags * |
| |
| static inline int inet_frags_init_net(struct netns_frags *nf) |
| { |
| - return percpu_counter_init(&nf->mem, 0, GFP_KERNEL); |
| + atomic_set(&nf->mem, 0); |
| + return 0; |
| } |
| static inline void inet_frags_uninit_net(struct netns_frags *nf) |
| { |
| - percpu_counter_destroy(&nf->mem); |
| } |
| |
| void inet_frags_exit_net(struct netns_frags *nf, struct inet_frags *f); |
| @@ -140,37 +135,24 @@ static inline bool inet_frag_evicting(st |
| |
| /* Memory Tracking Functions. */ |
| |
| -/* The default percpu_counter batch size is not big enough to scale to |
| - * fragmentation mem acct sizes. |
| - * The mem size of a 64K fragment is approx: |
| - * (44 fragments * 2944 truesize) + frag_queue struct(200) = 129736 bytes |
| - */ |
| -static unsigned int frag_percpu_counter_batch = 130000; |
| - |
| static inline int frag_mem_limit(struct netns_frags *nf) |
| { |
| - return percpu_counter_read(&nf->mem); |
| + return atomic_read(&nf->mem); |
| } |
| |
| static inline void sub_frag_mem_limit(struct netns_frags *nf, int i) |
| { |
| - __percpu_counter_add(&nf->mem, -i, frag_percpu_counter_batch); |
| + atomic_sub(i, &nf->mem); |
| } |
| |
| static inline void add_frag_mem_limit(struct netns_frags *nf, int i) |
| { |
| - __percpu_counter_add(&nf->mem, i, frag_percpu_counter_batch); |
| + atomic_add(i, &nf->mem); |
| } |
| |
| -static inline unsigned int sum_frag_mem_limit(struct netns_frags *nf) |
| +static inline int sum_frag_mem_limit(struct netns_frags *nf) |
| { |
| - unsigned int res; |
| - |
| - local_bh_disable(); |
| - res = percpu_counter_sum_positive(&nf->mem); |
| - local_bh_enable(); |
| - |
| - return res; |
| + return atomic_read(&nf->mem); |
| } |
| |
| /* RFC 3168 support : |
| --- a/net/ipv4/inet_fragment.c |
| +++ b/net/ipv4/inet_fragment.c |
| @@ -234,10 +234,8 @@ evict_again: |
| cond_resched(); |
| |
| if (read_seqretry(&f->rnd_seqlock, seq) || |
| - percpu_counter_sum(&nf->mem)) |
| + sum_frag_mem_limit(nf)) |
| goto evict_again; |
| - |
| - percpu_counter_destroy(&nf->mem); |
| } |
| EXPORT_SYMBOL(inet_frags_exit_net); |
| |