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
