[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHkRjk5KLR1390eiRdXcNmppVKWBgugutsuvhpesMwXYa6CRzA@mail.gmail.com>
Date: Thu, 23 Oct 2014 22:09:12 +0100
From: Catalin Marinas <catalin.marinas@....com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Thierry Reding <thierry.reding@...il.com>,
Marek Szyprowski <m.szyprowski@...sung.com>,
linux-mm <linux-mm@...ck.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm/cma: Make kmemleak ignore CMA regions
On 23 October 2014 21:22, Andrew Morton <akpm@...ux-foundation.org> wrote:
> On Tue, 21 Oct 2014 14:14:56 +0200 Thierry Reding <thierry.reding@...il.com> wrote:
>
>> From: Thierry Reding <treding@...dia.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@...dia.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists