Re: [PATCH 1/2] tracing: Add $cpu and $current probe-vars

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

(2013/11/26 4:29), Oleg Nesterov wrote:
> The probe can dump the registers or memory, but it is not possible
> to dump, say, current->pid. This patch adds the pseudo regs table,
> currently it has only two methods to get current/smp_processor_id
> but it can be trivially extended.
> 
> The syntax is '$cpu' and '$current', we overload FETCH_MTD_reg just
> to avoid the new fetch method(s).
> 
> Test-case: 452 == offsetof(task_struct, pid), 39 == __NR_getpid,
> 
>   # perf probe 'sys_getpid%return ret=$retval:s32 pid=+452($current):s32'
>   # perf record -e probe:sys_getpid perl -e 'syscall 39'
>   # perf --no-pager script | tail -1
> 	perl   586 [001]   753.102549: probe:sys_getpid: \
> 	(ffffffff81052c00 <- ffffffff8134d012) ret=586 pid=586
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>  kernel/trace/trace_probe.c |   61 +++++++++++++++++++++++++++++++++++++++++---
>  1 files changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 412e959..b066e9d 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -109,13 +109,14 @@ DEFINE_FETCH_##method(u64)
>  	  (FETCH_FUNC_NAME(method, string_size) == fn)) \
>  	 && (fn != NULL))
>  
> +static unsigned long probe_get_register(struct pt_regs *, unsigned long);
> +
>  /* Data fetch function templates */
>  #define DEFINE_FETCH_reg(type)						\
>  static __kprobes void FETCH_FUNC_NAME(reg, type)(struct pt_regs *regs,	\
>  					void *offset, void *dest)	\
>  {									\
> -	*(type *)dest = (type)regs_get_register(regs,			\
> -				(unsigned int)((unsigned long)offset));	\
> +	*(type *)dest = (type)probe_get_register(regs, (long)offset);	\
>  }
>  DEFINE_BASIC_FETCH_FUNCS(reg)
>  /* No string on the register */
> @@ -513,6 +514,52 @@ int traceprobe_split_symbol_offset(char *symbol, unsigned long *offset)
>  	return 0;
>  }
>  
> +static unsigned long pseudo_reg_cpu(void)
> +{
> +	return (unsigned long)raw_smp_processor_id();
> +}
> +
> +static unsigned long pseudo_reg_current(void)
> +{
> +	return (unsigned long)current;
> +}
> +
> +static struct {
> +	const char *name;
> +	unsigned long (*fetch)(void);
> +}
> +const pseudo_reg_table[] = {
> +	{
> +		.name	= "cpu",
> +		.fetch	= pseudo_reg_cpu,
> +	},
> +	{
> +		.name	= "current",
> +		.fetch	= pseudo_reg_current,
> +	},
> +};
> +
> +#define PSEUDO_REG_OFFSET	4096	/* arbitrary value > MAX_REG_OFFSET */
> +
> +static unsigned long probe_get_register(struct pt_regs *regs, unsigned long offset)
> +{
> +	if (offset < PSEUDO_REG_OFFSET)
> +		return regs_get_register(regs, offset);
> +
> +	return pseudo_reg_table[offset - PSEUDO_REG_OFFSET].fetch();
> +}


Hmm, I don't like this, since fetch_reg is the most frequently
used fetch method, and it actually uses the offset in different
way.

Why don't you make another fetch function for vars?
Perhaps, you can make fetch_retval more generic.

Thank you,

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