Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 26 Mar 2019 17:33:56 +0100
From:   Michal Hocko <mhocko@kernel.org>
To:     Catalin Marinas <catalin.marinas@arm.com>
Cc:     Matthew Wilcox <willy@infradead.org>, Qian Cai <cai@lca.pw>,
        akpm@linux-foundation.org, cl@linux.com, penberg@kernel.org,
        rientjes@google.com, iamjoonsoo.kim@lge.com, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] kmemleaak: survive in a low-memory situation
Message-ID: <20190326163356.GS28406@dhcp22.suse.cz>
References: <20190326154338.20594-1-cai@lca.pw>
 <20190326160536.GO10344@bombadil.infradead.org>
 <20190326162038.GH33308@arrakis.emea.arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190326162038.GH33308@arrakis.emea.arm.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue 26-03-19 16:20:41, Catalin Marinas wrote:
> On Tue, Mar 26, 2019 at 09:05:36AM -0700, Matthew Wilcox wrote:
> > On Tue, Mar 26, 2019 at 11:43:38AM -0400, Qian Cai wrote:
> > > Unless there is a brave soul to reimplement the kmemleak to embed it's
> > > metadata into the tracked memory itself in a foreseeable future, this
> > > provides a good balance between enabling kmemleak in a low-memory
> > > situation and not introducing too much hackiness into the existing
> > > code for now.
> > 
> > I don't understand kmemleak.  Kirill pointed me at this a few days ago:
> > 
> > https://gist.github.com/kiryl/3225e235fea390aa2e49bf625bbe83ec
> > 
> > It's caused by the XArray allocating memory using GFP_NOWAIT | __GFP_NOWARN.
> > kmemleak then decides it needs to allocate memory to track this memory.
> > So it calls kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
> > 
> > #define gfp_kmemleak_mask(gfp)  (((gfp) & (GFP_KERNEL | GFP_ATOMIC)) | \
> >                                  __GFP_NORETRY | __GFP_NOMEMALLOC | \
> >                                  __GFP_NOWARN | __GFP_NOFAIL)
> > 
> > then the page allocator gets to see GFP_NOFAIL | GFP_NOWAIT and gets angry.
> > 
> > But I don't understand why kmemleak needs to mess with the GFP flags at
> > all.
> 
> Originally, it was just preserving GFP_KERNEL | GFP_ATOMIC. Starting
> with commit 6ae4bd1f0bc4 ("kmemleak: Allow kmemleak metadata allocations
> to fail"), this mask changed, aimed at making kmemleak allocation
> failures less verbose (i.e. just disable it since it's a debug tool).
> 
> Commit d9570ee3bd1d ("kmemleak: allow to coexist with fault injection")
> introduced __GFP_NOFAIL but this came with its own problems which have
> been previously reported (the warning you mentioned is another one of
> these). We didn't get to any clear conclusion on how best to allow
> allocations to fail with fault injection but not for the kmemleak
> metadata. Your suggestion below would probably do the trick.

I have objected to that on several occasions. An implicit __GFP_NOFAIL
is simply broken and __GFP_NOWAIT allocations are a shiny example of
that. You cannot loop inside the allocator for an unbound amount of time
potentially with locks held. I have heard that there are some plans to
deal with that but nothing has really materialized AFAIK. d9570ee3bd1d
should be reverted I believe.

The proper way around is to keep a pool objects and keep spare objects
for restrected allocation contexts.
-- 
Michal Hocko
SUSE Labs