Revert "net: use lib/percpu_counter API for fragmentation mem accounting" (fb452a1a) · Commits · 戴 / test

This reverts commit . 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 ("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: ("net: use lib/percpu_counter API for fragmentation mem accounting") Fixes: ("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>

include/net/inet_frag.h

+9 −21

Original line number	Diff line number	Diff line
		#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,31 +135,24 @@ static inline bool inet_frag_evicting(struct inet_frag_queue *q)

		/* 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_batch(&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_batch(&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)
		{
		return percpu_counter_sum_positive(&nf->mem);
		return atomic_read(&nf->mem);
		}

		/* RFC 3168 support :

net/ipv4/inet_fragment.c

+1 −3

Original line number	Diff line number	Diff line
		@@ -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);

Admin message