Re: [PATCH] tracing - support multiple pids in set_pid_ftrace file

[Li Zefan <lizf@cn.fujitsu.com>]

jolsa@redhat.com wrote:
> Adding the possibility to set more than 1 pid in the set_pid_ftrace file,
> thus allowing to trace more than 1 independent processes.
> 
> Usage:
> 
> sh-4.0# echo 284 > ./set_ftrace_pid 
> sh-4.0# cat ./set_ftrace_pid 
> 284
> sh-4.0# echo 1 >> ./set_ftrace_pid 
> sh-4.0# echo 0 >> ./set_ftrace_pid 
> sh-4.0# cat ./set_ftrace_pid 
> swapper tasks
> 1
> 284
> sh-4.0# echo 4 > ./set_ftrace_pid 
> sh-4.0# cat ./set_ftrace_pid 
> 4
> sh-4.0# echo > ./set_ftrace_pid 
> sh-4.0# cat ./set_ftrace_pid 
> no pid
> sh-4.0# 
> 
> 
> wbr,
> jirka
> 
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
>  kernel/trace/ftrace.c |  226 +++++++++++++++++++++++++++++++++++--------------
>  kernel/trace/trace.h  |    4 +-
>  2 files changed, 164 insertions(+), 66 deletions(-)
>  
...
> +static void ftrace_pid_reset(void)
> +{
> +	struct ftrace_pid *fpid, *safe;
> +
> +	mutex_lock(&ftrace_lock);
> +	list_for_each_entry_safe(fpid, safe, &ftrace_pids, list) {
> +		struct pid *pid = fpid->pid;
> +
> +		clear_ftrace_pid_task(&pid);

Now we can change clear_ftrace_pid_task() to take struct pid *
instead of struct pid**.

> +
> +		if (pid != ftrace_swapper_pid)
> +			put_pid(pid);
> +

clear_ftrace_pid_task() already called put_pid().

> +		list_del(&fpid->list);
> +		kfree(fpid);
> +	}
> +
> +	ftrace_update_pid_func();
> +	ftrace_startup_enable(0);
> +
> +	mutex_unlock(&ftrace_lock);
> +}
> +
...
>  static ssize_t
>  ftrace_pid_write(struct file *filp, const char __user *ubuf,
>  		   size_t cnt, loff_t *ppos)
>  {
> -	struct pid *pid;
>  	char buf[64];
>  	long val;
>  	int ret;
> @@ -2897,57 +3016,36 @@ ftrace_pid_write(struct file *filp, const char __user *ubuf,
>  
>  	buf[cnt] = 0;
>  
> +	/*
> +	 * Allow the "echo > set_ftrace_pid" to clean
> +	 * the pids quietly.
> +	 */
> +	if (!*ppos && *buf == '\n' && cnt == 1)
> +		return 1;
> +

How about:
	strstrip(buf);
	if (strlen(buf) == 0)
		return 1;

So "echo -n '' > set_ftrace_pid" works too.

>  	ret = strict_strtol(buf, 10, &val);
>  	if (ret < 0)
>  		return ret;
>