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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49A6812F.4030303@linux.vnet.ibm.com>
Date:	Thu, 26 Feb 2009 12:46:55 +0100
From:	Peter Oberparleiter <oberpar@...ux.vnet.ibm.com>
To:	Li Wei <W.Li@....COM>
CC:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andi Kleen <andi@...stfloor.org>,
	Huang Ying <ying.huang@...el.com>,
	Sam Ravnborg <sam@...nborg.org>
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;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/