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

[Chunyu Hu <chuhu@redhat.com>]

----- Original Message -----
> From: "Tetsuo Handa" <penguin-kernel@i-love.sakura.ne.jp>
> To: "Chunyu Hu" <chuhu@redhat.com>
> Cc: mhocko@suse.com, malat@debian.org, dvyukov@google.com, linux-mm@kvack.org, "catalin marinas"
> <catalin.marinas@arm.com>
> Sent: Tuesday, May 29, 2018 9:46:59 PM
> Subject: Re: [PATCH] kmemleak: don't use __GFP_NOFAIL
> 
> On 2018/05/29 22:27, Chunyu Hu wrote:
> >>> I am not really familiar with the kmemleak code but the expectation that
> >>> you can make a forward progress in an unknown allocation context seems
> >>> broken to me. Why kmemleak cannot pre-allocate a pool of object_cache
> >>> and refill it from a reasonably strong contexts (e.g. in a sleepable
> >>> context)?
> >>
> >> Or, we can undo the original allocation if the kmemleak allocation failed?
> > 
> > If so, you are making kmemleak a fault injection trigger. But the original
> > purpose for adding GFP_NOFAIL[2] is just for making kmemleak avoid fault
> > injection.
> > (discussion in [1])
> 
> I don't think that applying fault injection to kmemleak allocations is bad
> (except that fault injection messages might be noisy).

Maybe we provide a way for user decide to apply fault inject to kmemleak or not 
is better, by adding another sys file in /sys/kernel/debug/failslab and the
fail_page_alloc.

> 
> > 
> > I'm trying with per task way for fault injection, and did some tries. In my
> > try, I removed this from NOFAIL kmemleak and kmemleak survived with the per
> > task fault injection helper (disable/enable of task). Maybe I can send
> > another
> > RFC for the api.
> 
> You could carry __GFP_NO_FAULT_INJECTION using per "struct task_struct" flag.

Thanks for this suggestion. 

I'm trying to reuse the make_it_fail field in task for fault injection. As adding
an extra memory alloc flag is not thought so good,  I think adding task flag
is either? 

> 
> But I think that undoing the original allocation if the kmemleak allocation
> failed
> has an advantage that it does not disable kmemleak when the system is under
> memory
> pressure (i.e. about to invoke the OOM killer); allowing us to test memory
> pressure
> conditions.

There should be benefit, this is redefining kmemleak's principle, currently it
won't affect other allocation directly, but if we free the other user's mem
alloc without thinking about the context seems also risk. 

> 
> 

-- 
Regards,
Chunyu Hu