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. > Anyway, you still get to search two trees for any pointer so there would > always be some performance impact. I just hoped they weren't be as bad. > In a normal system (not test module), how many elements would the alias > tree have? Just in our case, it's 512 at most. > Another approach - if we assume that there is a single alias per object > and such aliases don't overlap, we could just move (delete + re-insert) > the corresponding kmemleak_object in the tree to the alias position. > This way we only keep a single tree and a single object for an allocated > block. But in your use-case, the physical address of an object may > actually match the virtual address of a different object, so > lookup_object() needs to be iterative. You need two new kmemleak API > functions, e.g. kmemleak_alias() and kmemleak_unalias(), to be called > after allocation and before freeing a memory block. > > If we can't make the performance hit acceptable, we could go for the > first approach and maybe just extend kmemleak_scan_area with a function > pointer structure rather than adding a new one. But as I said > previously, my main issue with your original approach is that I would > prefer to call the kmemleak API at the point where the false positive is > allocated rather than where the parent object was. I'll try both and measure their impact again. Thank you for your comment. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
