Re: [RFC PATCH v2] iommu/amd: gray the 'irq_remap_table' object for kmemleak

From: Catalin Marinas
Date: Wed Dec 02 2015 - 05:52:29 EST


On 2 December 2015 at 10:37, Michael Wang <yun.wang@xxxxxxxxxxxxxxxx> wrote:
> On 11/25/2015 04:14 PM, Michael Wang wrote:
>> On 11/25/2015 04:08 PM, Joerg Roedel wrote:
> [snip]
>>>> This is caused by the 'irq_lookup_table' was allocated with
>>>> __get_free_pages() which won't create kmemleak object, thus it's
>>>> pointers won't be count as referencing 'irq_remap_table' in
>>>> kmemleak scan.
>>>
>>> Isn't it better to allocate the kmemleak object manually instead of
>>> ignoring all irq-table pointers? With this patch we might not notice any
>>> real leak of irq-tables.
>>
>> We've considered that too, but found that the irq-tables is not
>> dynamically alloc/free, they won't be freed once initialized, so there
>> is no leaking for such object :-)
>
> Is there any more concern? actually we just want to get rid of this
> annoying report on obj won't leak, if you're going to create obj for
> 'irq_lookup_table' that's also fine for us, or will you pick this patch?

My preference (from a kmemleak perspective) is to tell kmemleak about
the irq_lookup_table. Untested:

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 013bdfff2d4d..c41609f71cbe 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -27,6 +27,7 @@
#include <linux/amd-iommu.h>
#include <linux/export.h>
#include <linux/iommu.h>
+#include <linux/kmemleak.h>
#include <asm/pci-direct.h>
#include <asm/iommu.h>
#include <asm/gart.h>
@@ -1692,6 +1693,7 @@ static struct syscore_ops amd_iommu_syscore_ops = {

static void __init free_on_init_error(void)
{
+ kmemleak_free(irq_lookup_table);
free_pages((unsigned long)irq_lookup_table,
get_order(rlookup_table_size));

@@ -1906,6 +1908,7 @@ static int __init early_amd_iommu_init(void)
irq_lookup_table = (void *)__get_free_pages(
GFP_KERNEL | __GFP_ZERO,
get_order(rlookup_table_size));
+ kmemleak_alloc(irq_lookup_table, rlookup_table_size, 1, GFP_KERNEL);
if (!irq_lookup_table)
goto out;
}

--
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/