Re: Potential Double Scanning of struct page Physical Memory by kmemleak

[Catalin Marinas <catalin.marinas@arm.com>]

Hi Guo,

On Wed, Dec 25, 2024 at 04:09:12PM +0800, Weikang Guo wrote:
> Problem Description:
> 
> When CONFIG_SPARSEMEM is enabled, the memory for `struct page`objects
> in the sections is allocated via `sparse_buffer` using the
> `memblock_alloc_xxx` interface. Since memblock does not explicitly
> specify `MEMBLOCK_ALLOC_NOLEAKTRACE`, kmemleak will treat the memory
> as a gray object with mincount=0. In other words, the physical memory
> occupied by `struct page` will be scanned by kmemleak as a gray object
> during the scan thread.
> 
> Additionally, kmemleak also traverses and scans valid struct page
> objects in the zone . As a result, the physical memory occupied by
> struct page may end up being scanned twice by scan thread.

Yes, I can see how this happens. I don't remember how we ended up like
this, maybe kmemleak did not track memblock allocations in the early
days.

> Possible solution: Specify `MEMBLOCK_ALLOC_NOLEAKTRACE` when alloc
> `struct page memory`

I think that's the easiest and just let kmemleak scan the mem_map
explicitly, whether it's in the linear map or vmemmap.

The way we ended up with marking 'nokleaktrace' blocks in the memblock
API isn't great. This "flag" used to be called MEMBLOCK_ALLOC_KASAN and
only used by KASAN (implying accessible). But it's not an actual flag,
just some random value passed as the 'end' argument to memblock_alloc()
and friends. Luckily memmap_alloc() only needs MEMBLOCK_ALLOC_ACCESSIBLE
which is implied by MEMBLOCK_ALLOC_NOLEAKTRACE (though I can't find any
documentation about this).

Anyway, if you fix memmap_alloc(), please add a comment that kmemleak
scans this explicitly. Also add a comment where we define
MEMBLOCK_ALLOC_NOLEAKTRACE to state that it implies
MEMBLOCK_ALLOC_ACCESSIBLE. Ideally we should have named this
MEMBLOCK_ALLOC_ACCESSIBLE_NOLEAKTRACE but it's nearly half the
recommended line length.

Thanks.

-- 
Catalin