Re: [Patch 2/3] ksym_tracer: Allow bulk removal using empty or wildcard string input - ver II

[Frederic Weisbecker <fweisbec@gmail.com>]

On Sat, Jun 20, 2009 at 07:05:07PM +0530, K.Prasad wrote:
> Accept an empty string or wildcard input (of the form *:---) as input for
> ksym_trace_filter to perform bulk removal of all monitored entries.
> 
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> ---
>  kernel/trace/trace_ksym.c |   77 +++++++++++++++++++++++++---------------------
>  1 file changed, 43 insertions(+), 34 deletions(-)
> 
> Index: linux-2.6-tip.hbkpt/kernel/trace/trace_ksym.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/kernel/trace/trace_ksym.c
> +++ linux-2.6-tip.hbkpt/kernel/trace/trace_ksym.c
> @@ -23,6 +23,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/ftrace.h>
>  #include <linux/module.h>
> +#include <linux/ctype.h>
>  #include <linux/fs.h>
>  
>  #include "trace_output.h"
> @@ -138,6 +139,34 @@ static int ksym_trace_get_access_type(ch
>  	return access;
>  }
>  
> +static void ksym_trace_reset(struct trace_array *tr)
> +{
> +	struct trace_ksym *entry;
> +	struct hlist_node *node, *node1;
> +
> +	ksym_tracing_enabled = 0;
> +
> +	mutex_lock(&ksym_tracer_mutex);
> +	hlist_for_each_entry_safe(entry, node, node1, &ksym_filter_head,
> +								ksym_hlist) {
> +		unregister_kernel_hw_breakpoint(entry->ksym_hbp);
> +		ksym_filter_entry_count--;
> +		hlist_del_rcu(&(entry->ksym_hlist));
> +		synchronize_rcu();
> +		/* Free the 'input_string' only if reset
> +		 * after startup self-test
> +		 */
> +#ifdef CONFIG_FTRACE_SELFTEST
> +		if (strncmp(entry->ksym_hbp->info.name, KSYM_SELFTEST_ENTRY,
> +					strlen(KSYM_SELFTEST_ENTRY)) != 0)
> +#endif /* CONFIG_FTRACE_SELFTEST*/
> +			kfree(entry->ksym_hbp->info.name);
> +		kfree(entry->ksym_hbp);
> +		kfree(entry);
> +	}
> +	mutex_unlock(&ksym_tracer_mutex);
> +}
> +
>  /*
>   * There can be several possible malformed requests and we attempt to capture
>   * all of them. We enumerate some of the rules
> @@ -163,7 +192,7 @@ static int parse_ksym_trace_str(char *in
>  	/* Check for malformed request: (2), (1) and (5) */
>  	if ((!input_string) ||
>  		(strlen(input_string) != (KSYM_TRACER_OP_LEN + 1)) ||
> -			(*addr == 0))
> +			((*addr == 0) && strncmp(*ksymname, "*", strlen("*"))))
>  		goto return_code;
>  	ret = ksym_trace_get_access_type(input_string);
>  
> @@ -211,6 +240,7 @@ int process_new_ksym_entry(char *ksymnam
>  	}
>  	hlist_add_head_rcu(&(entry->ksym_hlist), &ksym_filter_head);
>  	ksym_filter_entry_count++;
> +	ksym_tracing_enabled = 1;
>  
>  	return 0;
>  }
> @@ -220,7 +250,7 @@ static ssize_t ksym_trace_filter_read(st
>  {
>  	struct trace_ksym *entry;
>  	struct hlist_node *node;
> -	char buf[KSYM_FILTER_ENTRY_LEN * KSYM_TRACER_MAX];
> +	char buf[KSYM_FILTER_ENTRY_LEN * KSYM_TRACER_MAX] = "\0";
>  	ssize_t ret, cnt = 0;
>  
>  	mutex_lock(&ksym_tracer_mutex);
> @@ -251,10 +281,6 @@ static ssize_t ksym_trace_filter_write(s
>  	unsigned long ksym_addr = 0;
>  	int ret, op, changed = 0;
>  
> -	/* Ignore echo "" > ksym_trace_filter */
> -	if (count == 0)
> -		return 0;
> -
>  	input_string = kzalloc(count, GFP_KERNEL);
>  	if (!input_string)
>  		return -ENOMEM;
> @@ -263,12 +289,23 @@ static ssize_t ksym_trace_filter_write(s
>  		kfree(input_string);
>  		return -EFAULT;
>  	}
> +	/* Clear all breakpoint requests if echo "" > ksym_trace_filter */
> +	if ((count == 1) && !isalnum(input_string[0])) {
> +		ksym_trace_reset(NULL);
> +		return count;
> +	}
>  
>  	ret = op = parse_ksym_trace_str(input_string, &ksymname, &ksym_addr);
>  	if (ret < 0) {
>  		kfree(input_string);
>  		return ret;
>  	}
> +	/* Clear all breakpoints if echo "*:---" > ksym_trace_filter */
> +	if ((strncmp(ksymname, "*", strlen("*")) == 0) && (op == 0)) {


If you returned -EINVAL above, 


if (*ksymname == '*' && !op)

..would be shorter

Also that doesn't check if symname = "*something:---"



> +		ksym_trace_reset(NULL);
> +		kfree(input_string);
> +		return count;
> +	}
>  
>  	mutex_lock(&ksym_tracer_mutex);
>  
> @@ -325,34 +362,6 @@ static const struct file_operations ksym
>  	.write		= ksym_trace_filter_write,
>  };
>  
> -static void ksym_trace_reset(struct trace_array *tr)
> -{
> -	struct trace_ksym *entry;
> -	struct hlist_node *node, *node1;
> -
> -	ksym_tracing_enabled = 0;
> -
> -	mutex_lock(&ksym_tracer_mutex);
> -	hlist_for_each_entry_safe(entry, node, node1, &ksym_filter_head,
> -								ksym_hlist) {
> -		unregister_kernel_hw_breakpoint(entry->ksym_hbp);
> -		ksym_filter_entry_count--;
> -		hlist_del_rcu(&(entry->ksym_hlist));
> -		synchronize_rcu();
> -		/* Free the 'input_string' only if reset
> -		 * after startup self-test
> -		 */
> -#ifdef CONFIG_FTRACE_SELFTEST
> -		if (strncmp(entry->ksym_hbp->info.name, KSYM_SELFTEST_ENTRY,
> -					strlen(KSYM_SELFTEST_ENTRY)) != 0)
> -#endif /* CONFIG_FTRACE_SELFTEST*/
> -			kfree(entry->ksym_hbp->info.name);
> -		kfree(entry->ksym_hbp);
> -		kfree(entry);
> -	}
> -	mutex_unlock(&ksym_tracer_mutex);
> -}
> -
>  static int ksym_trace_init(struct trace_array *tr)
>  {
>  	int cpu, ret = 0;