This is my second revision, now tested. Does the job I was looking for.
I fixed two issues from my previous untested revision, I removed the scan
lock, and also fixed the strncmp() count.
My kmemleak output was too polluted to do on the fly debugging, adding
a clear command lets you test the sections you want when you want with
a cleared fresh list of kmemleak objects.
Still not sure who this goes through so sending this to Linus.
Luis R. Rodriguez (5):
kmemleak: use bool for true/false questions
kmemleak: add clear command support
kmemleak: move common painting code together
kmemleak: fix sparse warning over overshadowed flags
kmemleak: fix sparse warning for static declarations
Documentation/kmemleak.txt | 17 ++++++++++
mm/kmemleak.c | 72 ++++++++++++++++++++++++++++++-------------
2 files changed, 67 insertions(+), 22 deletions(-)
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
mm/kmemleak.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 4872673..c6e71bb 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -264,17 +264,17 @@ static void kmemleak_disable(void);
* Newly created objects don't have any color assigned (object->count == -1)
* before the next memory scan when they become white.
*/
-static int color_white(const struct kmemleak_object *object)
+static bool color_white(const struct kmemleak_object *object)
{
return object->count != -1 && object->count < object->min_count;
}
-static int color_gray(const struct kmemleak_object *object)
+static bool color_gray(const struct kmemleak_object *object)
{
return object->min_count != -1 && object->count >= object->min_count;
}
-static int color_black(const struct kmemleak_object *object)
+static bool color_black(const struct kmemleak_object *object)
{
return object->min_count == -1;
}
@@ -284,7 +284,7 @@ static int color_black(const struct kmemleak_object *object)
* not be deleted and have a minimum age to avoid false positives caused by
* pointers temporarily stored in CPU registers.
*/
-static int unreferenced_object(struct kmemleak_object *object)
+static bool unreferenced_object(struct kmemleak_object *object)
{
return (object->flags & OBJECT_ALLOCATED) && color_white(object) &&
time_before_eq(object->jiffies + jiffies_min_age,
--
1.6.3.3
In an ideal world your kmemleak output will be small,
when its not you can use the clear command to ingore previously
annotated kmemleak objects. We do this by painting them black.
You can now use 'clear'; run some critical section code you'd
like to test, and then run 'scan'.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
Documentation/kmemleak.txt | 17 +++++++++++++++++
mm/kmemleak.c | 22 ++++++++++++++++++++++
2 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/Documentation/kmemleak.txt b/Documentation/kmemleak.txt
index 8906803..235090b 100644
--- a/Documentation/kmemleak.txt
+++ b/Documentation/kmemleak.txt
@@ -27,6 +27,13 @@ To trigger an intermediate memory scan:
# echo scan > /sys/kernel/debug/kmemleak
+To clear the list of all current possible memory leaks:
+
+ # echo clear > /sys/kernel/debug/kmemleak
+
+New leaks will then come up upon reading /sys/kernel/debug/kmemleak
+again.
+
Note that the orphan objects are listed in the order they were allocated
and one object at the beginning of the list may cause other subsequent
objects to be reported as orphan.
@@ -35,6 +42,8 @@ Memory scanning parameters can be modified at run-time by writing to the
/sys/kernel/debug/kmemleak file. The following parameters are supported:
off - disable kmemleak (irreversible)
+ clear - clear list of current memory leak suspects, done by
+ marking all current kmemleak objects as black.
stack=on - enable the task stacks scanning (default)
stack=off - disable the tasks stacks scanning
scan=on - start the automatic memory scanning thread (default)
@@ -79,6 +88,8 @@ The scanning algorithm steps:
gray set is finished
4. the remaining white objects are considered orphan and reported via
/sys/kernel/debug/kmemleak
+ 5. All black objects are ignored on the output of
+ /sys/kernel/debug/kmemleak
Some allocated memory blocks have pointers stored in the kernel's
internal data structures and they cannot be detected as orphans. To
@@ -86,6 +97,12 @@ avoid this, kmemleak can also store the number of values pointing to an
address inside the block address range that need to be found so that the
block is not considered a leak. One example is __vmalloc().
+Testing a sections with kmemleak
+--------------------------------
+
+You can use the 'clear' and 'scan' command over certain critical sections you
+would like to test.
+
Kmemleak API
------------
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index c6e71bb..b6bc34e 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1294,10 +1294,30 @@ static int kmemleak_release(struct inode *inode, struct file *file)
return seq_release(inode, file);
}
+static void kmemleak_clear(void)
+{
+ struct kmemleak_object *object;
+ unsigned long flags;
+
+ stop_scan_thread();
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(object, &object_list, object_list) {
+ spin_lock_irqsave(&object->lock, flags);
+ object->min_count = -1;
+ spin_unlock_irqrestore(&object->lock, flags);
+ }
+ rcu_read_unlock();
+
+ start_scan_thread();
+}
+
/*
* File write operation to configure kmemleak at run-time. The following
* commands can be written to the /sys/kernel/debug/kmemleak file:
* off - disable kmemleak (irreversible)
+ * clear - mark all current kmemleak objects as black to ingore
+ * printing them
* stack=on - enable the task stacks scanning
* stack=off - disable the tasks stacks scanning
* scan=on - start the automatic memory scanning thread
@@ -1324,6 +1344,8 @@ static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
if (strncmp(buf, "off", 3) == 0)
kmemleak_disable();
+ else if (strncmp(buf, "clear", 5) == 0)
+ kmemleak_clear();
else if (strncmp(buf, "stack=on", 8) == 0)
kmemleak_stack_scan = 1;
else if (strncmp(buf, "stack=off", 9) == 0)
--
1.6.3.3
When painting grey or black we do the same thing, bring
this together into a helper and identify coloring grey or
black explicitly with defines. This makes this a little
easier to read.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
mm/kmemleak.c | 32 +++++++++++++++++++-------------
1 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index b6bc34e..58c07b1 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -120,6 +120,9 @@ struct kmemleak_scan_area {
size_t length;
};
+#define KMEMLEAK_GREY 0
+#define KMEMLEAK_BLACK -1
+
/*
* Structure holding the metadata for each allocated memory block.
* Modifications to such objects should be made while holding the
@@ -266,17 +269,19 @@ static void kmemleak_disable(void);
*/
static bool color_white(const struct kmemleak_object *object)
{
- return object->count != -1 && object->count < object->min_count;
+ return object->count != KMEMLEAK_BLACK &&
+ object->count < object->min_count;
}
static bool color_gray(const struct kmemleak_object *object)
{
- return object->min_count != -1 && object->count >= object->min_count;
+ return object->min_count != KMEMLEAK_BLACK &&
+ object->count >= object->min_count;
}
static bool color_black(const struct kmemleak_object *object)
{
- return object->min_count == -1;
+ return object->min_count == KMEMLEAK_BLACK;
}
/*
@@ -604,13 +609,21 @@ static void delete_object_part(unsigned long ptr, size_t size)
put_object(object);
}
+
+static void paint_it(struct kmemleak_object *object, int color)
+{
+ unsigned long flags;
+ spin_lock_irqsave(&object->lock, flags);
+ object->min_count = color;
+ spin_unlock_irqrestore(&object->lock, flags);
+}
+
/*
* Make a object permanently as gray-colored so that it can no longer be
* reported as a leak. This is used in general to mark a false positive.
*/
static void make_gray_object(unsigned long ptr)
{
- unsigned long flags;
struct kmemleak_object *object;
object = find_and_get_object(ptr, 0);
@@ -618,10 +631,7 @@ static void make_gray_object(unsigned long ptr)
kmemleak_warn("Graying unknown object at 0x%08lx\n", ptr);
return;
}
-
- spin_lock_irqsave(&object->lock, flags);
- object->min_count = 0;
- spin_unlock_irqrestore(&object->lock, flags);
+ paint_it(object, KMEMLEAK_GREY);
put_object(object);
}
@@ -631,7 +641,6 @@ static void make_gray_object(unsigned long ptr)
*/
static void make_black_object(unsigned long ptr)
{
- unsigned long flags;
struct kmemleak_object *object;
object = find_and_get_object(ptr, 0);
@@ -639,10 +648,7 @@ static void make_black_object(unsigned long ptr)
kmemleak_warn("Blacking unknown object at 0x%08lx\n", ptr);
return;
}
-
- spin_lock_irqsave(&object->lock, flags);
- object->min_count = -1;
- spin_unlock_irqrestore(&object->lock, flags);
+ paint_it(object, KMEMLEAK_BLACK);
put_object(object);
}
--
1.6.3.3
This fixes this sparse warning:
mm/kmemleak.c:512:31: warning: symbol 'flags' shadows an earlier one
mm/kmemleak.c:448:23: originally declared here
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
mm/kmemleak.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 58c07b1..24e7a84 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -509,14 +509,14 @@ static void create_object(unsigned long ptr, size_t size, int min_count,
* random memory blocks.
*/
if (node != &object->tree_node) {
- unsigned long flags;
+ unsigned long flags_object;
kmemleak_stop("Cannot insert 0x%lx into the object search tree "
"(already existing)\n", ptr);
object = lookup_object(ptr, 1);
- spin_lock_irqsave(&object->lock, flags);
+ spin_lock_irqsave(&object->lock, flags_object);
dump_object_info(object);
- spin_unlock_irqrestore(&object->lock, flags);
+ spin_unlock_irqrestore(&object->lock, flags_object);
goto out;
}
--
1.6.3.3
This fixes these sparse warnings:
mm/kmemleak.c:1179:6: warning: symbol 'start_scan_thread' was not declared. Should it be static?
mm/kmemleak.c:1194:6: warning: symbol 'stop_scan_thread' was not declared. Should it be static?
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
mm/kmemleak.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 24e7a84..262e155 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1176,7 +1176,7 @@ static int kmemleak_scan_thread(void *arg)
* Start the automatic memory scanning thread. This function must be called
* with the scan_mutex held.
*/
-void start_scan_thread(void)
+static void start_scan_thread(void)
{
if (scan_thread)
return;
@@ -1191,7 +1191,7 @@ void start_scan_thread(void)
* Stop the automatic memory scanning thread. This function must be called
* with the scan_mutex held.
*/
-void stop_scan_thread(void)
+static void stop_scan_thread(void)
{
if (scan_thread) {
kthread_stop(scan_thread);
--
1.6.3.3
Hi Luis,
On Thu, Sep 3, 2009 at 8:35 AM, Luis R. Rodriguez<[email protected]> wrote:
> Still not sure who this goes through so sending this to Linus.
Time zone difference alert! I'm sorry but I haven't yet figured out
how to answer my emails while I'm sleeping. :-)
Seriously though, the patches should go through Catalin's git tree
that's hiding somewhere.
Pekka
On Thu, 2009-09-03 at 01:35 -0400, Luis R. Rodriguez wrote:
> Still not sure who this goes through so sending this to Linus.
As Pekka said, in some parts of the world the working day just started.
BTW, as I'm the kmemleak maintainer, the patches should probably go
through me (or at least get an ack), especially since I have other
kmemleak patches to push (the kmemleak branch on
git://linux-arm.org/linux-2.6.git).
Thanks. I'll have a look at the patches today.
--
Catalin
On Thu, 2009-09-03 at 01:35 -0400, Luis R. Rodriguez wrote:
> In an ideal world your kmemleak output will be small,
> when its not you can use the clear command to ingore previously
> annotated kmemleak objects. We do this by painting them black.
Making the objects "black" means that they are completely ignored by
kmemleak and they are assumed not to contain any valid references.
Therefore they won't be scanned and many of the newly allocated objects
would be false positives.
You may want to make them "gray" and only those which were reported as
unreferenced, something like below:
if ((object->flags & OBJECT_REPORTED) && unreferenced_object(object))
make_gray_object(object->pointer)
--
Catalin
On Thu, 2009-09-03 at 01:35 -0400, Luis R. Rodriguez wrote:
> This fixes this sparse warning:
> mm/kmemleak.c:512:31: warning: symbol 'flags' shadows an earlier one
> mm/kmemleak.c:448:23: originally declared here
>
> Signed-off-by: Luis R. Rodriguez <[email protected]>
> ---
> mm/kmemleak.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 58c07b1..24e7a84 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -509,14 +509,14 @@ static void create_object(unsigned long ptr, size_t size, int min_count,
> * random memory blocks.
> */
> if (node != &object->tree_node) {
> - unsigned long flags;
> + unsigned long flags_object;
>
> kmemleak_stop("Cannot insert 0x%lx into the object search tree "
> "(already existing)\n", ptr);
> object = lookup_object(ptr, 1);
> - spin_lock_irqsave(&object->lock, flags);
> + spin_lock_irqsave(&object->lock, flags_object);
> dump_object_info(object);
> - spin_unlock_irqrestore(&object->lock, flags);
> + spin_unlock_irqrestore(&object->lock, flags_object);
As Pekka said, we only need spin_lock() variant here as the interrupts
are already disabled.
--
Catalin
On Thu, Sep 3, 2009 at 9:55 AM, Catalin Marinas<[email protected]> wrote:
> On Thu, 2009-09-03 at 01:35 -0400, Luis R. Rodriguez wrote:
>> This fixes this sparse warning:
>> mm/kmemleak.c:512:31: warning: symbol 'flags' shadows an earlier one
>> mm/kmemleak.c:448:23: originally declared here
>>
>> Signed-off-by: Luis R. Rodriguez <[email protected]>
>> ---
>> mm/kmemleak.c | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
>> index 58c07b1..24e7a84 100644
>> --- a/mm/kmemleak.c
>> +++ b/mm/kmemleak.c
>> @@ -509,14 +509,14 @@ static void create_object(unsigned long ptr, size_t size, int min_count,
>> * random memory blocks.
>> */
>> if (node != &object->tree_node) {
>> - unsigned long flags;
>> + unsigned long flags_object;
>>
>> kmemleak_stop("Cannot insert 0x%lx into the object search tree "
>> "(already existing)\n", ptr);
>> object = lookup_object(ptr, 1);
>> - spin_lock_irqsave(&object->lock, flags);
>> + spin_lock_irqsave(&object->lock, flags_object);
>> dump_object_info(object);
>> - spin_unlock_irqrestore(&object->lock, flags);
>> + spin_unlock_irqrestore(&object->lock, flags_object);
>
> As Pekka said, we only need spin_lock() variant here as the interrupts
> are already disabled.
OK will respin thanks.
Luis
On Thu, Sep 03, 2009 at 01:30:13AM -0700, Catalin Marinas wrote:
> On Thu, 2009-09-03 at 01:35 -0400, Luis R. Rodriguez wrote:
> > Still not sure who this goes through so sending this to Linus.
>
> As Pekka said, in some parts of the world the working day just started.
> BTW, as I'm the kmemleak maintainer, the patches should probably go
> through me (or at least get an ack), especially since I have other
> kmemleak patches to push (the kmemleak branch on
> git://linux-arm.org/linux-2.6.git).
I'll respin on this tree with some more changes requested.
Luis
On Thu, Sep 03, 2009 at 09:52:11AM -0700, Catalin Marinas wrote:
> On Thu, 2009-09-03 at 01:35 -0400, Luis R. Rodriguez wrote:
> > In an ideal world your kmemleak output will be small,
> > when its not you can use the clear command to ingore previously
> > annotated kmemleak objects. We do this by painting them black.
>
> Making the objects "black" means that they are completely ignored by
> kmemleak and they are assumed not to contain any valid references.
> Therefore they won't be scanned and many of the newly allocated objects
> would be false positives.
Got it, BTW can you elaborate as to why painting objects black would
create false positives for newly allocated objects? I fail to understand
why.
> You may want to make them "gray" and only those which were reported as
> unreferenced, something like below:
>
> if ((object->flags & OBJECT_REPORTED) && unreferenced_object(object))
> make_gray_object(object->pointer)
Thanks, will use this.
Luis
On Fri, 2009-09-04 at 13:26 -0700, Luis R. Rodriguez wrote:
> On Thu, Sep 03, 2009 at 09:52:11AM -0700, Catalin Marinas wrote:
> > On Thu, 2009-09-03 at 01:35 -0400, Luis R. Rodriguez wrote:
> > > In an ideal world your kmemleak output will be small,
> > > when its not you can use the clear command to ingore previously
> > > annotated kmemleak objects. We do this by painting them black.
> >
> > Making the objects "black" means that they are completely ignored by
> > kmemleak and they are assumed not to contain any valid references.
> > Therefore they won't be scanned and many of the newly allocated objects
> > would be false positives.
>
> Got it, BTW can you elaborate as to why painting objects black would
> create false positives for newly allocated objects? I fail to understand
> why.
Black objects are not scanned by kmemleak. Making valid objects black as
per your patch, they may (actually quite likely) contain pointers to
newly allocated objects. Since they won't be scanned, the newly
allocated objects are reported as unreferenced as no pointers to them
were found during scanning.
--
Catalin