Re: [PATCH 3/4] gcov: add gcov profiling infrastructure

[Peter Oberparleiter <oberpar@linux.vnet.ibm.com>]

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;