Re: [PATCH 12/17] tracing/probes: Implement 'memory' fetch method for uprobes

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

(2013/12/09 15:20), Namhyung Kim wrote:

> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index b4f28bc39959..d0b4a42dafcf 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -103,83 +103,6 @@ DEFINE_BASIC_FETCH_FUNCS(retval)
>  #define fetch_retval_string		NULL
>  #define fetch_retval_string_size	NULL
>  
> -#define DEFINE_FETCH_memory(type)					\
> -__kprobes void FETCH_FUNC_NAME(memory, type)(struct pt_regs *regs,	\
> -					  void *addr, void *dest)	\
> -{									\
> -	type retval;							\
> -	if (probe_kernel_address(addr, retval))				\
> -		*(type *)dest = 0;					\
> -	else								\
> -		*(type *)dest = retval;					\
> -}
> -DEFINE_BASIC_FETCH_FUNCS(memory)
> -/*
> - * Fetch a null-terminated string. Caller MUST set *(u32 *)dest with max
> - * length and relative data location.
> - */
> -__kprobes void FETCH_FUNC_NAME(memory, string)(struct pt_regs *regs,
> -						      void *addr, void *dest)
> -{
> -	long ret;
> -	int maxlen = get_rloc_len(*(u32 *)dest);
> -	u8 *dst = get_rloc_data(dest);
> -	u8 *src = addr;
> -	mm_segment_t old_fs = get_fs();
> -
> -	if (!maxlen)
> -		return;
> -
> -	/*
> -	 * Try to get string again, since the string can be changed while
> -	 * probing.
> -	 */
> -	set_fs(KERNEL_DS);
> -	pagefault_disable();
> -
> -	do
> -		ret = __copy_from_user_inatomic(dst++, src++, 1);
> -	while (dst[-1] && ret == 0 && src - (u8 *)addr < maxlen);
> -
> -	dst[-1] = '\0';
> -	pagefault_enable();
> -	set_fs(old_fs);
> -
> -	if (ret < 0) {	/* Failed to fetch string */
> -		((u8 *)get_rloc_data(dest))[0] = '\0';
> -		*(u32 *)dest = make_data_rloc(0, get_rloc_offs(*(u32 *)dest));
> -	} else {
> -		*(u32 *)dest = make_data_rloc(src - (u8 *)addr,
> -					      get_rloc_offs(*(u32 *)dest));
> -	}
> -}
> -
> -/* Return the length of string -- including null terminal byte */
> -__kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
> -							void *addr, void *dest)
> -{
> -	mm_segment_t old_fs;
> -	int ret, len = 0;
> -	u8 c;
> -
> -	old_fs = get_fs();
> -	set_fs(KERNEL_DS);
> -	pagefault_disable();
> -
> -	do {
> -		ret = __copy_from_user_inatomic(&c, (u8 *)addr + len, 1);
> -		len++;
> -	} while (c && ret == 0 && len < MAX_STRING_SIZE);
> -
> -	pagefault_enable();
> -	set_fs(old_fs);
> -
> -	if (ret < 0)	/* Failed to check the length */
> -		*(u32 *)dest = 0;
> -	else
> -		*(u32 *)dest = len;
> -}
> -
>  /* Dereference memory access function */
>  struct deref_fetch_param {
>  	struct fetch_param	orig;
> @@ -493,7 +416,7 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
>  			dprm->offset = offset;
>  			dprm->fetch = t->fetch[FETCH_MTD_memory];
>  			dprm->fetch_size = get_fetch_size_function(t,
> -							dprm->fetch, ttbl);
> +							dprm->fetch, ftbl);

What is this change? I guess it should be included in 11/17...
The rest of this patch is good for me.

Thank you,


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