* [PATCH 1/3] tracing/stat: change dummpy_cmp() to return -1
@ 2009-05-27 3:04 Li Zefan
2009-05-27 3:04 ` [PATCH 2/3] tracing/stat: remember to free root node Li Zefan
2009-05-27 3:05 ` [PATCH 3/3] tracing/stat: do some cleanups Li Zefan
0 siblings, 2 replies; 6+ messages in thread
From: Li Zefan @ 2009-05-27 3:04 UTC (permalink / raw)
To: Frederic Weisbecker, Steven Rostedt; +Cc: Ingo Molnar, LKML
Currently the output of trace_stat/workqueues is totally reversed:
# cat /debug/tracing/trace_stat/workqueues
...
1 17 17 210 37 `-blk_unplug_work+0x0/0x57
1 3779 3779 181 11 |-cfq_kick_queue+0x0/0x2f
1 3796 3796 kblockd/1:120
...
The correct output should be:
1 3796 3796 kblockd/1:120
1 3779 3779 181 11 |-cfq_kick_queue+0x0/0x2f
1 17 17 210 37 `-blk_unplug_work+0x0/0x57
It's caused by "tracing/stat: replace linked list by an rbtree for sorting"
(53059c9b67a62a3dc8c80204d3da42b9267ea5a0).
dummpy_cmp() should return -1, so rb_node will always be inserted as
right-most node in the rbtree, thus we sort the output in ascending
order.
[ Impact: fix the output of trace_stat/workqueues ]
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/trace/trace_stat.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index 2e849b5..8b7420a 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -127,7 +127,7 @@ insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
*/
static int dummy_cmp(void *p1, void *p2)
{
- return 1;
+ return -1;
}
/*
--
1.5.4.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] tracing/stat: remember to free root node
2009-05-27 3:04 [PATCH 1/3] tracing/stat: change dummpy_cmp() to return -1 Li Zefan
@ 2009-05-27 3:04 ` Li Zefan
2009-05-27 3:05 ` [PATCH 3/3] tracing/stat: do some cleanups Li Zefan
1 sibling, 0 replies; 6+ messages in thread
From: Li Zefan @ 2009-05-27 3:04 UTC (permalink / raw)
To: Frederic Weisbecker, Steven Rostedt; +Cc: Ingo Molnar, LKML
When closing a trace_stat file, we destroy the rbtree constructed during
file open, but there is memory leak that the root node is not freed.
[ Impact: fix memory leak when closing a trace_stat file ]
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/trace/trace_stat.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index 8b7420a..a80d947 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -60,8 +60,8 @@ static struct rb_node *release_next(struct rb_node *node)
return node->rb_right;
else {
if (!parent)
- return NULL;
- if (parent->rb_left == node)
+ ;
+ else if (parent->rb_left == node)
parent->rb_left = NULL;
else
parent->rb_right = NULL;
--
1.5.4.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] tracing/stat: do some cleanups
2009-05-27 3:04 [PATCH 1/3] tracing/stat: change dummpy_cmp() to return -1 Li Zefan
2009-05-27 3:04 ` [PATCH 2/3] tracing/stat: remember to free root node Li Zefan
@ 2009-05-27 3:05 ` Li Zefan
2009-05-27 3:42 ` [PATCH 3/3 -v2] " Li Zefan
1 sibling, 1 reply; 6+ messages in thread
From: Li Zefan @ 2009-05-27 3:05 UTC (permalink / raw)
To: Frederic Weisbecker, Steven Rostedt; +Cc: Ingo Molnar, LKML
- remove duplicate code in stat_seq_init()
- update comments to reflect the change from stat list to stat rbtree
[ Impact: clean up ]
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/trace/trace_stat.c | 54 +++++++++++++++++---------------------------
1 files changed, 21 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index a80d947..05611fa 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -93,10 +93,15 @@ static void destroy_session(struct stat_session *session)
typedef int (*cmp_stat_t)(void *, void *);
-static void
-insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
+static int insert_stat(struct rb_root *root, void *stat, cmp_stat_t cmp)
{
struct rb_node **new = &(root->rb_node), *parent = NULL;
+ struct stat_node *data;
+
+ data = zalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ data->stat = stat;
/*
* Figure out where to put new node
@@ -118,12 +123,13 @@ insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
rb_link_node(&data->node, parent, new);
rb_insert_color(&data->node, root);
+ return 0;
}
/*
* For tracers that don't provide a stat_cmp callback.
- * This one will force an immediate insertion on tail of
- * the list.
+ * This one will force an insertion as right-most node
+ * in the rbtree.
*/
static int dummy_cmp(void *p1, void *p2)
{
@@ -131,15 +137,14 @@ static int dummy_cmp(void *p1, void *p2)
}
/*
- * Initialize the stat list at each trace_stat file opening.
+ * Initialize the stat rbtree at each trace_stat file opening.
* All of these copies and sorting are required on all opening
* since the stats could have changed between two file sessions.
*/
static int stat_seq_init(struct stat_session *session)
{
struct tracer_stat *ts = session->ts;
- struct stat_node *new_entry;
- struct rb_root *root;
+ struct rb_root *root = &seesion->stat_root;
void *stat;
int ret = 0;
int i;
@@ -154,23 +159,12 @@ static int stat_seq_init(struct stat_session *session)
if (!stat)
goto exit;
- /*
- * The first entry. Actually this is the second, but the first
- * one (the stat_list head) is pointless.
- */
- new_entry = kzalloc(sizeof(*new_entry), GFP_KERNEL);
- if (!new_entry) {
- ret = -ENOMEM;
+ ret = insert_stat(root, stat, ts->stat_cmp);
+ if (ret)
goto exit;
- }
- root = &session->stat_root;
- insert_stat(root, new_entry, dummy_cmp);
-
- new_entry->stat = stat;
/*
- * Iterate over the tracer stat entries and store them in a sorted
- * list.
+ * Iterate over the tracer stat entries and store them in an rbtree.
*/
for (i = 1; ; i++) {
stat = ts->stat_next(stat, i);
@@ -179,22 +173,16 @@ static int stat_seq_init(struct stat_session *session)
if (!stat)
break;
- new_entry = kzalloc(sizeof(*new_entry), GFP_KERNEL);
- if (!new_entry) {
- ret = -ENOMEM;
- goto exit_free_list;
- }
-
- new_entry->stat = stat;
-
- insert_stat(root, new_entry, ts->stat_cmp);
+ ret = insert_stat(root, stat, ts->stat_cmp);
+ if (ret)
+ goto exit_free_rbtree;
}
exit:
mutex_unlock(&session->stat_mutex);
return ret;
-exit_free_list:
+exit_free_rbtree:
reset_stat_session(session);
mutex_unlock(&session->stat_mutex);
return ret;
@@ -207,7 +195,7 @@ static void *stat_seq_start(struct seq_file *s, loff_t *pos)
struct rb_node *node;
int i;
- /* Prevent from tracer switch or stat_list modification */
+ /* Prevent from tracer switch or rbtree modification */
mutex_lock(&session->stat_mutex);
/* If we are in the beginning of the file, print the headers */
@@ -285,7 +273,7 @@ static int tracing_stat_open(struct inode *inode, struct file *file)
}
/*
- * Avoid consuming memory with our now useless list.
+ * Avoid consuming memory with our now useless rbtree.
*/
static int tracing_stat_release(struct inode *i, struct file *f)
{
--
1.5.4.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3 -v2] tracing/stat: do some cleanups
2009-05-27 3:05 ` [PATCH 3/3] tracing/stat: do some cleanups Li Zefan
@ 2009-05-27 3:42 ` Li Zefan
0 siblings, 0 replies; 6+ messages in thread
From: Li Zefan @ 2009-05-27 3:42 UTC (permalink / raw)
To: Frederic Weisbecker, Steven Rostedt; +Cc: Ingo Molnar, LKML
- remove duplicate code in stat_seq_init()
- update comments to reflect the change from stat list to stat rbtree
[ Impact: clean up ]
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
Please ignore the previous patch. I fixed compile errors but forgot
to "git-add && git-commit --amend"
---
kernel/trace/trace_stat.c | 54 +++++++++++++++++---------------------------
1 files changed, 21 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index a80d947..e8b50f3 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -93,10 +93,15 @@ static void destroy_session(struct stat_session *session)
typedef int (*cmp_stat_t)(void *, void *);
-static void
-insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
+static int insert_stat(struct rb_root *root, void *stat, cmp_stat_t cmp)
{
struct rb_node **new = &(root->rb_node), *parent = NULL;
+ struct stat_node *data;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ data->stat = stat;
/*
* Figure out where to put new node
@@ -118,12 +123,13 @@ insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
rb_link_node(&data->node, parent, new);
rb_insert_color(&data->node, root);
+ return 0;
}
/*
* For tracers that don't provide a stat_cmp callback.
- * This one will force an immediate insertion on tail of
- * the list.
+ * This one will force an insertion as right-most node
+ * in the rbtree.
*/
static int dummy_cmp(void *p1, void *p2)
{
@@ -131,15 +137,14 @@ static int dummy_cmp(void *p1, void *p2)
}
/*
- * Initialize the stat list at each trace_stat file opening.
+ * Initialize the stat rbtree at each trace_stat file opening.
* All of these copies and sorting are required on all opening
* since the stats could have changed between two file sessions.
*/
static int stat_seq_init(struct stat_session *session)
{
struct tracer_stat *ts = session->ts;
- struct stat_node *new_entry;
- struct rb_root *root;
+ struct rb_root *root = &session->stat_root;
void *stat;
int ret = 0;
int i;
@@ -154,23 +159,12 @@ static int stat_seq_init(struct stat_session *session)
if (!stat)
goto exit;
- /*
- * The first entry. Actually this is the second, but the first
- * one (the stat_list head) is pointless.
- */
- new_entry = kzalloc(sizeof(*new_entry), GFP_KERNEL);
- if (!new_entry) {
- ret = -ENOMEM;
+ ret = insert_stat(root, stat, ts->stat_cmp);
+ if (ret)
goto exit;
- }
- root = &session->stat_root;
- insert_stat(root, new_entry, dummy_cmp);
-
- new_entry->stat = stat;
/*
- * Iterate over the tracer stat entries and store them in a sorted
- * list.
+ * Iterate over the tracer stat entries and store them in an rbtree.
*/
for (i = 1; ; i++) {
stat = ts->stat_next(stat, i);
@@ -179,22 +173,16 @@ static int stat_seq_init(struct stat_session *session)
if (!stat)
break;
- new_entry = kzalloc(sizeof(*new_entry), GFP_KERNEL);
- if (!new_entry) {
- ret = -ENOMEM;
- goto exit_free_list;
- }
-
- new_entry->stat = stat;
-
- insert_stat(root, new_entry, ts->stat_cmp);
+ ret = insert_stat(root, stat, ts->stat_cmp);
+ if (ret)
+ goto exit_free_rbtree;
}
exit:
mutex_unlock(&session->stat_mutex);
return ret;
-exit_free_list:
+exit_free_rbtree:
reset_stat_session(session);
mutex_unlock(&session->stat_mutex);
return ret;
@@ -207,7 +195,7 @@ static void *stat_seq_start(struct seq_file *s, loff_t *pos)
struct rb_node *node;
int i;
- /* Prevent from tracer switch or stat_list modification */
+ /* Prevent from tracer switch or rbtree modification */
mutex_lock(&session->stat_mutex);
/* If we are in the beginning of the file, print the headers */
@@ -285,7 +273,7 @@ static int tracing_stat_open(struct inode *inode, struct file *file)
}
/*
- * Avoid consuming memory with our now useless list.
+ * Avoid consuming memory with our now useless rbtree.
*/
static int tracing_stat_release(struct inode *i, struct file *f)
{
--
1.5.4.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 0/3] tracing/stat: fixes, cleanups
@ 2009-05-28 16:42 Frederic Weisbecker
2009-05-28 16:42 ` [PATCH 3/3] tracing/stat: do some cleanups Frederic Weisbecker
0 siblings, 1 reply; 6+ messages in thread
From: Frederic Weisbecker @ 2009-05-28 16:42 UTC (permalink / raw)
To: Ingo Molnar; +Cc: LKML, Frederic Weisbecker, Li Zefan, Steven Rostedt, Zhaolei
Hi,
Here are the latest histogram tracing core updates.
Those are about output fixes, memory leak fixes and code clarity.
Thanks,
Frederic.
The following changes since commit 53059c9b67a62a3dc8c80204d3da42b9267ea5a0:
Frederic Weisbecker (1):
tracing/stat: replace linked list by an rbtree for sorting
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git tracing/stat
Li Zefan (3):
tracing/stat: change dummpy_cmp() to return -1
tracing/stat: remember to free root node
tracing/stat: do some cleanups
kernel/trace/trace_stat.c | 60 ++++++++++++++++++---------------------------
1 files changed, 24 insertions(+), 36 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/3] tracing/stat: do some cleanups
2009-05-28 16:42 [PATCH 0/3] tracing/stat: fixes, cleanups Frederic Weisbecker
@ 2009-05-28 16:42 ` Frederic Weisbecker
0 siblings, 0 replies; 6+ messages in thread
From: Frederic Weisbecker @ 2009-05-28 16:42 UTC (permalink / raw)
To: Ingo Molnar; +Cc: LKML, Li Zefan, Steven Rostedt, Zhaolei, Frederic Weisbecker
From: Li Zefan <lizf@cn.fujitsu.com>
- remove duplicate code in stat_seq_init()
- update comments to reflect the change from stat list to stat rbtree
[ Impact: clean up ]
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
kernel/trace/trace_stat.c | 54 +++++++++++++++++---------------------------
1 files changed, 21 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index a80d947..e8b50f3 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -93,10 +93,15 @@ static void destroy_session(struct stat_session *session)
typedef int (*cmp_stat_t)(void *, void *);
-static void
-insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
+static int insert_stat(struct rb_root *root, void *stat, cmp_stat_t cmp)
{
struct rb_node **new = &(root->rb_node), *parent = NULL;
+ struct stat_node *data;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ data->stat = stat;
/*
* Figure out where to put new node
@@ -118,12 +123,13 @@ insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
rb_link_node(&data->node, parent, new);
rb_insert_color(&data->node, root);
+ return 0;
}
/*
* For tracers that don't provide a stat_cmp callback.
- * This one will force an immediate insertion on tail of
- * the list.
+ * This one will force an insertion as right-most node
+ * in the rbtree.
*/
static int dummy_cmp(void *p1, void *p2)
{
@@ -131,15 +137,14 @@ static int dummy_cmp(void *p1, void *p2)
}
/*
- * Initialize the stat list at each trace_stat file opening.
+ * Initialize the stat rbtree at each trace_stat file opening.
* All of these copies and sorting are required on all opening
* since the stats could have changed between two file sessions.
*/
static int stat_seq_init(struct stat_session *session)
{
struct tracer_stat *ts = session->ts;
- struct stat_node *new_entry;
- struct rb_root *root;
+ struct rb_root *root = &session->stat_root;
void *stat;
int ret = 0;
int i;
@@ -154,23 +159,12 @@ static int stat_seq_init(struct stat_session *session)
if (!stat)
goto exit;
- /*
- * The first entry. Actually this is the second, but the first
- * one (the stat_list head) is pointless.
- */
- new_entry = kzalloc(sizeof(*new_entry), GFP_KERNEL);
- if (!new_entry) {
- ret = -ENOMEM;
+ ret = insert_stat(root, stat, ts->stat_cmp);
+ if (ret)
goto exit;
- }
- root = &session->stat_root;
- insert_stat(root, new_entry, dummy_cmp);
-
- new_entry->stat = stat;
/*
- * Iterate over the tracer stat entries and store them in a sorted
- * list.
+ * Iterate over the tracer stat entries and store them in an rbtree.
*/
for (i = 1; ; i++) {
stat = ts->stat_next(stat, i);
@@ -179,22 +173,16 @@ static int stat_seq_init(struct stat_session *session)
if (!stat)
break;
- new_entry = kzalloc(sizeof(*new_entry), GFP_KERNEL);
- if (!new_entry) {
- ret = -ENOMEM;
- goto exit_free_list;
- }
-
- new_entry->stat = stat;
-
- insert_stat(root, new_entry, ts->stat_cmp);
+ ret = insert_stat(root, stat, ts->stat_cmp);
+ if (ret)
+ goto exit_free_rbtree;
}
exit:
mutex_unlock(&session->stat_mutex);
return ret;
-exit_free_list:
+exit_free_rbtree:
reset_stat_session(session);
mutex_unlock(&session->stat_mutex);
return ret;
@@ -207,7 +195,7 @@ static void *stat_seq_start(struct seq_file *s, loff_t *pos)
struct rb_node *node;
int i;
- /* Prevent from tracer switch or stat_list modification */
+ /* Prevent from tracer switch or rbtree modification */
mutex_lock(&session->stat_mutex);
/* If we are in the beginning of the file, print the headers */
@@ -285,7 +273,7 @@ static int tracing_stat_open(struct inode *inode, struct file *file)
}
/*
- * Avoid consuming memory with our now useless list.
+ * Avoid consuming memory with our now useless rbtree.
*/
static int tracing_stat_release(struct inode *i, struct file *f)
{
--
1.6.2.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 1/3] tracing/stat: sort in ascending order
@ 2009-05-25 8:46 Li Zefan
2009-05-25 8:46 ` [PATCH 3/3] tracing/stat: do some cleanups Li Zefan
0 siblings, 1 reply; 6+ messages in thread
From: Li Zefan @ 2009-05-25 8:46 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Frederic Weisbecker, Steven Rostedt, LKML
Currently the output of trace_stat/workqueues is totally reversed:
# cat /debug/tracing/trace_stat/workqueues
...
1 17 17 210 37 `-blk_unplug_work+0x0/0x57
1 3779 3779 181 11 |-cfq_kick_queue+0x0/0x2f
1 3796 3796 kblockd/1:120
...
The correct output should be:
1 3796 3796 kblockd/1:120
1 3779 3779 181 11 |-cfq_kick_queue+0x0/0x2f
1 17 17 210 37 `-blk_unplug_work+0x0/0x57
It's caused by "tracing/stat: replace linked list by an rbtree for sorting"
(53059c9b67a62a3dc8c80204d3da42b9267ea5a0).
Though we can simply change dummy_cmp() to return -1 instead of 1, IMO
it's better to always do ascending sorting in trace_stat.c, and leave each
stat tracer to decide whether to sort in descending or ascending order.
[ Impact: fix the output of trace_stat/workqueue ]
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/trace/ftrace.c | 12 ++++++------
kernel/trace/trace_branch.c | 5 +++--
kernel/trace/trace_stat.c | 6 +-----
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 140699a..3dd16bd 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -315,29 +315,29 @@ static void *function_stat_start(struct tracer_stat *trace)
}
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-/* function graph compares on total time */
+/* function graph compares on total time in reverse order */
static int function_stat_cmp(void *p1, void *p2)
{
struct ftrace_profile *a = p1;
struct ftrace_profile *b = p2;
- if (a->time < b->time)
- return -1;
if (a->time > b->time)
+ return -1;
+ if (a->time < b->time)
return 1;
else
return 0;
}
#else
-/* not function graph compares against hits */
+/* not function graph compares against hits in reverse order */
static int function_stat_cmp(void *p1, void *p2)
{
struct ftrace_profile *a = p1;
struct ftrace_profile *b = p2;
- if (a->counter < b->counter)
- return -1;
if (a->counter > b->counter)
+ return -1;
+ if (a->counter < b->counter)
return 1;
else
return 0;
diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
index 7a7a9fd..df58411 100644
--- a/kernel/trace/trace_branch.c
+++ b/kernel/trace/trace_branch.c
@@ -301,9 +301,10 @@ static int annotated_branch_stat_cmp(void *p1, void *p2)
percent_a = get_incorrect_percent(a);
percent_b = get_incorrect_percent(b);
- if (percent_a < percent_b)
- return -1;
+ /* sort in descending order */
if (percent_a > percent_b)
+ return -1;
+ if (percent_a < percent_b)
return 1;
else
return 0;
diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index 2e849b5..6efbcb4 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -98,10 +98,6 @@ insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
{
struct rb_node **new = &(root->rb_node), *parent = NULL;
- /*
- * Figure out where to put new node
- * This is a descendent sorting
- */
while (*new) {
struct stat_node *this;
int result;
@@ -110,7 +106,7 @@ insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
result = cmp(data->stat, this->stat);
parent = *new;
- if (result >= 0)
+ if (result < 0)
new = &((*new)->rb_left);
else
new = &((*new)->rb_right);
--
1.5.4.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] tracing/stat: do some cleanups
2009-05-25 8:46 [PATCH 1/3] tracing/stat: sort in ascending order Li Zefan
@ 2009-05-25 8:46 ` Li Zefan
0 siblings, 0 replies; 6+ messages in thread
From: Li Zefan @ 2009-05-25 8:46 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Frederic Weisbecker, Steven Rostedt, LKML
- remove duplicate code in stat_seq_init()
- update comments to reflect the change from stat list to stat rbtree
[ Impact: clean up ]
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
kernel/trace/trace_stat.c | 56 +++++++++++++++++---------------------------
1 files changed, 22 insertions(+), 34 deletions(-)
diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c
index ed18701..abf2ba1 100644
--- a/kernel/trace/trace_stat.c
+++ b/kernel/trace/trace_stat.c
@@ -8,7 +8,6 @@
*
*/
-
#include <linux/list.h>
#include <linux/rbtree.h>
#include <linux/debugfs.h>
@@ -64,10 +63,15 @@ static void destroy_session(struct stat_session *session)
typedef int (*cmp_stat_t)(void *, void *);
-static void
-insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
+static int insert_stat(struct rb_root *root, void *stat, cmp_stat_t cmp)
{
struct rb_node **new = &(root->rb_node), *parent = NULL;
+ struct stat_node *data;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ data->stat = stat;
while (*new) {
struct stat_node *this;
@@ -85,12 +89,13 @@ insert_stat(struct rb_root *root, struct stat_node *data, cmp_stat_t cmp)
rb_link_node(&data->node, parent, new);
rb_insert_color(&data->node, root);
+ return 0;
}
/*
* For tracers that don't provide a stat_cmp callback.
- * This one will force an immediate insertion on tail of
- * the list.
+ * This one will force an insertion on rightmost node
+ * of the tree.
*/
static int dummy_cmp(void *p1, void *p2)
{
@@ -98,15 +103,14 @@ static int dummy_cmp(void *p1, void *p2)
}
/*
- * Initialize the stat list at each trace_stat file opening.
+ * Initialize the stat rbtree at each trace_stat file opening.
* All of these copies and sorting are required on all opening
* since the stats could have changed between two file sessions.
*/
static int stat_seq_init(struct stat_session *session)
{
struct tracer_stat *ts = session->ts;
- struct stat_node *new_entry;
- struct rb_root *root;
+ struct rb_root *root = &session->stat_root;
void *stat;
int ret = 0;
int i;
@@ -121,23 +125,13 @@ static int stat_seq_init(struct stat_session *session)
if (!stat)
goto exit;
- /*
- * The first entry. Actually this is the second, but the first
- * one (the stat_list head) is pointless.
- */
- new_entry = kzalloc(sizeof(*new_entry), GFP_KERNEL);
- if (!new_entry) {
- ret = -ENOMEM;
+ ret = insert_stat(root, stat, ts->stat_cmp);
+ if (ret)
goto exit;
- }
- root = &session->stat_root;
- insert_stat(root, new_entry, dummy_cmp);
-
- new_entry->stat = stat;
/*
- * Iterate over the tracer stat entries and store them in a sorted
- * list.
+ * Iterate over the tracer stat entries and store them
+ * in an rbtree.
*/
for (i = 1; ; i++) {
stat = ts->stat_next(stat, i);
@@ -146,22 +140,16 @@ static int stat_seq_init(struct stat_session *session)
if (!stat)
break;
- new_entry = kzalloc(sizeof(*new_entry), GFP_KERNEL);
- if (!new_entry) {
- ret = -ENOMEM;
- goto exit_free_list;
- }
-
- new_entry->stat = stat;
-
- insert_stat(root, new_entry, ts->stat_cmp);
+ ret = insert_stat(root, stat, ts->stat_cmp);
+ if (ret)
+ goto exit_free_rbtree;
}
exit:
mutex_unlock(&session->stat_mutex);
return ret;
-exit_free_list:
+exit_free_rbtree:
reset_stat_session(session);
mutex_unlock(&session->stat_mutex);
return ret;
@@ -174,7 +162,7 @@ static void *stat_seq_start(struct seq_file *s, loff_t *pos)
struct rb_node *node;
int i;
- /* Prevent from tracer switch or stat_list modification */
+ /* Prevent from tracer switch or rbtree modification */
mutex_lock(&session->stat_mutex);
/* If we are in the beginning of the file, print the headers */
@@ -252,7 +240,7 @@ static int tracing_stat_open(struct inode *inode, struct file *file)
}
/*
- * Avoid consuming memory with our now useless list.
+ * Avoid consuming memory with our now useless rbtree.
*/
static int tracing_stat_release(struct inode *i, struct file *f)
{
--
1.5.4.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-05-28 16:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-27 3:04 [PATCH 1/3] tracing/stat: change dummpy_cmp() to return -1 Li Zefan
2009-05-27 3:04 ` [PATCH 2/3] tracing/stat: remember to free root node Li Zefan
2009-05-27 3:05 ` [PATCH 3/3] tracing/stat: do some cleanups Li Zefan
2009-05-27 3:42 ` [PATCH 3/3 -v2] " Li Zefan
-- strict thread matches above, loose matches on Subject: below --
2009-05-28 16:42 [PATCH 0/3] tracing/stat: fixes, cleanups Frederic Weisbecker
2009-05-28 16:42 ` [PATCH 3/3] tracing/stat: do some cleanups Frederic Weisbecker
2009-05-25 8:46 [PATCH 1/3] tracing/stat: sort in ascending order Li Zefan
2009-05-25 8:46 ` [PATCH 3/3] tracing/stat: do some cleanups Li Zefan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox