On 7/16/19 11:23 AM, Qian Cai wrote:
On Wed, 2019-07-17 at 01:50 +0800, Yang Shi wrote:
When running ltp's oom test with kmemleak enabled, the below warning was
triggerred since kernel detects __GFP_NOFAIL & ~__GFP_DIRECT_RECLAIM is
passed in:
WARNING: CPU: 105 PID: 2138 at mm/page_alloc.c:4608
__alloc_pages_nodemask+0x1c31/0x1d50
Modules linked in: loop dax_pmem dax_pmem_core ip_tables x_tables xfs
virtio_net net_failover virtio_blk failover ata_generic virtio_pci virtio_ring
virtio libata
CPU: 105 PID: 2138 Comm: oom01 Not tainted 5.2.0-next-20190710+ #7
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.2-0-
g5f4c7b1-prebuilt.qemu-project.org 04/01/2014
RIP: 0010:__alloc_pages_nodemask+0x1c31/0x1d50
...
kmemleak_alloc+0x4e/0xb0
kmem_cache_alloc+0x2a7/0x3e0
? __kmalloc+0x1d6/0x470
? ___might_sleep+0x9c/0x170
? mempool_alloc+0x2b0/0x2b0
mempool_alloc_slab+0x2d/0x40
mempool_alloc+0x118/0x2b0
? __kasan_check_read+0x11/0x20
? mempool_resize+0x390/0x390
? lock_downgrade+0x3c0/0x3c0
bio_alloc_bioset+0x19d/0x350
? __swap_duplicate+0x161/0x240
? bvec_alloc+0x1b0/0x1b0
? do_raw_spin_unlock+0xa8/0x140
? _raw_spin_unlock+0x27/0x40
get_swap_bio+0x80/0x230
? __x64_sys_madvise+0x50/0x50
? end_swap_bio_read+0x310/0x310
? __kasan_check_read+0x11/0x20
? check_chain_key+0x24e/0x300
? bdev_write_page+0x55/0x130
__swap_writepage+0x5ff/0xb20
The mempool_alloc_slab() clears __GFP_DIRECT_RECLAIM, however kmemleak has
__GFP_NOFAIL set all the time due to commit
d9570ee3bd1d4f20ce63485f5ef05663866fe6c0 ("kmemleak: allow to coexist
with fault injection"). But, it doesn't make any sense to have
__GFP_NOFAIL and ~__GFP_DIRECT_RECLAIM specified at the same time.
According to the discussion on the mailing list, the commit should be
reverted for short term solution. Catalin Marinas would follow up with a
better
solution for longer term.
The failure rate of kmemleak metadata allocation may increase in some
circumstances, but this should be expected side effect.
As mentioned in anther thread, the situation for kmemleak under memory pressure
has already been unhealthy. I don't feel comfortable to make it even worse by
reverting this commit alone. This could potentially make kmemleak kill itself
easier and miss some more real memory leak later.
To make it really a short-term solution before the reverting, I think someone
needs to follow up with the mempool solution with tunable pool size mentioned
in,
https://lore.kernel.org/linux-mm/20190328145917.GC10283@xxxxxxxxxxxxxxxxxxxx/
I personally not very confident that Catalin will find some time soon to
implement embedding kmemleak metadata into the slab. Even he or someone does
eventually, it probably need quite some time to test and edge out many of corner
cases that kmemleak could have by its natural.
Thanks for sharing some background. I didn't notice this topic had been
discussed. I'm not sure if this revert would make things worse since I'm
supposed real memory leak would be detected sooner before oom kicks in,
and kmemleak is already broken with __GFP_NOFAIL.
It seems everyone agree __GFP_NPFAIL should be removed? Anyway, I would
like leave the decision to Catalin.
Suggested-by: Catalin Marinas <catalin.marinas@xxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxx>
Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
Cc: David Rientjes <rientjes@xxxxxxxxxx>
Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
Cc: Qian Cai <cai@xxxxxx>
Signed-off-by: Yang Shi <yang.shi@xxxxxxxxxxxxxxxxx>
---
mm/kmemleak.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 9dd581d..884a5e3 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -114,7 +114,7 @@
/* GFP bitmask for kmemleak internal allocations */
#define gfp_kmemleak_mask(gfp) (((gfp) & (GFP_KERNEL | GFP_ATOMIC)) |
\
__GFP_NORETRY | __GFP_NOMEMALLOC | \
- __GFP_NOWARN | __GFP_NOFAIL)
+ __GFP_NOWARN)
/* scanning area inside a memory block */
struct kmemleak_scan_area {
