Messages in this thread Patch in this message | | | Date | Thu, 26 Feb 2009 12:46:55 +0100 | From | Peter Oberparleiter <> | Subject | Re: [PATCH 3/4] gcov: add gcov profiling infrastructure |
| |
Li Wei wrote: > On Tue, 2009-02-03 at 13:47 +0100, Peter Oberparleiter wrote: >> +/* write() implementation for reset file. Reset all profiling data to zero >> + * and remove ghost nodes. */ >> +static ssize_t reset_write(struct file *file, const char __user *addr, >> + size_t len, loff_t *pos) >> +{ >> + struct gcov_node *node; >> + struct gcov_node *r; >> + >> + mutex_lock(&node_lock); >> + list_for_each_entry_safe(node, r, &all_head, all) { >> + if (node->info) >> + gcov_info_reset(node->info); >> + else >> + remove_node(node); >> + } >> + mutex_unlock(&node_lock); >> + >> + return len; >> +} > > The remove_node above may have deleted the node that r points at. How > about replacing all_head with a leaves_head that contains all leaf > nodes, and iterate the latter instead?
You're right again - with the fix for node->parent I'm running into kernel oopses because of this problem. But instead of creating and maintaining a new new leaf-nodes list, I'd prefer simply restarting the loop when remove_node has been called. This results in a bit more computing effort but doesn't increase data complexity any more than necessary. My proposed fix looks something like this (unless there are objections, I'll include this one in the re-post):
--- kernel/gcov/fs.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
Index: linux-2.6.29-rc6/kernel/gcov/fs.c =================================================================== --- linux-2.6.29-rc6.orig/kernel/gcov/fs.c +++ linux-2.6.29-rc6/kernel/gcov/fs.c @@ -453,15 +453,22 @@ static ssize_t reset_write(struct file * size_t len, loff_t *pos) { struct gcov_node *node; - struct gcov_node *r; + int restart; mutex_lock(&node_lock); - list_for_each_entry_safe(node, r, &all_head, all) { - if (node->info) - gcov_info_reset(node->info); - else - remove_node(node); - } + do { + restart = 0; + list_for_each_entry(node, &all_head, all) { + if (node->info) + gcov_info_reset(node->info); + else if (list_empty(&node->children)) { + remove_node(node); + /* Several nodes may have gone - restart. */ + restart = 1; + break; + } + } + } while (restart); mutex_unlock(&node_lock); return len;
| |