Re: [RFC][PATCH 0/1] kmemleak: Fix false positive with alias

[Hiroshi DOYU]

Hi Catalin,

From: Hiroshi DOYU <Hiroshi.DOYU@xxxxxxxxx>
Subject: Re: [RFC][PATCH 0/1] kmemleak: Fix false positive with alias
Date: Tue, 29 Jun 2010 07:44:23 +0300 (EEST)

> From: ext Catalin Marinas <catalin.marinas@xxxxxxx>
> Subject: Re: [RFC][PATCH 0/1] kmemleak: Fix false positive with alias
> Date: Mon, 28 Jun 2010 16:46:12 +0200
> 
>> Hi,
>> 
>> (and sorry for the delay)
>> 
>> On Fri, 2010-06-18 at 07:04 +0100, Hiroshi DOYU wrote:
>>> This is another version of "kmemleak: Fix false positive", which
>>> introduces another alias tree to keep track of all alias address of
>>> each objects, based on the discussion(*1)
>>> 
>>> You can also find the previous one(*2), which uses special scan area
>>> for alias addresses with a conversion function.
>>> 
>>> Compared with both methods, it seems that the current one takes a bit
>>> longer to scan as below, tested with 512 elementes of (*3).
>>> 
>>> "kmemleak: Fix false positive with alias":
>>> # time echo scan > /mnt/kmemleak
>>> real    0m 8.40s
>>> user    0m 0.00s
>>> sys     0m 8.40s
>>> 
>>> "kmemleak: Fix false positive with special scan":
>>> # time echo scan > /mnt/kmemleak
>>> real    0m 3.96s
>>> user    0m 0.00s
>>> sys     0m 3.96s
>> 
>> Have you tried without your patches (just the test module but without
>> aliasing the pointers)? I'm curious what's the impact of your first set
>> of patches.
> 
> IIRC, not much difference against the one with the first patches. I'll
> measure it again later.
> 
>>> For our case(*4) to reduce false positives for the 2nd level IOMMU
>>> pagetable allocation, the previous special scan  seems to be enough
>>> lightweight, although there might be possiblity to improve alias
>>> one and also I might misunderstand the original proposal of aliasing.
>> 
>> The performance impact is indeed pretty high, though some parts of the
>> code look over-engineered to me (the __scan_block function with a loop
>> going through an array of two function pointers - I think the compiler
>> cannot figure out what to inline). You could just extend the
>> find_and_get_object() to search both trees under a single spinlock
>> region (as locking also takes time).
> 
> Ok, a good point.

Now there's not much difference with the attached patch, a new version
of alias.

/ # modprobe kmemleak-special-test use_alias=0
/ # time echo scan > /sys/kernel/debug/kmemleak
real    0m 2.30s
user    0m 0.00s
sys     0m 2.30s

/ # modprobe kmemleak-special-test use_alias=1
/ # time echo scan > /sys/kernel/debug/kmemleak
real    0m 3.91s
user    0m 0.00s
sys     0m 3.91s