Re: [PATCH 06/13] tracing/kprobes: Move common functions to trace_probe.c

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

(2013/07/31 18:03), Namhyung Kim wrote:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> The __get_data_size() and store_trace_args() will be used by uprobes
> too.  Move them to a common location.

Hmm, could you move this into trace_probe.h? Since this will
be used in fast path, I'd like to keep it as inlined.

> 
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: zhangwei(Jovi) <jovi.zhangwei@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  kernel/trace/trace_kprobe.c | 48 ---------------------------------------------
>  kernel/trace/trace_probe.c  | 46 +++++++++++++++++++++++++++++++++++++++++++
>  kernel/trace/trace_probe.h  |  4 ++++
>  3 files changed, 50 insertions(+), 48 deletions(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 21d85a40740c..d4c10fb8b8f8 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -845,54 +845,6 @@ const struct fetch_type kprobes_fetch_type_table[] = {
>  	ASSIGN_FETCH_TYPE(s64, u64, 1),
>  };
>  
> -/* Sum up total data length for dynamic arraies (strings) */
> -static __kprobes int __get_data_size(struct trace_probe *tp,
> -				     struct pt_regs *regs)
> -{
> -	int i, ret = 0;
> -	u32 len;
> -
> -	for (i = 0; i < tp->nr_args; i++)
> -		if (unlikely(tp->args[i].fetch_size.fn)) {
> -			call_fetch(&tp->args[i].fetch_size, regs, &len);
> -			ret += len;
> -		}
> -
> -	return ret;
> -}
> -
> -/* Store the value of each argument */
> -static __kprobes void store_trace_args(int ent_size, struct trace_probe *tp,
> -				       struct pt_regs *regs,
> -				       u8 *data, int maxlen)
> -{
> -	int i;
> -	u32 end = tp->size;
> -	u32 *dl;	/* Data (relative) location */
> -
> -	for (i = 0; i < tp->nr_args; i++) {
> -		if (unlikely(tp->args[i].fetch_size.fn)) {
> -			/*
> -			 * First, we set the relative location and
> -			 * maximum data length to *dl
> -			 */
> -			dl = (u32 *)(data + tp->args[i].offset);
> -			*dl = make_data_rloc(maxlen, end - tp->args[i].offset);
> -			/* Then try to fetch string or dynamic array data */
> -			call_fetch(&tp->args[i].fetch, regs, dl);
> -			/* Reduce maximum length */
> -			end += get_rloc_len(*dl);
> -			maxlen -= get_rloc_len(*dl);
> -			/* Trick here, convert data_rloc to data_loc */
> -			*dl = convert_rloc_to_loc(*dl,
> -				 ent_size + tp->args[i].offset);
> -		} else
> -			/* Just fetching data normally */
> -			call_fetch(&tp->args[i].fetch, regs,
> -				   data + tp->args[i].offset);
> -	}
> -}
> -
>  /* Kprobe handler */
>  static __kprobes void
>  __kprobe_trace_func(struct trace_kprobe *tp, struct pt_regs *regs,
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index c123a890c5b1..5e491d82b6ad 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -701,3 +701,49 @@ out:
>  
>  	return ret;
>  }
> +
> +/* Sum up total data length for dynamic arraies (strings) */
> +__kprobes int __get_data_size(struct trace_probe *tp, struct pt_regs *regs)
> +{
> +	int i, ret = 0;
> +	u32 len;
> +
> +	for (i = 0; i < tp->nr_args; i++)
> +		if (unlikely(tp->args[i].fetch_size.fn)) {
> +			call_fetch(&tp->args[i].fetch_size, regs, &len);
> +			ret += len;
> +		}
> +
> +	return ret;
> +}
> +
> +/* Store the value of each argument */
> +__kprobes void store_trace_args(int ent_size, struct trace_probe *tp,
> +				struct pt_regs *regs, u8 *data, int maxlen)
> +{
> +	int i;
> +	u32 end = tp->size;
> +	u32 *dl;	/* Data (relative) location */
> +
> +	for (i = 0; i < tp->nr_args; i++) {
> +		if (unlikely(tp->args[i].fetch_size.fn)) {
> +			/*
> +			 * First, we set the relative location and
> +			 * maximum data length to *dl
> +			 */
> +			dl = (u32 *)(data + tp->args[i].offset);
> +			*dl = make_data_rloc(maxlen, end - tp->args[i].offset);
> +			/* Then try to fetch string or dynamic array data */
> +			call_fetch(&tp->args[i].fetch, regs, dl);
> +			/* Reduce maximum length */
> +			end += get_rloc_len(*dl);
> +			maxlen -= get_rloc_len(*dl);
> +			/* Trick here, convert data_rloc to data_loc */
> +			*dl = convert_rloc_to_loc(*dl,
> +				 ent_size + tp->args[i].offset);
> +		} else
> +			/* Just fetching data normally */
> +			call_fetch(&tp->args[i].fetch, regs,
> +				   data + tp->args[i].offset);
> +	}
> +}
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index a939b7114e78..888cd7d4cd3c 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -297,3 +297,7 @@ extern ssize_t traceprobe_probes_write(struct file *file,
>  		int (*createfn)(int, char**));
>  
>  extern int traceprobe_command(const char *buf, int (*createfn)(int, char**));
> +
> +extern int __get_data_size(struct trace_probe *tp, struct pt_regs *regs);
> +extern void store_trace_args(int ent_size, struct trace_probe *tp,
> +			     struct pt_regs *regs, u8 *data, int maxlen);
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com