Skip to content

Commit

Permalink
ftrace - fix dynamic ftrace memory leak
Browse files Browse the repository at this point in the history
The ftrace dynamic function update allocates a record to store the
instruction pointers that are being modified. If the modified
instruction pointer fails to update, then the record is marked as
failed and nothing more is done.

Worse, if the modification fails, but the record ip function is still
called, it will allocate a new record and try again. In just a matter
of time, will this cause a serious memory leak and crash the system.

This patch plugs this memory leak. When a record fails, it is
included back into the pool of records to be used. Now a record may
fail over and over again, but the number of allocated records will
not increase.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
  • Loading branch information
Steven Rostedt authored and Thomas Gleixner committed May 23, 2008
1 parent 088b1e4 commit 37ad508
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 6 deletions.
7 changes: 4 additions & 3 deletions include/linux/ftrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@ extern void mcount(void);
# define FTRACE_HASHSIZE (1<<FTRACE_HASHBITS)

enum {
FTRACE_FL_FAILED = (1 << 0),
FTRACE_FL_FILTER = (1 << 1),
FTRACE_FL_ENABLED = (1 << 2),
FTRACE_FL_FREE = (1 << 0),
FTRACE_FL_FAILED = (1 << 1),
FTRACE_FL_FILTER = (1 << 2),
FTRACE_FL_ENABLED = (1 << 3),
};

struct dyn_ftrace {
Expand Down
45 changes: 42 additions & 3 deletions kernel/trace/ftrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ static int ftraced_suspend;

static int ftrace_record_suspend;

static struct dyn_ftrace *ftrace_free_records;

static inline int
notrace ftrace_ip_in_hash(unsigned long ip, unsigned long key)
{
Expand All @@ -211,8 +213,35 @@ ftrace_add_hash(struct dyn_ftrace *node, unsigned long key)
hlist_add_head(&node->node, &ftrace_hash[key]);
}

static notrace void ftrace_free_rec(struct dyn_ftrace *rec)
{
/* no locking, only called from kstop_machine */

rec->ip = (unsigned long)ftrace_free_records;
ftrace_free_records = rec;
rec->flags |= FTRACE_FL_FREE;
}

static notrace struct dyn_ftrace *ftrace_alloc_dyn_node(unsigned long ip)
{
struct dyn_ftrace *rec;

/* First check for freed records */
if (ftrace_free_records) {
rec = ftrace_free_records;

/* todo, disable tracing altogether on this warning */
if (unlikely(!(rec->flags & FTRACE_FL_FREE))) {
WARN_ON_ONCE(1);
ftrace_free_records = NULL;
return NULL;
}

ftrace_free_records = (void *)rec->ip;
memset(rec, 0, sizeof(*rec));
return rec;
}

if (ftrace_pages->index == ENTRIES_PER_PAGE) {
if (!ftrace_pages->next)
return NULL;
Expand Down Expand Up @@ -356,8 +385,16 @@ __ftrace_replace_code(struct dyn_ftrace *rec,
}

failed = ftrace_modify_code(ip, old, new);
if (failed)
rec->flags |= FTRACE_FL_FAILED;
if (failed) {
unsigned long key;
/* It is possible that the function hasn't been converted yet */
key = hash_long(ip, FTRACE_HASHBITS);
if (!ftrace_ip_in_hash(ip, key)) {
rec->flags |= FTRACE_FL_FAILED;
ftrace_free_rec(rec);
}

}
}

static void notrace ftrace_replace_code(int enable)
Expand Down Expand Up @@ -407,8 +444,10 @@ ftrace_code_disable(struct dyn_ftrace *rec)
call = ftrace_call_replace(ip, MCOUNT_ADDR);

failed = ftrace_modify_code(ip, call, nop);
if (failed)
if (failed) {
rec->flags |= FTRACE_FL_FAILED;
ftrace_free_rec(rec);
}
}

static int notrace __ftrace_modify_code(void *data)
Expand Down

0 comments on commit 37ad508

Please sign in to comment.