From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758934AbcISMBr (ORCPT ); Mon, 19 Sep 2016 08:01:47 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:34668 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751713AbcISMBh (ORCPT ); Mon, 19 Sep 2016 08:01:37 -0400 Date: Mon, 19 Sep 2016 14:01:34 +0200 From: Michal Hocko To: Johannes Weiner Cc: Andrew Morton , Tejun Heo , "David S. Miller" , linux-mm@kvack.org, cgroups@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, Vladimir Davydov Subject: Re: [PATCH 1/3] mm: memcontrol: make per-cpu charge cache IRQ-safe for socket accounting Message-ID: <20160919120133.GM10785@dhcp22.suse.cz> References: <20160914194846.11153-1-hannes@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160914194846.11153-1-hannes@cmpxchg.org> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [Fixup Vladimir's email] On Wed 14-09-16 15:48:44, Johannes Weiner wrote: > From: Johannes Weiner > > During cgroup2 rollout into production, we started encountering css > refcount underflows and css access crashes in the memory controller. > Splitting the heavily shared css reference counter into logical users > narrowed the imbalance down to the cgroup2 socket memory accounting. > > The problem turns out to be the per-cpu charge cache. Cgroup1 had a > separate socket counter, but the new cgroup2 socket accounting goes > through the common charge path that uses a shared per-cpu cache for > all memory that is being tracked. Those caches are safe against > scheduling preemption, but not against interrupts - such as the newly > added packet receive path. When cache draining is interrupted by > network RX taking pages out of the cache, the resuming drain operation > will put references of in-use pages, thus causing the imbalance. > > Disable IRQs during all per-cpu charge cache operations. > > Fixes: f7e1cb6ec51b ("mm: memcontrol: account socket memory in unified hierarchy memory controller") > Cc: # 4.5+ > Signed-off-by: Johannes Weiner Acked-by: Michal Hocko > --- > mm/memcontrol.c | 31 ++++++++++++++++++++++--------- > 1 file changed, 22 insertions(+), 9 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 7a8d6624758a..60bb830abc34 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1710,17 +1710,22 @@ static DEFINE_MUTEX(percpu_charge_mutex); > static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > { > struct memcg_stock_pcp *stock; > + unsigned long flags; > bool ret = false; > > if (nr_pages > CHARGE_BATCH) > return ret; > > - stock = &get_cpu_var(memcg_stock); > + local_irq_save(flags); > + > + stock = this_cpu_ptr(&memcg_stock); > if (memcg == stock->cached && stock->nr_pages >= nr_pages) { > stock->nr_pages -= nr_pages; > ret = true; > } > - put_cpu_var(memcg_stock); > + > + local_irq_restore(flags); > + > return ret; > } > > @@ -1741,15 +1746,18 @@ static void drain_stock(struct memcg_stock_pcp *stock) > stock->cached = NULL; > } > > -/* > - * This must be called under preempt disabled or must be called by > - * a thread which is pinned to local cpu. > - */ > static void drain_local_stock(struct work_struct *dummy) > { > - struct memcg_stock_pcp *stock = this_cpu_ptr(&memcg_stock); > + struct memcg_stock_pcp *stock; > + unsigned long flags; > + > + local_irq_save(flags); > + > + stock = this_cpu_ptr(&memcg_stock); > drain_stock(stock); > clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags); > + > + local_irq_restore(flags); > } > > /* > @@ -1758,14 +1766,19 @@ static void drain_local_stock(struct work_struct *dummy) > */ > static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > { > - struct memcg_stock_pcp *stock = &get_cpu_var(memcg_stock); > + struct memcg_stock_pcp *stock; > + unsigned long flags; > + > + local_irq_save(flags); > > + stock = this_cpu_ptr(&memcg_stock); > if (stock->cached != memcg) { /* reset if necessary */ > drain_stock(stock); > stock->cached = memcg; > } > stock->nr_pages += nr_pages; > - put_cpu_var(memcg_stock); > + > + local_irq_restore(flags); > } > > /* > -- > 2.9.3 -- Michal Hocko SUSE Labs From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Hocko Subject: Re: [PATCH 1/3] mm: memcontrol: make per-cpu charge cache IRQ-safe for socket accounting Date: Mon, 19 Sep 2016 14:01:34 +0200 Message-ID: <20160919120133.GM10785@dhcp22.suse.cz> References: <20160914194846.11153-1-hannes@cmpxchg.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andrew Morton , Tejun Heo , "David S. Miller" , linux-mm@kvack.org, cgroups@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, Vladimir Davydov To: Johannes Weiner Return-path: Content-Disposition: inline In-Reply-To: <20160914194846.11153-1-hannes@cmpxchg.org> Sender: owner-linux-mm@kvack.org List-Id: netdev.vger.kernel.org [Fixup Vladimir's email] On Wed 14-09-16 15:48:44, Johannes Weiner wrote: > From: Johannes Weiner > > During cgroup2 rollout into production, we started encountering css > refcount underflows and css access crashes in the memory controller. > Splitting the heavily shared css reference counter into logical users > narrowed the imbalance down to the cgroup2 socket memory accounting. > > The problem turns out to be the per-cpu charge cache. Cgroup1 had a > separate socket counter, but the new cgroup2 socket accounting goes > through the common charge path that uses a shared per-cpu cache for > all memory that is being tracked. Those caches are safe against > scheduling preemption, but not against interrupts - such as the newly > added packet receive path. When cache draining is interrupted by > network RX taking pages out of the cache, the resuming drain operation > will put references of in-use pages, thus causing the imbalance. > > Disable IRQs during all per-cpu charge cache operations. > > Fixes: f7e1cb6ec51b ("mm: memcontrol: account socket memory in unified hierarchy memory controller") > Cc: # 4.5+ > Signed-off-by: Johannes Weiner Acked-by: Michal Hocko > --- > mm/memcontrol.c | 31 ++++++++++++++++++++++--------- > 1 file changed, 22 insertions(+), 9 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 7a8d6624758a..60bb830abc34 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1710,17 +1710,22 @@ static DEFINE_MUTEX(percpu_charge_mutex); > static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > { > struct memcg_stock_pcp *stock; > + unsigned long flags; > bool ret = false; > > if (nr_pages > CHARGE_BATCH) > return ret; > > - stock = &get_cpu_var(memcg_stock); > + local_irq_save(flags); > + > + stock = this_cpu_ptr(&memcg_stock); > if (memcg == stock->cached && stock->nr_pages >= nr_pages) { > stock->nr_pages -= nr_pages; > ret = true; > } > - put_cpu_var(memcg_stock); > + > + local_irq_restore(flags); > + > return ret; > } > > @@ -1741,15 +1746,18 @@ static void drain_stock(struct memcg_stock_pcp *stock) > stock->cached = NULL; > } > > -/* > - * This must be called under preempt disabled or must be called by > - * a thread which is pinned to local cpu. > - */ > static void drain_local_stock(struct work_struct *dummy) > { > - struct memcg_stock_pcp *stock = this_cpu_ptr(&memcg_stock); > + struct memcg_stock_pcp *stock; > + unsigned long flags; > + > + local_irq_save(flags); > + > + stock = this_cpu_ptr(&memcg_stock); > drain_stock(stock); > clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags); > + > + local_irq_restore(flags); > } > > /* > @@ -1758,14 +1766,19 @@ static void drain_local_stock(struct work_struct *dummy) > */ > static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages) > { > - struct memcg_stock_pcp *stock = &get_cpu_var(memcg_stock); > + struct memcg_stock_pcp *stock; > + unsigned long flags; > + > + local_irq_save(flags); > > + stock = this_cpu_ptr(&memcg_stock); > if (stock->cached != memcg) { /* reset if necessary */ > drain_stock(stock); > stock->cached = memcg; > } > stock->nr_pages += nr_pages; > - put_cpu_var(memcg_stock); > + > + local_irq_restore(flags); > } > > /* > -- > 2.9.3 -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org