From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 453BCCA9EBD for ; Thu, 24 Oct 2019 08:14:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1896721872 for ; Thu, 24 Oct 2019 08:14:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1571904892; bh=WzV4gtHEfjr3CBeEJoCF3xe52hV0QGkh855TcAsOGhY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=okPcerPlLICsxBwlY3Bo8+X6s2Y501Ecq0mT+zwqXZfsggzzkQqbF1+p+5zmblzA8 5Enh89rIvCp4PTX3KRc5erupJB3AggJQlI1p/jy3D021J8QgfgR7X4RiZD1hkwiIaG NxEI3XIN/Ry98h8v3LmXD4CaJfSWzhJsGbzYqyVk= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2438050AbfJXIOt (ORCPT ); Thu, 24 Oct 2019 04:14:49 -0400 Received: from mx2.suse.de ([195.135.220.15]:54914 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726395AbfJXIOs (ORCPT ); Thu, 24 Oct 2019 04:14:48 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 271B5B5F6; Thu, 24 Oct 2019 08:14:46 +0000 (UTC) Date: Thu, 24 Oct 2019 10:14:45 +0200 From: Michal Hocko To: Johannes Weiner Cc: Shakeel Butt , Andrew Morton , Linux MM , LKML , Cgroups , netdev@vger.kernel.org, Kernel Team Subject: Re: [PATCH] mm: memcontrol: fix network errors from failing __GFP_ATOMIC charges Message-ID: <20191024081445.GR17610@dhcp22.suse.cz> References: <20191022233708.365764-1-hannes@cmpxchg.org> <20191023064012.GB754@dhcp22.suse.cz> <20191023154618.GA366316@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed 23-10-19 10:38:36, Shakeel Butt wrote: > On Wed, Oct 23, 2019 at 8:46 AM Johannes Weiner wrote: > > > > On Wed, Oct 23, 2019 at 08:40:12AM +0200, Michal Hocko wrote: [...] > > > On the other hand this would allow to break the isolation by an > > > unpredictable amount. Should we put a simple cap on how much we can go > > > over the limit. If the memcg limit reclaim is not able to keep up with > > > those overflows then even __GFP_ATOMIC allocations have to fail. What do > > > you think? > > > > I don't expect a big overrun in practice, and it appears that Google > > has been letting even NOWAIT allocations pass through without > > isolation issues. > > We have been overcharging for __GFP_HIGH allocations for couple of > years and see no isolation issues in the production. > > > Likewise, we have been force-charging the skmem for > > a while now and it hasn't been an issue for reclaim to keep up. > > > > My experience from production is that it's a whole lot easier to debug > > something like a memory.max overrun than it is to debug a machine that > > won't respond to networking. So that's the side I would err on. It is definitely good to hear that your production systems are working well. I was not really worried about normal workloads but rather malicious kind of (work)loads where memcg is used to contain a potentially untrusted entities. That's where an unbounded atomic charges escapes would be a much bigger deal. Maybe this is not the case now because we do not have that many accounted __GFP_ATOMIC requests (I have tried to audit but gave up very shortly afterwards because there are not that many using __GFP_ACCOUNT directly so they are likely hidden behind SLAB_ACCOUNT). But I do not really like that uncertainty. If you have a really strong opinion on an explicit limit then I would like to see at least some warning to the kernel log so that we learn when some workloads hit a pathological paths that and act upon that. Does that sound like something you would agree to? E.g. something like diff --git a/mm/page_counter.c b/mm/page_counter.c index de31470655f6..e6999f6cf79e 100644 --- a/mm/page_counter.c +++ b/mm/page_counter.c @@ -62,6 +62,8 @@ void page_counter_cancel(struct page_counter *counter, unsigned long nr_pages) WARN_ON_ONCE(new < 0); } +#define SAFE_OVERFLOW 1024 + /** * page_counter_charge - hierarchically charge pages * @counter: counter @@ -82,8 +84,14 @@ void page_counter_charge(struct page_counter *counter, unsigned long nr_pages) * This is indeed racy, but we can live with some * inaccuracy in the watermark. */ - if (new > c->watermark) + if (new > c->watermark) { c->watermark = new; + if (new > c->max + SAFE_OVERFLOW) { + pr_warn("Max limit %lu breached, usage:%lu. Please report.\n", + c->max, atomic_long_read(&c->usage); + dump_stack(); + } + } } } -- Michal Hocko SUSE Labs