All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Florian Westphal <fw@strlen.de>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.13 01/52] Revert "net: use lib/percpu_counter API for fragmentation mem accounting"
Date: Mon, 18 Sep 2017 11:09:29 +0200	[thread overview]
Message-ID: <20170918090904.284175796@linuxfoundation.org> (raw)
In-Reply-To: <20170918090904.072766209@linuxfoundation.org>

4.13-stable review patch.  If anyone has any objections, please let me know.

------------------

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  |   30 +++++++++---------------------
 net/ipv4/inet_fragment.c |    4 +---
 2 files changed, 10 insertions(+), 24 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,31 +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_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 :
--- 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);
 

  reply	other threads:[~2017-09-18  9:10 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-18  9:09 [PATCH 4.13 00/52] 4.13.3-stable review Greg Kroah-Hartman
2017-09-18  9:09 ` Greg Kroah-Hartman [this message]
2017-09-18  9:09 ` [PATCH 4.13 02/52] Revert "net: fix percpu memory leaks" Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 03/52] gianfar: Fix Tx flow control deactivation Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 04/52] vhost_net: correctly check tx avail during rx busy polling Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 05/52] ip6_gre: update mtu properly in ip6gre_err Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 06/52] udp: drop head states only when all skb references are gone Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 07/52] ipv6: fix memory leak with multiple tables during netns destruction Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 08/52] ipv6: fix typo in fib6_net_exit() Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 09/52] sctp: fix missing wake ups in some situations Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 10/52] tcp: fix a request socket leak Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 11/52] ip_tunnel: fix setting ttl and tos value in collect_md mode Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 12/52] f2fs: let fill_super handle roll-forward errors Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 13/52] f2fs: check hot_data for roll-forward recovery Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 14/52] thunderbolt: Remove superfluous check Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 15/52] thunderbolt: Make key root-only accessible Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 16/52] thunderbolt: Allow clearing the key Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 17/52] x86/fsgsbase/64: Fully initialize FS and GS state in start_thread_common Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 18/52] x86/fsgsbase/64: Report FSBASE and GSBASE correctly in core dumps Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 19/52] x86/switch_to/64: Rewrite FS/GS switching yet again to fix AMD CPUs Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 20/52] x86/mm, mm/hwpoison: Clear PRESENT bit for kernel 1:1 mappings of poison pages Greg Kroah-Hartman
2017-09-18  9:09   ` Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 21/52] ovl: fix false positive ESTALE on lookup Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 22/52] fuse: allow server to run in different pid_ns Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 23/52] idr: remove WARN_ON_ONCE() when trying to replace negative ID Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 24/52] libnvdimm, btt: check memory allocation failure Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 25/52] libnvdimm: fix integer overflow static analysis warning Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 26/52] xfs: write unmount record for ro mounts Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 27/52] xfs: toggle readonly state around xfs_log_mount_finish Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 28/52] xfs: Add infrastructure needed for error propagation during buffer IO failure Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 29/52] xfs: Properly retry failed inode items in case of error during buffer writeback Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 30/52] xfs: fix recovery failure when log record header wraps log end Greg Kroah-Hartman
2017-09-18  9:09 ` [PATCH 4.13 31/52] xfs: always verify the log tail during recovery Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 32/52] xfs: fix log recovery corruption error due to tail overwrite Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 33/52] xfs: handle -EFSCORRUPTED during head/tail verification Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 34/52] xfs: stop searching for free slots in an inode chunk when there are none Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 35/52] xfs: evict all inodes involved with log redo item Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 36/52] xfs: check for race with xfs_reclaim_inode() in xfs_ifree_cluster() Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 37/52] xfs: open-code xfs_buf_item_dirty() Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 38/52] xfs: remove unnecessary dirty bli format check for ordered bufs Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 39/52] xfs: ordered buffer log items are never formatted Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 40/52] xfs: refactor buffer logging into buffer dirtying helper Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 41/52] xfs: dont log dirty ranges for ordered buffers Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 42/52] xfs: skip bmbt block ino validation during owner change Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 43/52] xfs: move bmbt owner change to last step of extent swap Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 44/52] xfs: disallow marking previously dirty buffers as ordered Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 45/52] xfs: relog dirty buffers during swapext bmbt owner change Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 46/52] xfs: disable per-inode DAX flag Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 47/52] xfs: fix incorrect log_flushed on fsync Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 48/52] xfs: dont set v3 xflags for v2 inodes Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 49/52] xfs: open code end_buffer_async_write in xfs_finish_page_writeback Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 50/52] xfs: use kmem_free to free return value of kmem_zalloc Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 51/52] md/raid1/10: reset bio allocated from mempool Greg Kroah-Hartman
2017-09-18  9:10 ` [PATCH 4.13 52/52] md/raid5: release/flush io in raid5_do_work() Greg Kroah-Hartman
2017-09-18 19:29 ` [PATCH 4.13 00/52] 4.13.3-stable review Guenter Roeck
2017-09-18 20:17 ` Shuah Khan
2017-09-19  6:33   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170918090904.284175796@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=brouer@redhat.com \
    --cc=davem@davemloft.net \
    --cc=fw@strlen.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.