Re: [PATCH v2 0/3] kmemleak: Fix false positive with special scan
From: ext Catalin Marinas
Subject: Re: [PATCH v2 0/3] kmemleak: Fix false positive with special scan
Date: Wed, 2 Jun 2010 16:12:29 +0200
> On Wed, 2010-06-02 at 12:34 +0100, Hiroshi DOYU wrote:
>> From: ext Catalin Marinas
>> > Can we not add a new prio tree (or just use the existing one) for
>> > pointer aliases? The advantage is that you only have a single function
>> > to call, something like kmemleak_add_alias() and you do it at the point
>> > the value was converted.
>>
>> Actually I considered the above aliasing a little bit but I gave up
>> soon.
>>
>> I was afraid that this method might consume way more memory since this
>> just adds another member for "struct kmemleak_object", but adding a
>> single member for all objects. The number of kmemleak_object is
>> usually numerous.
>
> We could use a different tree with a "struct kmemleak_alias" structure
> which is much smaller. Something like below:
>
> struct kmemleak_alias {
> struct list_head alias_list;
> struct prio_tree_node tree_node;
> struct kmemleak_object *object;
> }
The above seems to be better than I thought. I'll give this a try.
> And an alias_list member would be added to kmemleak_object as well.
>
> Would the alias tree need to allow overlapping? Like different IOMMU
> mappings with the same address (but pointing to different physical
> memory).
Not for omap iommu.
omap iommu can have multiple instances, multiple devices can have each
own address spaces respectively. This doesn't affect this kmemleak
false positive.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] kmemleak: Fix false positive with special scan
On Wed, 2010-06-02 at 12:34 +0100, Hiroshi DOYU wrote:
> From: ext Catalin Marinas
> > Can we not add a new prio tree (or just use the existing one) for
> > pointer aliases? The advantage is that you only have a single function
> > to call, something like kmemleak_add_alias() and you do it at the point
> > the value was converted.
>
> Actually I considered the above aliasing a little bit but I gave up
> soon.
>
> I was afraid that this method might consume way more memory since this
> just adds another member for "struct kmemleak_object", but adding a
> single member for all objects. The number of kmemleak_object is
> usually numerous.
We could use a different tree with a "struct kmemleak_alias" structure
which is much smaller. Something like below:
struct kmemleak_alias {
struct list_head alias_list;
struct prio_tree_node tree_node;
struct kmemleak_object *object;
}
And an alias_list member would be added to kmemleak_object as well.
Would the alias tree need to allow overlapping? Like different IOMMU
mappings with the same address (but pointing to different physical
memory).
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] kmemleak: Fix false positive with special scan
From: Hiroshi DOYU Subject: Re: [PATCH v2 0/3] kmemleak: Fix false positive with special scan Date: Wed, 02 Jun 2010 14:34:58 +0300 (EEST) > From: ext Catalin Marinas > Subject: Re: [PATCH v2 0/3] kmemleak: Fix false positive with special scan > Date: Wed, 2 Jun 2010 12:01:24 +0200 > >> Hi, >> >> Sorry for the delay, I eventually got the time to look at your patches. > > Thank you for your review. > >> On Tue, 2010-06-01 at 11:25 +0100, Hiroshi DOYU wrote: >>> There is a false positive case that a pointer is calculated by other >>> methods than the usual container_of macro. "kmemleak_ignore" can cover >>> such a false positive, but it would loose the advantage of memory leak >>> detection. This patch allows kmemleak to work with such false >>> positives by introducing a new special memory block with a specified >>> calculation formula. A client module can register its area with a >>> conversion function, with which function kmemleak scan could calculate >>> a correct pointer. >> >> While something needs to be done to cover these situations, I'm not so >> convinced about the method as it complicates the code requiring such >> conversion by having to insert two kmemleak hooks and a callback >> function. >> >> Can we not add a new prio tree (or just use the existing one) for >> pointer aliases? The advantage is that you only have a single function >> to call, something like kmemleak_add_alias() and you do it at the point >> the value was converted. Ok, I understand now. Please ignore my previous. I'll try the above. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] kmemleak: Fix false positive with special scan
From: ext Catalin Marinas Subject: Re: [PATCH v2 0/3] kmemleak: Fix false positive with special scan Date: Wed, 2 Jun 2010 12:01:24 +0200 > Hi, > > Sorry for the delay, I eventually got the time to look at your patches. Thank you for your review. > On Tue, 2010-06-01 at 11:25 +0100, Hiroshi DOYU wrote: >> There is a false positive case that a pointer is calculated by other >> methods than the usual container_of macro. "kmemleak_ignore" can cover >> such a false positive, but it would loose the advantage of memory leak >> detection. This patch allows kmemleak to work with such false >> positives by introducing a new special memory block with a specified >> calculation formula. A client module can register its area with a >> conversion function, with which function kmemleak scan could calculate >> a correct pointer. > > While something needs to be done to cover these situations, I'm not so > convinced about the method as it complicates the code requiring such > conversion by having to insert two kmemleak hooks and a callback > function. > > Can we not add a new prio tree (or just use the existing one) for > pointer aliases? The advantage is that you only have a single function > to call, something like kmemleak_add_alias() and you do it at the point > the value was converted. Actually I considered the above aliasing a little bit but I gave up soon. I was afraid that this method might consume way more memory since this just adds another member for "struct kmemleak_object", but adding a single member for all objects. The number of kmemleak_object is usually numerous. Do you think that this increase of memory consumption is acceptable? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] kmemleak: Fix false positive with special scan
Hi, Sorry for the delay, I eventually got the time to look at your patches. On Tue, 2010-06-01 at 11:25 +0100, Hiroshi DOYU wrote: > There is a false positive case that a pointer is calculated by other > methods than the usual container_of macro. "kmemleak_ignore" can cover > such a false positive, but it would loose the advantage of memory leak > detection. This patch allows kmemleak to work with such false > positives by introducing a new special memory block with a specified > calculation formula. A client module can register its area with a > conversion function, with which function kmemleak scan could calculate > a correct pointer. While something needs to be done to cover these situations, I'm not so convinced about the method as it complicates the code requiring such conversion by having to insert two kmemleak hooks and a callback function. Can we not add a new prio tree (or just use the existing one) for pointer aliases? The advantage is that you only have a single function to call, something like kmemleak_add_alias() and you do it at the point the value was converted. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/3] kmemleak: Fix false positive with special scan
Hi, There is a false positive case that a pointer is calculated by other methods than the usual container_of macro. "kmemleak_ignore" can cover such a false positive, but it would loose the advantage of memory leak detection. This patch allows kmemleak to work with such false positives by introducing a new special memory block with a specified calculation formula. A client module can register its area with a conversion function, with which function kmemleak scan could calculate a correct pointer. For this version 2, to avoid client kernel module being unloaded before unregistering special conversion, module reference count is used. This was pointed by Phil Carmody. A typical use case could be the IOMMU pagetable allocation which stores pointers to the second level of page tables with some conversion, for example, a physical address with attribution bits. Right now I don't have other use cases but I hope that there could be some that this special scan works with. Test: # echo scan > kmemleak # modprobe kmemleak-special-test [ 1328.260162] Stored 1...@dfc5ac00 -> 9fc5ac01 [ 1328.264984] Stored 1...@dfc5b800 -> 9fc5b801 [ 1328.269500] Stored 1...@dfc5b400 -> 9fc5b401 [ 1328.273895] Stored 1...@dfc5b000 -> 9fc5b001 [ 1328.278381] Stored 1...@deb9bc00 -> 9eb9bc01 [ 1328.282714] Stored 1...@deea6c00 -> 9eea6c01 [ 1328.287139] Stored 1...@deea7c00 -> 9eea7c01 [ 1328.291473] Stored 1...@deea7800 -> 9eea7801 # echo scan > kmemleak [ 1344.062591] kmemleak: 8 new suspected memory leaks (see /sys/kernel/debug/kmemleak) # rmmod kmemleak-special-test # echo scan > kmemleak # modprobe kmemleak-special-test timeout=60 [ 71.758850] Stored 1...@dfc5b000 -> 9fc5b001 [ 71.763702] Stored 1...@dfc5b400 -> 9fc5b401 [ 71.768066] Stored 1...@dfc5b800 -> 9fc5b801 [ 71.772583] Stored 1...@dfc5bc00 -> 9fc5bc01 [ 71.776977] Stored 1...@deea6000 -> 9eea6001 [ 71.781341] Stored 1...@deea6400 -> 9eea6401 [ 71.785736] Stored 1...@deea6800 -> 9eea6801 [ 71.790069] Stored 1...@deea6c00 -> 9eea6c01 [ 71.794433] kmemleak_special_init: Registered special scan: bf000360 # echo scan > kmemleak [ 79.588836] custom_conversion: Converted 9fc5b001 -> dfc5b000 [ 79.594696] custom_conversion: Converted 9fc5b401 -> dfc5b400 [ 79.600494] custom_conversion: Converted 9fc5b801 -> dfc5b800 [ 79.606292] custom_conversion: Converted 9fc5bc01 -> dfc5bc00 [ 79.612060] custom_conversion: Converted 9eea6001 -> deea6000 [ 79.617889] custom_conversion: Converted 9eea6401 -> deea6400 [ 79.623687] custom_conversion: Converted 9eea6801 -> deea6800 [ 79.629486] custom_conversion: Converted 9eea6c01 -> deea6c00 # rmmod kmemleak-special-test rmmod: cannot unload 'kmemleak_special_test': Resource temporarily unavailable # lsmod kmemleak-special-test Module Size Used byNot tainted kmemleak_special_test 1467 1 # [ 131.800354] no_special_func: Unregistered special scan bf000360 # lsmod kmemleak-special-test Module Size Used byNot tainted kmemleak_special_test 1467 0 # rmmod kmemleak-special-test Hiroshi DOYU (3): kmemleak: Fix false positives with special scan kmemleak: Add special scan test case omap iommu: kmemleak: Fix false positive with special scan arch/arm/plat-omap/iommu.c | 19 +++ include/linux/kmemleak.h |5 ++ mm/Makefile|2 +- mm/kmemleak-special-test.c | 94 mm/kmemleak.c | 114 ++- 5 files changed, 230 insertions(+), 4 deletions(-) create mode 100644 mm/kmemleak-special-test.c -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
