Re: [PATCH] kmemleak: don't use __GFP_NOFAIL

From: Catalin Marinas <catalin.marinas@arm.com>
To: Michal Hocko <mhocko@suse.com>
Cc: Chunyu Hu <chuhu@redhat.com>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	malat@debian.org, dvyukov@google.com, linux-mm@kvack.org,
	Akinobu Mita <akinobu.mita@gmail.com>
Subject: Re: [PATCH] kmemleak: don't use __GFP_NOFAIL
Date: Thu, 31 May 2018 16:22:26 +0100	[thread overview]
Message-ID: <20180531152225.2ck6ach4lma4zeim@armageddon.cambridge.arm.com> (raw)
In-Reply-To: <20180530123826.GF27180@dhcp22.suse.cz>

Hi Michal,

I'm catching up with this thread.

On Wed, May 30, 2018 at 02:38:26PM +0200, Michal Hocko wrote:
> On Wed 30-05-18 07:42:59, Chunyu Hu wrote:
> > From: "Michal Hocko" <mhocko@suse.com>
> > > want to have a pre-populated pool of objects for those requests. The
> > > obvious question is how to balance such a pool. It ain't easy to track
> > > memory by allocating more memory...
> > 
> > This solution is going to make kmemleak trace really nofail. We can think
> > later.
> > 
> > while I'm thinking about if fault inject can be disabled via flag in task.
> > 
> > Actually, I'm doing something like below, the disable_fault_inject() is
> > just setting a flag in task->make_it_fail. But this will depend on if
> > fault injection accept a change like this. CCing Akinobu 
> 
> You still seem to be missing my point I am afraid (or I am ;). So say
> that you want to track a GFP_NOWAIT allocation request. So create_object
> will get called with that gfp mask and no matter what you try here your
> tracking object will be allocated in a weak allocation context as well
> and disable kmemleak. So it only takes a more heavy memory pressure and
> the tracing is gone...

create_object() indeed gets the originating gfp but it only cares
whether it was GFP_KERNEL or GFP_ATOMIC. gfp_kmemleak_mask() masks out
all the other flags when allocating a struct kmemleak_object (and adds
some of its own).

This has worked ok so far. There is a higher risk of GFP_ATOMIC
allocations failing but I haven't seen issues with kmemleak unless you
run it under heavy memory pressure (and kmemleak just disables itself).
With fault injection, such pressure is simulated with the side effect of
rendering kmemleak unusable.

Kmemleak could implement its own allocation mechanism (maybe even
skipping the slab altogether) but I feel it's overkill just to cope with
the specific case of fault injection. Also, it's not easy to figure out
how to balance such pool and it may end up calling alloc_pages() which
in turn can inject a fault.

Could we tweak gfp_kmemleak_mask() to still pass __GFP_NOFAIL but in a
compatible way (e.g. by setting __GFP_DIRECT_RECLAIM) when fault
injection is enabled?

Otherwise, I'd prefer some per-thread temporary fault injection
disabling.

-- 
Catalin