lkml.org 
[lkml]   [2014]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] mm/cma: Make kmemleak ignore CMA regions
On 23 October 2014 21:22, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Tue, 21 Oct 2014 14:14:56 +0200 Thierry Reding <thierry.reding@gmail.com> wrote:
>
>> From: Thierry Reding <treding@nvidia.com>
>>
>> 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@nvidia.com>
>> ---
>> 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


\
 
 \ /
  Last update: 2014-10-23 23:41    [W:0.053 / U:0.804 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site