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

Li Wei
2009-02-26T10:12:10Z

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?

---
kernel/gcov/fs.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/kernel/gcov/fs.c b/kernel/gcov/fs.c
index 6675c82..aab3be7 100644
--- a/kernel/gcov/fs.c
+++ b/kernel/gcov/fs.c
@@ -29,7 +29,7 @@
* struct gcov_node - represents a debugfs entry
* @list: list head for child node list
* @children: child nodes
- * @all: list head for list of all nodes
+ * @leaves: list head for list of all leaf nodes
* @parent: parent node
* @info: associated profiling data structure if not a directory
* @ghost: when an object file containing profiling data is unloaded we keep a
@@ -47,7 +47,7 @@
struct gcov_node {
struct list_head list;
struct list_head children;
- struct list_head all;
+ struct list_head leaves;
struct gcov_node *parent;
struct gcov_info *info;
struct gcov_info *ghost;
@@ -60,7 +60,7 @@ static const char objtree[] = OBJTREE;
static const char srctree[] = SRCTREE;
static struct gcov_node root_node;
static struct dentry *reset_dentry;
-static LIST_HEAD(all_head);
+static LIST_HEAD(leaves_head);
static DEFINE_MUTEX(node_lock);

/* If non-zero, keep copies of profiling data for unloaded modules. */
@@ -206,7 +206,7 @@ static struct gcov_node *get_node_by_name(const char *name)
struct gcov_node *node;
struct gcov_info *info;

- list_for_each_entry(node, &all_head, all) {
+ list_for_each_entry(node, &leaves_head, leaves) {
info = get_node_info(node);
if (info && (strcmp(info->filename, name) == 0))
return node;
@@ -355,7 +355,7 @@ static void init_node(struct gcov_node *node, struct gcov_info *info,
{
INIT_LIST_HEAD(&node->list);
INIT_LIST_HEAD(&node->children);
- INIT_LIST_HEAD(&node->all);
+ INIT_LIST_HEAD(&node->leaves);
node->info = info;
if (name)
strcpy(node->name, name);
@@ -389,7 +389,8 @@ static struct gcov_node *new_node(struct gcov_node *parent,
if (info)
add_links(node, parent->dentry);
list_add(&node->list, &parent->children);
- list_add(&node->all, &all_head);
+ list_add(&node->leaves, &leaves_head)From e3ffddfa9af66bf6f9df400b63ae88f2ac90562c Mon Sep 17 00:00:00 2001
From: Li Wei <w.li@sun.com>
Date: Thu, 26 Feb 2009 17:53:37 +0800
Subject: [PATCH] gcov: fix panic in reset_write

remove_node may delete the next node in all_head. Replace the all_head
with a list of all leaf nodes, and process leaves_head in reset_write.
;
+ list_del_init(&parent->leaves);

return node;
}
@@ -412,7 +413,7 @@ static void remove_links(struct gcov_node *node)
static void release_node(struct gcov_node *node)
{
list_del(&node->list);
- list_del(&node->all);
+ list_del(&node->leaves);
debugfs_remove(node->dentry);
remove_links(node);
if (node->ghost)
@@ -456,7 +457,7 @@ static ssize_t reset_write(struct file *file, const char __user *addr,
struct gcov_node *r;

mutex_lock(&node_lock);
- list_for_each_entry_safe(node, r, &all_head, all) {
+ list_for_each_entry_safe(node, r, &leaves_head, leaves) {
if (node->info)
gcov_info_reset(node->info);
else
@@ -480,7 +481,7 @@ static const struct file_operations gcov_reset_fops = {
.read = reset_read,
};

-/* Create a node for a given profiling data set and add it to all lists and
+/* Create a node for a given profiling data set and add it to leaves lists and
* debugfs. Needs to be called with node_lock held. */
static void add_node(struct gcov_info *info)
{
--
1.5.6.3