Re: [PATCH] mm/cma: Make kmemleak ignore CMA regions

[Catalin Marinas]

On 23 October 2014 21:22, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 21 Oct 2014 14:14:56 +0200 Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
>
>> From: Thierry Reding <treding@xxxxxxxxxx>
>>
>> kmemleak will add allocations as objects to a pool. The memory allocated
>> for each object in this pool is periodically searched for pointers to
>> other allocated objects. This only works for memory that is mapped into
>> the kernel's virtual address space, which happens not to be the case for
>> most CMA regions.
>>
>> Furthermore, CMA regions are typically used to store data transferred to
>> or from a device and therefore don't contain pointers to other objects.
>>
>> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
>> ---
>> Note: I'm not sure this is really the right fix. But without this, the
>> kernel crashes on the first execution of the scan_gray_list() because
>> it tries to access highmem. Perhaps a more appropriate fix would be to
>> reject any object that can't map to a kernel virtual address?
>
> Let's cc Catalin.
>
>> --- a/mm/cma.c
>> +++ b/mm/cma.c
>> @@ -280,6 +280,7 @@ int __init cma_declare_contiguous(phys_addr_t base,
>>                       ret = -ENOMEM;
>>                       goto err;
>>               } else {
>> +                     kmemleak_ignore(phys_to_virt(addr));
>>                       base = addr;
>>               }
>>       }

I wonder whether using __va() for the argument of kmemleak_alloc() in
memblock_alloc_range_nid() is always correct. Is
memblock.current_limit guaranteed to be in lowmem? If not, I think we
need some logic not to call kmemleak_alloc() for all memblock
allocations (and avoid the need to ignore them later).

> And let's tell our poor readers why we did stuff.  Something like this.
>
> --- a/mm/cma.c~mm-cma-make-kmemleak-ignore-cma-regions-fix
> +++ a/mm/cma.c
> @@ -280,6 +280,10 @@ int __init cma_declare_contiguous(phys_a
>                         ret = -ENOMEM;
>                         goto err;
>                 } else {
> +                       /*
> +                        * kmemleak writes metadata to the tracked objects, but
> +                        * this address isn't mapped and accessible.
> +                        */
>                         kmemleak_ignore(phys_to_virt(addr));
>                         base = addr;
>                 }

The reason is different, as per Therry's patch description. Kmemleak
does not write metadata to the tracked objects but reads them during
memory scanning. So maybe something like "kmemleak scans/reads tracked
objects for pointers to other objects but this address isn't mapped
and accessible."

A better API to use here would have been kmemleak_no_scan(), however,
I don't think we care about such CMA pointers anyway since they seem
to be tracked by physical address which kmemleak doesn't store.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/