| Message ID | 20160811170812.GF18366@e104818-lin.cambridge.arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Catalin, Thanks for the response! On Thursday 11 August 2016 10:38 PM, Catalin Marinas wrote: > On Thu, Aug 11, 2016 at 07:48:12PM +0300, Grygorii Strashko wrote: >> On 08/11/2016 06:54 PM, Catalin Marinas wrote: >>> On Thu, Aug 11, 2016 at 05:20:51PM +0530, Vignesh R wrote: >>>> I see the below message from kmemleak when booting linux-next on AM335x >>>> GP EVM and DRA7 EVM >>> >>> Can you also reproduce it with 4.8-rc1? Yes, I can reproduce this on 4.8.0-rc1-g4b9eaf33d83d [...] > > diff --git a/mm/memblock.c b/mm/memblock.c > index 483197ef613f..7d3361d53ac2 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -723,7 +723,8 @@ int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size) > (unsigned long long)base + size - 1, > (void *)_RET_IP_); > > - kmemleak_free_part(__va(base), size); > + if (base < __pa(high_memory)) > + kmemleak_free_part(__va(base), size); > return memblock_remove_range(&memblock.reserved, base, size); > } > > @@ -1152,7 +1153,8 @@ static phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size, > * The min_count is set to 0 so that memblock allocations are > * never reported as leaks. > */ > - kmemleak_alloc(__va(found), size, 0, 0); > + if (found < __pa(high_memory)) > + kmemleak_alloc(__va(found), size, 0, 0); > return found; > } > return 0; > @@ -1399,7 +1401,8 @@ void __init __memblock_free_early(phys_addr_t base, phys_addr_t size) > memblock_dbg("%s: [%#016llx-%#016llx] %pF\n", > __func__, (u64)base, (u64)base + size - 1, > (void *)_RET_IP_); > - kmemleak_free_part(__va(base), size); > + if (base < __pa(high_memory)) > + kmemleak_free_part(__va(base), size); > memblock_remove_range(&memblock.reserved, base, size); > } > > @@ -1419,7 +1422,8 @@ void __init __memblock_free_late(phys_addr_t base, phys_addr_t size) > memblock_dbg("%s: [%#016llx-%#016llx] %pF\n", > __func__, (u64)base, (u64)base + size - 1, > (void *)_RET_IP_); > - kmemleak_free_part(__va(base), size); > + if (base < __pa(high_memory)) > + kmemleak_free_part(__va(base), size); > cursor = PFN_UP(base); > end = PFN_DOWN(base + size); > > With above change on 4.8-rc1, I see a different warning from kmemleak: [ 0.002918] kmemleak: Trying to color unknown object at 0xfe800000 as Black [ 0.002943] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.0-rc1-00121-g4b9eaf33d83d-dirty #59 [ 0.002955] Hardware name: Generic AM33XX (Flattened Device Tree) [ 0.003000] [<c01100fc>] (unwind_backtrace) from [<c010c264>] (show_stack+0x10/0x14) [ 0.003027] [<c010c264>] (show_stack) from [<c049040c>] (dump_stack+0xac/0xe0) [ 0.003052] [<c049040c>] (dump_stack) from [<c02971c0>] (paint_ptr+0x78/0x9c) [ 0.003074] [<c02971c0>] (paint_ptr) from [<c0b25e20>] (kmemleak_init+0x1cc/0x284) [ 0.003104] [<c0b25e20>] (kmemleak_init) from [<c0b00bc0>] (start_kernel+0x2d8/0x3b4) [ 0.003122] [<c0b00bc0>] (start_kernel) from [<8000807c>] (0x8000807c) [ 0.003133] kmemleak: Early log backtrace: [ 0.003146] [<c0b3c9cc>] dma_contiguous_reserve+0x80/0x94 [ 0.003170] [<c0b06810>] arm_memblock_init+0x130/0x184 [ 0.003191] [<c0b04210>] setup_arch+0x58c/0xc00 [ 0.003208] [<c0b00940>] start_kernel+0x58/0x3b4 [ 0.003224] [<8000807c>] 0x8000807c [ 0.003239] [<ffffffff>] 0xffffffff Full boot log: http://pastebin.ubuntu.com/23048180/
On 08/11/2016 08:08 PM, Catalin Marinas wrote: > On Thu, Aug 11, 2016 at 07:48:12PM +0300, Grygorii Strashko wrote: >> On 08/11/2016 06:54 PM, Catalin Marinas wrote: >>> On Thu, Aug 11, 2016 at 05:20:51PM +0530, Vignesh R wrote: >>>> I see the below message from kmemleak when booting linux-next on AM335x >>>> GP EVM and DRA7 EVM >>> >>> Can you also reproduce it with 4.8-rc1? >>> >>>> [ 0.803934] kmemleak: Cannot insert 0xff7f1000 into the object search tree (overlaps existing) >>>> [ 0.803950] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc1-next-20160809 #497 >>>> [ 0.803958] Hardware name: Generic DRA72X (Flattened Device Tree) >>>> [ 0.803979] [<c0110104>] (unwind_backtrace) from [<c010c24c>] (show_stack+0x10/0x14) >>>> [ 0.803994] [<c010c24c>] (show_stack) from [<c0490df0>] (dump_stack+0xac/0xe0) >>>> [ 0.804010] [<c0490df0>] (dump_stack) from [<c0296f88>] (create_object+0x214/0x278) >>>> [ 0.804025] [<c0296f88>] (create_object) from [<c07c770c>] (kmemleak_alloc_percpu+0x54/0xc0) >>>> [ 0.804038] [<c07c770c>] (kmemleak_alloc_percpu) from [<c025fb08>] (pcpu_alloc+0x368/0x5fc) >>>> [ 0.804052] [<c025fb08>] (pcpu_alloc) from [<c0b1bfbc>] (crash_notes_memory_init+0x10/0x40) >>>> [ 0.804064] [<c0b1bfbc>] (crash_notes_memory_init) from [<c010188c>] (do_one_initcall+0x3c/0x178) >>>> [ 0.804075] [<c010188c>] (do_one_initcall) from [<c0b00e98>] (kernel_init_freeable+0x1fc/0x2c8) >>>> [ 0.804086] [<c0b00e98>] (kernel_init_freeable) from [<c07c66b0>] (kernel_init+0x8/0x114) >>>> [ 0.804098] [<c07c66b0>] (kernel_init) from [<c0107910>] (ret_from_fork+0x14/0x24) >>> >>> This is the allocation stack trace, going via pcpu_alloc(). >>> >>>> [ 0.804106] kmemleak: Kernel memory leak detector disabled >>>> [ 0.804113] kmemleak: Object 0xfe800000 (size 16777216): >>>> [ 0.804121] kmemleak: comm "swapper/0", pid 0, jiffies 4294937296 >>>> [ 0.804127] kmemleak: min_count = -1 >>>> [ 0.804132] kmemleak: count = 0 >>>> [ 0.804138] kmemleak: flags = 0x5 >>>> [ 0.804143] kmemleak: checksum = 0 >>>> [ 0.804149] kmemleak: backtrace: >>>> [ 0.804155] [<c0b26a90>] cma_declare_contiguous+0x16c/0x214 >>>> [ 0.804170] [<c0b3c9c0>] dma_contiguous_reserve_area+0x30/0x64 >>>> [ 0.804183] [<c0b3ca74>] dma_contiguous_reserve+0x80/0x94 >>>> [ 0.804195] [<c0b06810>] arm_memblock_init+0x130/0x184 >>>> [ 0.804207] [<c0b04214>] setup_arch+0x590/0xc08 >>>> [ 0.804217] [<c0b00940>] start_kernel+0x58/0x3b4 >>>> [ 0.804227] [<8000807c>] 0x8000807c >>>> [ 0.804237] [<ffffffff>] 0xffffffff >>> >>> This seems to be the original object that was allocated via >>> cma_declare_contiguous(): 16MB range from 0xfe800000 to 0xff800000. >>> Since the pointer returned by pcpu_alloc is 0xff7f1000 falls in the 16MB >>> CMA range, kmemleak gets confused (it doesn't allow overlapping >>> objects). >>> >>> So what I think goes wrong is that the kmemleak_alloc(__va(found)) call >>> in memblock_alloc_range_nid() doesn't get the right value for the VA of >>> the CMA block. The memblock_alloc_range() call in >>> cma_declare_contiguous() asks for memory above high_memory, hence on a >>> 32-bit architecture with highmem enabled, __va() use is not really >>> valid, returning the wrong address. The existing kmemleak object is >>> bogus, it shouldn't have been created in the first place. >>> >>> Now I'm trying to figure out how to differentiate between lowmem >>> memblocks and highmem ones. Ignoring the kmemleak_alloc() calls >>> altogether in mm/memblock.c is probably not an option as it would lead >>> to lots of false positives. >> >> But cma_declare_contiguous() calls - >> /* >> * kmemleak scans/reads tracked objects for pointers to other >> * objects but this address isn't mapped and accessible >> */ >> kmemleak_ignore(phys_to_virt(addr)); >> >> Does it means above code is incorrect also? > > Yes, as long as the phys_to_virt() use is invalid. You may get away with > this, depending on the SoC. Also, kmemleak_ignore() here is meant to > tell kmemleak not to bother with scanning or reporting such memory since > it is not meant for pointers but it still keeps track of it. The only > way to remove it from kmemleak is replace this with kmemleak_free(). But > That's more of a hack since phys_to_virt(addr) is still invalid. > >> It's a little bit strange that this can be seen only now, because >> commit 95b0e655f9 ("ARM: mm: don't limit default CMA region only to low memor") >> is pretty old. > > You might want to double check my scenario above but I guess we've been > lucky. So either some configuration changed and arm_dma_limit > > arm_lowmem_limit or the random VA for the CMA memory didn't overlap with > any other block. > Thanks a lot for explanation.
diff --git a/mm/memblock.c b/mm/memblock.c index 483197ef613f..7d3361d53ac2 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -723,7 +723,8 @@ int __init_memblock memblock_free(phys_addr_t base, phys_addr_t size) (unsigned long long)base + size - 1, (void *)_RET_IP_); - kmemleak_free_part(__va(base), size); + if (base < __pa(high_memory)) + kmemleak_free_part(__va(base), size); return memblock_remove_range(&memblock.reserved, base, size); } @@ -1152,7 +1153,8 @@ static phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size, * The min_count is set to 0 so that memblock allocations are * never reported as leaks. */ - kmemleak_alloc(__va(found), size, 0, 0); + if (found < __pa(high_memory)) + kmemleak_alloc(__va(found), size, 0, 0); return found; } return 0; @@ -1399,7 +1401,8 @@ void __init __memblock_free_early(phys_addr_t base, phys_addr_t size) memblock_dbg("%s: [%#016llx-%#016llx] %pF\n", __func__, (u64)base, (u64)base + size - 1, (void *)_RET_IP_); - kmemleak_free_part(__va(base), size); + if (base < __pa(high_memory)) + kmemleak_free_part(__va(base), size); memblock_remove_range(&memblock.reserved, base, size); } @@ -1419,7 +1422,8 @@ void __init __memblock_free_late(phys_addr_t base, phys_addr_t size) memblock_dbg("%s: [%#016llx-%#016llx] %pF\n", __func__, (u64)base, (u64)base + size - 1, (void *)_RET_IP_); - kmemleak_free_part(__va(base), size); + if (base < __pa(high_memory)) + kmemleak_free_part(__va(base), size); cursor = PFN_UP(base); end = PFN_DOWN(base + size);