Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-12-03 Thread Masami Hiramatsu 
(2013/12/03 15:23), Namhyung Kim wrote:
>>> So do you want me to change to make it simpler without a fetch_param?
>>
>> Yes, yes. Assuming that you agree, of course.
> 
> Okay, here goes v8 then. :)
> 
> I also push it (with other changes come from Srikar) to the
> uprobe/fetch-v8 branch in my tree for your convenience. :)

This looks good for me :)

>>From 25bc2bab14e9461f8b15e34f26d1de51f96e44e1 Mon Sep 17 00:00:00 2001
> From: Namhyung Kim 
> Date: Mon, 25 Nov 2013 13:42:47 +0900
> Subject: [PATCH v8 17/17] tracing/uprobes: Add @+file_offset fetch method
> 
> Enable to fetch data from a file offset.  Currently it only supports
> fetching from same binary uprobe set.  It'll translate the file offset
> to a proper virtual address in the process.
> 
> The syntax is "@+OFFSET" as it does similar to normal memory fetching
> (@ADDR) which does no address translation.
> 
> To do it, change fifth argument to receive 'void *priv' and make it
> set only for uprobes so that we can determine the arg is for kprobes
> or uprobes easily.
> 
> Suggested-by: Oleg Nesterov 
> Cc: Masami Hiramatsu 
> Cc: Srikar Dronamraju 
> Cc: Oleg Nesterov 
> Cc: zhangwei(Jovi) 
> Cc: Arnaldo Carvalho de Melo 
> Signed-off-by: Namhyung Kim 

Acked-by: Masami Hiramatsu 

Thank you!

> ---
>  Documentation/trace/uprobetracer.txt |  1 +
>  kernel/trace/trace_kprobe.c  |  8 
>  kernel/trace/trace_probe.c   | 13 +++-
>  kernel/trace/trace_probe.h   |  2 ++
>  kernel/trace/trace_uprobe.c  | 40 
> 
>  5 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/trace/uprobetracer.txt 
> b/Documentation/trace/uprobetracer.txt
> index 6e5cff263e2b..f1cf9a34ad9d 100644
> --- a/Documentation/trace/uprobetracer.txt
> +++ b/Documentation/trace/uprobetracer.txt
> @@ -32,6 +32,7 @@ Synopsis of uprobe_tracer
>FETCHARGS : Arguments. Each probe can have up to 128 args.
> %REG : Fetch register REG
> @ADDR : Fetch memory at ADDR (ADDR should be in userspace)
> +   @+OFFSET  : Fetch memory at OFFSET (OFFSET from same file as PATH)
> $stackN   : Fetch Nth entry of stack (N >= 0)
> $stack: Fetch stack address.
> $retval   : Fetch return value.(*)
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 8b32236ae890..1cf8f3375559 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -239,6 +239,14 @@ DEFINE_BASIC_FETCH_FUNCS(symbol)
>  DEFINE_FETCH_symbol(string)
>  DEFINE_FETCH_symbol(string_size)
>  
> +/* kprobes don't support file_offset fetch methods */
> +#define fetch_file_offset_u8 NULL
> +#define fetch_file_offset_u16NULL
> +#define fetch_file_offset_u32NULL
> +#define fetch_file_offset_u64NULL
> +#define fetch_file_offset_string NULL
> +#define fetch_file_offset_string_sizeNULL
> +
>  /* Fetch type information table */
>  const struct fetch_type kprobes_fetch_type_table[] = {
>   /* Special types */
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index 464ec506ec08..705c06ba0a8a 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -373,7 +373,7 @@ static int parse_probe_arg(char *arg, const struct 
> fetch_type *t,
>   }
>   break;
>  
> - case '@':   /* memory or symbol */
> + case '@':   /* memory, file-offset or symbol */
>   if (isdigit(arg[1])) {
>   ret = kstrtoul(arg + 1, 0, );
>   if (ret)
> @@ -381,6 +381,17 @@ static int parse_probe_arg(char *arg, const struct 
> fetch_type *t,
>  
>   f->fn = t->fetch[FETCH_MTD_memory];
>   f->data = (void *)param;
> + } else if (arg[1] == '+') {
> + /* kprobes don't support file offsets */
> + if (is_kprobe)
> + return -EINVAL;
> +
> + ret = kstrtol(arg + 2, 0, );
> + if (ret)
> + break;
> +
> + f->fn = t->fetch[FETCH_MTD_file_offset];
> + f->data = (void *)offset;
>   } else {
>   /* uprobes don't support symbols */
>   if (!is_kprobe)
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 385206bbbf8b..d9afeb580cbf 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -106,6 +106,7 @@ enum {
>   FETCH_MTD_symbol,
>   FETCH_MTD_deref,
>   FETCH_MTD_bitfield,
> + FETCH_MTD_file_offset,
>   FETCH_MTD_END,
>  };
>  
> @@ -217,6 +218,7 @@ ASSIGN_FETCH_FUNC(memory, ftype), \
>  ASSIGN_FETCH_FUNC(symbol, ftype),\
>  ASSIGN_FETCH_FUNC(deref, ftype), \
>  ASSIGN_FETCH_FUNC(bitfield, ftype),

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-12-03 Thread Masami Hiramatsu 
(2013/12/03 15:23), Namhyung Kim wrote:
 So do you want me to change to make it simpler without a fetch_param?

 Yes, yes. Assuming that you agree, of course.
 
 Okay, here goes v8 then. :)
 
 I also push it (with other changes come from Srikar) to the
 uprobe/fetch-v8 branch in my tree for your convenience. :)

This looks good for me :)

From 25bc2bab14e9461f8b15e34f26d1de51f96e44e1 Mon Sep 17 00:00:00 2001
 From: Namhyung Kim namhyung@lge.com
 Date: Mon, 25 Nov 2013 13:42:47 +0900
 Subject: [PATCH v8 17/17] tracing/uprobes: Add @+file_offset fetch method
 
 Enable to fetch data from a file offset.  Currently it only supports
 fetching from same binary uprobe set.  It'll translate the file offset
 to a proper virtual address in the process.
 
 The syntax is @+OFFSET as it does similar to normal memory fetching
 (@ADDR) which does no address translation.
 
 To do it, change fifth argument to receive 'void *priv' and make it
 set only for uprobes so that we can determine the arg is for kprobes
 or uprobes easily.
 
 Suggested-by: Oleg Nesterov o...@redhat.com
 Cc: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 Cc: Srikar Dronamraju sri...@linux.vnet.ibm.com
 Cc: Oleg Nesterov o...@redhat.com
 Cc: zhangwei(Jovi) jovi.zhang...@huawei.com
 Cc: Arnaldo Carvalho de Melo a...@ghostprotocols.net
 Signed-off-by: Namhyung Kim namhy...@kernel.org

Acked-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com

Thank you!

 ---
  Documentation/trace/uprobetracer.txt |  1 +
  kernel/trace/trace_kprobe.c  |  8 
  kernel/trace/trace_probe.c   | 13 +++-
  kernel/trace/trace_probe.h   |  2 ++
  kernel/trace/trace_uprobe.c  | 40 
 
  5 files changed, 63 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/trace/uprobetracer.txt 
 b/Documentation/trace/uprobetracer.txt
 index 6e5cff263e2b..f1cf9a34ad9d 100644
 --- a/Documentation/trace/uprobetracer.txt
 +++ b/Documentation/trace/uprobetracer.txt
 @@ -32,6 +32,7 @@ Synopsis of uprobe_tracer
FETCHARGS : Arguments. Each probe can have up to 128 args.
 %REG : Fetch register REG
 @ADDR : Fetch memory at ADDR (ADDR should be in userspace)
 +   @+OFFSET  : Fetch memory at OFFSET (OFFSET from same file as PATH)
 $stackN   : Fetch Nth entry of stack (N = 0)
 $stack: Fetch stack address.
 $retval   : Fetch return value.(*)
 diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
 index 8b32236ae890..1cf8f3375559 100644
 --- a/kernel/trace/trace_kprobe.c
 +++ b/kernel/trace/trace_kprobe.c
 @@ -239,6 +239,14 @@ DEFINE_BASIC_FETCH_FUNCS(symbol)
  DEFINE_FETCH_symbol(string)
  DEFINE_FETCH_symbol(string_size)
  
 +/* kprobes don't support file_offset fetch methods */
 +#define fetch_file_offset_u8 NULL
 +#define fetch_file_offset_u16NULL
 +#define fetch_file_offset_u32NULL
 +#define fetch_file_offset_u64NULL
 +#define fetch_file_offset_string NULL
 +#define fetch_file_offset_string_sizeNULL
 +
  /* Fetch type information table */
  const struct fetch_type kprobes_fetch_type_table[] = {
   /* Special types */
 diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
 index 464ec506ec08..705c06ba0a8a 100644
 --- a/kernel/trace/trace_probe.c
 +++ b/kernel/trace/trace_probe.c
 @@ -373,7 +373,7 @@ static int parse_probe_arg(char *arg, const struct 
 fetch_type *t,
   }
   break;
  
 - case '@':   /* memory or symbol */
 + case '@':   /* memory, file-offset or symbol */
   if (isdigit(arg[1])) {
   ret = kstrtoul(arg + 1, 0, param);
   if (ret)
 @@ -381,6 +381,17 @@ static int parse_probe_arg(char *arg, const struct 
 fetch_type *t,
  
   f-fn = t-fetch[FETCH_MTD_memory];
   f-data = (void *)param;
 + } else if (arg[1] == '+') {
 + /* kprobes don't support file offsets */
 + if (is_kprobe)
 + return -EINVAL;
 +
 + ret = kstrtol(arg + 2, 0, offset);
 + if (ret)
 + break;
 +
 + f-fn = t-fetch[FETCH_MTD_file_offset];
 + f-data = (void *)offset;
   } else {
   /* uprobes don't support symbols */
   if (!is_kprobe)
 diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
 index 385206bbbf8b..d9afeb580cbf 100644
 --- a/kernel/trace/trace_probe.h
 +++ b/kernel/trace/trace_probe.h
 @@ -106,6 +106,7 @@ enum {
   FETCH_MTD_symbol,
   FETCH_MTD_deref,
   FETCH_MTD_bitfield,
 + FETCH_MTD_file_offset,
   FETCH_MTD_END,
  };
  
 @@ -217,6 +218,7 @@ ASSIGN_FETCH_FUNC(memory, ftype), \
  ASSIGN_FETCH_FUNC(symbol, ftype),\

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-12-02 Thread Namhyung Kim 
>> So do you want me to change to make it simpler without a fetch_param?
>
> Yes, yes. Assuming that you agree, of course.

Okay, here goes v8 then. :)

I also push it (with other changes come from Srikar) to the
uprobe/fetch-v8 branch in my tree for your convenience. :)

Thanks,
Namhyung


>From 25bc2bab14e9461f8b15e34f26d1de51f96e44e1 Mon Sep 17 00:00:00 2001
From: Namhyung Kim 
Date: Mon, 25 Nov 2013 13:42:47 +0900
Subject: [PATCH v8 17/17] tracing/uprobes: Add @+file_offset fetch method

Enable to fetch data from a file offset.  Currently it only supports
fetching from same binary uprobe set.  It'll translate the file offset
to a proper virtual address in the process.

The syntax is "@+OFFSET" as it does similar to normal memory fetching
(@ADDR) which does no address translation.

To do it, change fifth argument to receive 'void *priv' and make it
set only for uprobes so that we can determine the arg is for kprobes
or uprobes easily.

Suggested-by: Oleg Nesterov 
Cc: Masami Hiramatsu 
Cc: Srikar Dronamraju 
Cc: Oleg Nesterov 
Cc: zhangwei(Jovi) 
Cc: Arnaldo Carvalho de Melo 
Signed-off-by: Namhyung Kim 
---
 Documentation/trace/uprobetracer.txt |  1 +
 kernel/trace/trace_kprobe.c  |  8 
 kernel/trace/trace_probe.c   | 13 +++-
 kernel/trace/trace_probe.h   |  2 ++
 kernel/trace/trace_uprobe.c  | 40 
 5 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/uprobetracer.txt 
b/Documentation/trace/uprobetracer.txt
index 6e5cff263e2b..f1cf9a34ad9d 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -32,6 +32,7 @@ Synopsis of uprobe_tracer
   FETCHARGS : Arguments. Each probe can have up to 128 args.
%REG : Fetch register REG
@ADDR   : Fetch memory at ADDR (ADDR should be in userspace)
+   @+OFFSET: Fetch memory at OFFSET (OFFSET from same file as PATH)
$stackN : Fetch Nth entry of stack (N >= 0)
$stack  : Fetch stack address.
$retval : Fetch return value.(*)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 8b32236ae890..1cf8f3375559 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -239,6 +239,14 @@ DEFINE_BASIC_FETCH_FUNCS(symbol)
 DEFINE_FETCH_symbol(string)
 DEFINE_FETCH_symbol(string_size)
 
+/* kprobes don't support file_offset fetch methods */
+#define fetch_file_offset_u8   NULL
+#define fetch_file_offset_u16  NULL
+#define fetch_file_offset_u32  NULL
+#define fetch_file_offset_u64  NULL
+#define fetch_file_offset_string   NULL
+#define fetch_file_offset_string_size  NULL
+
 /* Fetch type information table */
 const struct fetch_type kprobes_fetch_type_table[] = {
/* Special types */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 464ec506ec08..705c06ba0a8a 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -373,7 +373,7 @@ static int parse_probe_arg(char *arg, const struct 
fetch_type *t,
}
break;
 
-   case '@':   /* memory or symbol */
+   case '@':   /* memory, file-offset or symbol */
if (isdigit(arg[1])) {
ret = kstrtoul(arg + 1, 0, );
if (ret)
@@ -381,6 +381,17 @@ static int parse_probe_arg(char *arg, const struct 
fetch_type *t,
 
f->fn = t->fetch[FETCH_MTD_memory];
f->data = (void *)param;
+   } else if (arg[1] == '+') {
+   /* kprobes don't support file offsets */
+   if (is_kprobe)
+   return -EINVAL;
+
+   ret = kstrtol(arg + 2, 0, );
+   if (ret)
+   break;
+
+   f->fn = t->fetch[FETCH_MTD_file_offset];
+   f->data = (void *)offset;
} else {
/* uprobes don't support symbols */
if (!is_kprobe)
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 385206bbbf8b..d9afeb580cbf 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -106,6 +106,7 @@ enum {
FETCH_MTD_symbol,
FETCH_MTD_deref,
FETCH_MTD_bitfield,
+   FETCH_MTD_file_offset,
FETCH_MTD_END,
 };
 
@@ -217,6 +218,7 @@ ASSIGN_FETCH_FUNC(memory, ftype),   \
 ASSIGN_FETCH_FUNC(symbol, ftype),  \
 ASSIGN_FETCH_FUNC(deref, ftype),   \
 ASSIGN_FETCH_FUNC(bitfield, ftype),\
+ASSIGN_FETCH_FUNC(file_offset, ftype), \
  } \
}
 
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index f86a6a711de9..f5cbed7e7709 100644
---

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-12-02 Thread Namhyung Kim 
 So do you want me to change to make it simpler without a fetch_param?

 Yes, yes. Assuming that you agree, of course.

Okay, here goes v8 then. :)

I also push it (with other changes come from Srikar) to the
uprobe/fetch-v8 branch in my tree for your convenience. :)

Thanks,
Namhyung


From 25bc2bab14e9461f8b15e34f26d1de51f96e44e1 Mon Sep 17 00:00:00 2001
From: Namhyung Kim namhyung@lge.com
Date: Mon, 25 Nov 2013 13:42:47 +0900
Subject: [PATCH v8 17/17] tracing/uprobes: Add @+file_offset fetch method

Enable to fetch data from a file offset.  Currently it only supports
fetching from same binary uprobe set.  It'll translate the file offset
to a proper virtual address in the process.

The syntax is @+OFFSET as it does similar to normal memory fetching
(@ADDR) which does no address translation.

To do it, change fifth argument to receive 'void *priv' and make it
set only for uprobes so that we can determine the arg is for kprobes
or uprobes easily.

Suggested-by: Oleg Nesterov o...@redhat.com
Cc: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Srikar Dronamraju sri...@linux.vnet.ibm.com
Cc: Oleg Nesterov o...@redhat.com
Cc: zhangwei(Jovi) jovi.zhang...@huawei.com
Cc: Arnaldo Carvalho de Melo a...@ghostprotocols.net
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
 Documentation/trace/uprobetracer.txt |  1 +
 kernel/trace/trace_kprobe.c  |  8 
 kernel/trace/trace_probe.c   | 13 +++-
 kernel/trace/trace_probe.h   |  2 ++
 kernel/trace/trace_uprobe.c  | 40 
 5 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/uprobetracer.txt 
b/Documentation/trace/uprobetracer.txt
index 6e5cff263e2b..f1cf9a34ad9d 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -32,6 +32,7 @@ Synopsis of uprobe_tracer
   FETCHARGS : Arguments. Each probe can have up to 128 args.
%REG : Fetch register REG
@ADDR   : Fetch memory at ADDR (ADDR should be in userspace)
+   @+OFFSET: Fetch memory at OFFSET (OFFSET from same file as PATH)
$stackN : Fetch Nth entry of stack (N = 0)
$stack  : Fetch stack address.
$retval : Fetch return value.(*)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 8b32236ae890..1cf8f3375559 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -239,6 +239,14 @@ DEFINE_BASIC_FETCH_FUNCS(symbol)
 DEFINE_FETCH_symbol(string)
 DEFINE_FETCH_symbol(string_size)
 
+/* kprobes don't support file_offset fetch methods */
+#define fetch_file_offset_u8   NULL
+#define fetch_file_offset_u16  NULL
+#define fetch_file_offset_u32  NULL
+#define fetch_file_offset_u64  NULL
+#define fetch_file_offset_string   NULL
+#define fetch_file_offset_string_size  NULL
+
 /* Fetch type information table */
 const struct fetch_type kprobes_fetch_type_table[] = {
/* Special types */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 464ec506ec08..705c06ba0a8a 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -373,7 +373,7 @@ static int parse_probe_arg(char *arg, const struct 
fetch_type *t,
}
break;
 
-   case '@':   /* memory or symbol */
+   case '@':   /* memory, file-offset or symbol */
if (isdigit(arg[1])) {
ret = kstrtoul(arg + 1, 0, param);
if (ret)
@@ -381,6 +381,17 @@ static int parse_probe_arg(char *arg, const struct 
fetch_type *t,
 
f-fn = t-fetch[FETCH_MTD_memory];
f-data = (void *)param;
+   } else if (arg[1] == '+') {
+   /* kprobes don't support file offsets */
+   if (is_kprobe)
+   return -EINVAL;
+
+   ret = kstrtol(arg + 2, 0, offset);
+   if (ret)
+   break;
+
+   f-fn = t-fetch[FETCH_MTD_file_offset];
+   f-data = (void *)offset;
} else {
/* uprobes don't support symbols */
if (!is_kprobe)
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 385206bbbf8b..d9afeb580cbf 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -106,6 +106,7 @@ enum {
FETCH_MTD_symbol,
FETCH_MTD_deref,
FETCH_MTD_bitfield,
+   FETCH_MTD_file_offset,
FETCH_MTD_END,
 };
 
@@ -217,6 +218,7 @@ ASSIGN_FETCH_FUNC(memory, ftype),   \
 ASSIGN_FETCH_FUNC(symbol, ftype),  \
 ASSIGN_FETCH_FUNC(deref, ftype),   \
 ASSIGN_FETCH_FUNC(bitfield, ftype),\
+ASSIGN_FETCH_FUNC(file_offset, ftype), \
  }

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-29 Thread Oleg Nesterov 
Hi Namhyung,

On 11/29, Namhyung Kim wrote:
>
> Hi Oleg,
>
> On Thu, 28 Nov 2013 17:31:48 +0100, Oleg Nesterov wrote:
> > On 11/28, Namhyung Kim wrote:
> >>
> >> I thought we need a fetch_param anyway if we will add support for
> >> cross-fetch later.  But I won't insist it strongly, I can delay it to
> >> later work and make current code simpler if you want. :)
> >
> > OK, great,
>
> So do you want me to change to make it simpler without a fetch_param?

Yes, yes. Assuming that you agree, of course.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-29 Thread Oleg Nesterov 
Hi Namhyung,

On 11/29, Namhyung Kim wrote:

 Hi Oleg,

 On Thu, 28 Nov 2013 17:31:48 +0100, Oleg Nesterov wrote:
  On 11/28, Namhyung Kim wrote:
 
  I thought we need a fetch_param anyway if we will add support for
  cross-fetch later.  But I won't insist it strongly, I can delay it to
  later work and make current code simpler if you want. :)
 
  OK, great,

 So do you want me to change to make it simpler without a fetch_param?

Yes, yes. Assuming that you agree, of course.

Oleg.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-28 Thread Namhyung Kim 
Hi Oleg,

On Thu, 28 Nov 2013 17:31:48 +0100, Oleg Nesterov wrote:
> On 11/28, Namhyung Kim wrote:
>>
>> I thought we need a fetch_param anyway if we will add support for
>> cross-fetch later.  But I won't insist it strongly, I can delay it to
>> later work and make current code simpler if you want. :)
>
> OK, great,

So do you want me to change to make it simpler without a fetch_param?

>
>> >>  static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs 
>> >> *regs)
>> >>  {
>> >>   struct trace_uprobe *tu;
>> >> + struct uprobe_task *utask;
>> >>   int ret = 0;
>> >>
>> >>   tu = container_of(con, struct trace_uprobe, consumer);
>> >>   tu->nhit++;
>> >>
>> >> + utask = current->utask;
>> >> + if (utask == NULL)
>> >> + return UPROBE_HANDLER_REMOVE;
>> >
>> > Hmm, why? The previous change ensures ->utask is not NULL? If we hit
>> > NULL we have a bug, we should not remove this uprobe.
>>
>> Yes, I just want to be defensive. :)
>>
>> So do you suggest to add BUG_ON()?
>
> We are going to crash with the same effect if it is NULL ;)

I see.  I'll just get rid of the if (...) part.

>
>> And can I convert or remove a
>> similar check in uprobes.c:pre_ssout() too?
>
> Well, yes, we _can_ do this. But unless you have the strong opinion
> I'd suggest to not do this. At least right now.
>
> To remind, perhaps we can revert the previous patch later if we find
> a better solution (placeholder).

OK, I'll just leave it as is.

>
> And. Note that we will change this code in any case. I suggested to
> use ->vaddr to avoid the other (potentially conflicting) changes in
> uprobes.h. Even if we use current->utask, we should add another member
> into the union. But again, it would be better to do this later, and
> the change will be trivial.

Got it.  Thank you for the explanation.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-28 Thread Oleg Nesterov 
On 11/28, Namhyung Kim wrote:
>
> I thought we need a fetch_param anyway if we will add support for
> cross-fetch later.  But I won't insist it strongly, I can delay it to
> later work and make current code simpler if you want. :)

OK, great,

> >>  static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs 
> >> *regs)
> >>  {
> >>struct trace_uprobe *tu;
> >> +  struct uprobe_task *utask;
> >>int ret = 0;
> >>
> >>tu = container_of(con, struct trace_uprobe, consumer);
> >>tu->nhit++;
> >>
> >> +  utask = current->utask;
> >> +  if (utask == NULL)
> >> +  return UPROBE_HANDLER_REMOVE;
> >
> > Hmm, why? The previous change ensures ->utask is not NULL? If we hit
> > NULL we have a bug, we should not remove this uprobe.
>
> Yes, I just want to be defensive. :)
>
> So do you suggest to add BUG_ON()?

We are going to crash with the same effect if it is NULL ;)

> And can I convert or remove a
> similar check in uprobes.c:pre_ssout() too?

Well, yes, we _can_ do this. But unless you have the strong opinion
I'd suggest to not do this. At least right now.

To remind, perhaps we can revert the previous patch later if we find
a better solution (placeholder).

And. Note that we will change this code in any case. I suggested to
use ->vaddr to avoid the other (potentially conflicting) changes in
uprobes.h. Even if we use current->utask, we should add another member
into the union. But again, it would be better to do this later, and
the change will be trivial.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-28 Thread Oleg Nesterov 
On 11/28, Namhyung Kim wrote:

 I thought we need a fetch_param anyway if we will add support for
 cross-fetch later.  But I won't insist it strongly, I can delay it to
 later work and make current code simpler if you want. :)

OK, great,

   static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs 
  *regs)
   {
 struct trace_uprobe *tu;
  +  struct uprobe_task *utask;
 int ret = 0;
 
 tu = container_of(con, struct trace_uprobe, consumer);
 tu-nhit++;
 
  +  utask = current-utask;
  +  if (utask == NULL)
  +  return UPROBE_HANDLER_REMOVE;
 
  Hmm, why? The previous change ensures -utask is not NULL? If we hit
  NULL we have a bug, we should not remove this uprobe.

 Yes, I just want to be defensive. :)

 So do you suggest to add BUG_ON()?

We are going to crash with the same effect if it is NULL ;)

 And can I convert or remove a
 similar check in uprobes.c:pre_ssout() too?

Well, yes, we _can_ do this. But unless you have the strong opinion
I'd suggest to not do this. At least right now.

To remind, perhaps we can revert the previous patch later if we find
a better solution (placeholder).

And. Note that we will change this code in any case. I suggested to
use -vaddr to avoid the other (potentially conflicting) changes in
uprobes.h. Even if we use current-utask, we should add another member
into the union. But again, it would be better to do this later, and
the change will be trivial.

Oleg.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-28 Thread Namhyung Kim 
Hi Oleg,

On Thu, 28 Nov 2013 17:31:48 +0100, Oleg Nesterov wrote:
 On 11/28, Namhyung Kim wrote:

 I thought we need a fetch_param anyway if we will add support for
 cross-fetch later.  But I won't insist it strongly, I can delay it to
 later work and make current code simpler if you want. :)

 OK, great,

So do you want me to change to make it simpler without a fetch_param?


   static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs 
  *regs)
   {
struct trace_uprobe *tu;
  + struct uprobe_task *utask;
int ret = 0;
 
tu = container_of(con, struct trace_uprobe, consumer);
tu-nhit++;
 
  + utask = current-utask;
  + if (utask == NULL)
  + return UPROBE_HANDLER_REMOVE;
 
  Hmm, why? The previous change ensures -utask is not NULL? If we hit
  NULL we have a bug, we should not remove this uprobe.

 Yes, I just want to be defensive. :)

 So do you suggest to add BUG_ON()?

 We are going to crash with the same effect if it is NULL ;)

I see.  I'll just get rid of the if (...) part.


 And can I convert or remove a
 similar check in uprobes.c:pre_ssout() too?

 Well, yes, we _can_ do this. But unless you have the strong opinion
 I'd suggest to not do this. At least right now.

 To remind, perhaps we can revert the previous patch later if we find
 a better solution (placeholder).

OK, I'll just leave it as is.


 And. Note that we will change this code in any case. I suggested to
 use -vaddr to avoid the other (potentially conflicting) changes in
 uprobes.h. Even if we use current-utask, we should add another member
 into the union. But again, it would be better to do this later, and
 the change will be trivial.

Got it.  Thank you for the explanation.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-27 Thread Namhyung Kim 
Hi Oleg,

On Wed, 27 Nov 2013 19:55:46 +0100, Oleg Nesterov wrote:
> Hi Namhyung,
>
> I'll certainly try to read (and even apply ;) this series carefully.

Thanks in advance. :)

>
> But let me make a couple of nits right now, even if I do not understand
> this code yet.

Okay.

>
> On 11/27, Namhyung Kim wrote:
>>
>> +} else if (arg[1] == '+') {
>> +struct file_offset_fetch_param *foprm;
>> +
>> +/* kprobes don't support file offsets */
>> +if (is_kprobe)
>> +return -EINVAL;
>> +
>> +ret = kstrtol(arg + 2, 0, );
>> +if (ret)
>> +break;
>> +
>> +foprm = kzalloc(sizeof(*foprm), GFP_KERNEL);
>> +if (!foprm)
>> +return -ENOMEM;
>> +
>> +foprm->tu = priv;
>> +foprm->offset = offset;
>
> Hmm. I am not sure, but can't we simplify this?
>
> Why do we need this foprm at all? To pass tu/offset obviously. But
> why we need to store this info in fetch_param?
>
> translate_user_vaddr() needs to access utask->vaddr anyway. It seems
> to me it would be more clean to do the following:
>
>   1. Add
>   struct xxx {
>   struct trace_uprobe *tu;
>   unsigned long bp_addr;
>   };
>
>  in trace_uprobe.c.
>
>   2. Add
>
>   struct xxx info = {
>   .tu = tu,
>   .bp_addr = instruction_pointer(regs);
>   };
>
>   current->utask->vaddr = (long)
>
>  into uprobe_dispatcher() and uretprobe_dispatcher() (the latter
>  should obviously use func instead of instruction_pointer).
>
>3. FETCH_FUNC_NAME(file_offset, type) can do
>
>   struct xxx *info = (void*)current->utask->vaddr;
>   void *addr = data + info->bp_addr - info->tu->offset;
>
>   return FETCH_FUNC_NAME(memory, type)(regs, aaddr, dest);
>
>4. Now, the only change we need in parse_probe_arg("@") is that
>   it should use either FETCH_MTD_memory or FETCH_MTD_file_offset
>   depending on arg[0] == '+'.
>
>   And we do not need to pass "void *prive" to parse_probe_arg().
>
> What do you think? One again, I can be easily wrong, I didn't read the
> code yet.

You are absolutely right.

I thought we need a fetch_param anyway if we will add support for
cross-fetch later.  But I won't insist it strongly, I can delay it to
later work and make current code simpler if you want. :)

>
>>  static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs 
>> *regs)
>>  {
>>  struct trace_uprobe *tu;
>> +struct uprobe_task *utask;
>>  int ret = 0;
>>  
>>  tu = container_of(con, struct trace_uprobe, consumer);
>>  tu->nhit++;
>>  
>> +utask = current->utask;
>> +if (utask == NULL)
>> +return UPROBE_HANDLER_REMOVE;
>
> Hmm, why? The previous change ensures ->utask is not NULL? If we hit
> NULL we have a bug, we should not remove this uprobe.

Yes, I just want to be defensive. :)

So do you suggest to add BUG_ON()?  And can I convert or remove a
similar check in uprobes.c:pre_ssout() too?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-27 Thread Oleg Nesterov 
Hi Namhyung,

I'll certainly try to read (and even apply ;) this series carefully.

But let me make a couple of nits right now, even if I do not understand
this code yet.

On 11/27, Namhyung Kim wrote:
>
> + } else if (arg[1] == '+') {
> + struct file_offset_fetch_param *foprm;
> +
> + /* kprobes don't support file offsets */
> + if (is_kprobe)
> + return -EINVAL;
> +
> + ret = kstrtol(arg + 2, 0, );
> + if (ret)
> + break;
> +
> + foprm = kzalloc(sizeof(*foprm), GFP_KERNEL);
> + if (!foprm)
> + return -ENOMEM;
> +
> + foprm->tu = priv;
> + foprm->offset = offset;

Hmm. I am not sure, but can't we simplify this?

Why do we need this foprm at all? To pass tu/offset obviously. But
why we need to store this info in fetch_param?

translate_user_vaddr() needs to access utask->vaddr anyway. It seems
to me it would be more clean to do the following:

1. Add
struct xxx {
struct trace_uprobe *tu;
unsigned long bp_addr;
};

   in trace_uprobe.c.

2. Add

struct xxx info = {
.tu = tu,
.bp_addr = instruction_pointer(regs);
};

current->utask->vaddr = (long)

   into uprobe_dispatcher() and uretprobe_dispatcher() (the latter
   should obviously use func instead of instruction_pointer).

 3. FETCH_FUNC_NAME(file_offset, type) can do

struct xxx *info = (void*)current->utask->vaddr;
void *addr = data + info->bp_addr - info->tu->offset;

return FETCH_FUNC_NAME(memory, type)(regs, aaddr, dest);

 4. Now, the only change we need in parse_probe_arg("@") is that
it should use either FETCH_MTD_memory or FETCH_MTD_file_offset
depending on arg[0] == '+'.

And we do not need to pass "void *prive" to parse_probe_arg().

What do you think? One again, I can be easily wrong, I didn't read the
code yet.

>  static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs 
> *regs)
>  {
>   struct trace_uprobe *tu;
> + struct uprobe_task *utask;
>   int ret = 0;
>  
>   tu = container_of(con, struct trace_uprobe, consumer);
>   tu->nhit++;
>  
> + utask = current->utask;
> + if (utask == NULL)
> + return UPROBE_HANDLER_REMOVE;

Hmm, why? The previous change ensures ->utask is not NULL? If we hit
NULL we have a bug, we should not remove this uprobe.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-27 Thread Oleg Nesterov 
Hi Namhyung,

I'll certainly try to read (and even apply ;) this series carefully.

But let me make a couple of nits right now, even if I do not understand
this code yet.

On 11/27, Namhyung Kim wrote:

 + } else if (arg[1] == '+') {
 + struct file_offset_fetch_param *foprm;
 +
 + /* kprobes don't support file offsets */
 + if (is_kprobe)
 + return -EINVAL;
 +
 + ret = kstrtol(arg + 2, 0, offset);
 + if (ret)
 + break;
 +
 + foprm = kzalloc(sizeof(*foprm), GFP_KERNEL);
 + if (!foprm)
 + return -ENOMEM;
 +
 + foprm-tu = priv;
 + foprm-offset = offset;

Hmm. I am not sure, but can't we simplify this?

Why do we need this foprm at all? To pass tu/offset obviously. But
why we need to store this info in fetch_param?

translate_user_vaddr() needs to access utask-vaddr anyway. It seems
to me it would be more clean to do the following:

1. Add
struct xxx {
struct trace_uprobe *tu;
unsigned long bp_addr;
};

   in trace_uprobe.c.

2. Add

struct xxx info = {
.tu = tu,
.bp_addr = instruction_pointer(regs);
};

current-utask-vaddr = (long)info;

   into uprobe_dispatcher() and uretprobe_dispatcher() (the latter
   should obviously use func instead of instruction_pointer).

 3. FETCH_FUNC_NAME(file_offset, type) can do

struct xxx *info = (void*)current-utask-vaddr;
void *addr = data + info-bp_addr - info-tu-offset;

return FETCH_FUNC_NAME(memory, type)(regs, aaddr, dest);

 4. Now, the only change we need in parse_probe_arg(@) is that
it should use either FETCH_MTD_memory or FETCH_MTD_file_offset
depending on arg[0] == '+'.

And we do not need to pass void *prive to parse_probe_arg().

What do you think? One again, I can be easily wrong, I didn't read the
code yet.

  static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs 
 *regs)
  {
   struct trace_uprobe *tu;
 + struct uprobe_task *utask;
   int ret = 0;
  
   tu = container_of(con, struct trace_uprobe, consumer);
   tu-nhit++;
  
 + utask = current-utask;
 + if (utask == NULL)
 + return UPROBE_HANDLER_REMOVE;

Hmm, why? The previous change ensures -utask is not NULL? If we hit
NULL we have a bug, we should not remove this uprobe.

Oleg.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-27 Thread Namhyung Kim 
Hi Oleg,

On Wed, 27 Nov 2013 19:55:46 +0100, Oleg Nesterov wrote:
 Hi Namhyung,

 I'll certainly try to read (and even apply ;) this series carefully.

Thanks in advance. :)


 But let me make a couple of nits right now, even if I do not understand
 this code yet.

Okay.


 On 11/27, Namhyung Kim wrote:

 +} else if (arg[1] == '+') {
 +struct file_offset_fetch_param *foprm;
 +
 +/* kprobes don't support file offsets */
 +if (is_kprobe)
 +return -EINVAL;
 +
 +ret = kstrtol(arg + 2, 0, offset);
 +if (ret)
 +break;
 +
 +foprm = kzalloc(sizeof(*foprm), GFP_KERNEL);
 +if (!foprm)
 +return -ENOMEM;
 +
 +foprm-tu = priv;
 +foprm-offset = offset;

 Hmm. I am not sure, but can't we simplify this?

 Why do we need this foprm at all? To pass tu/offset obviously. But
 why we need to store this info in fetch_param?

 translate_user_vaddr() needs to access utask-vaddr anyway. It seems
 to me it would be more clean to do the following:

   1. Add
   struct xxx {
   struct trace_uprobe *tu;
   unsigned long bp_addr;
   };

  in trace_uprobe.c.

   2. Add

   struct xxx info = {
   .tu = tu,
   .bp_addr = instruction_pointer(regs);
   };

   current-utask-vaddr = (long)info;

  into uprobe_dispatcher() and uretprobe_dispatcher() (the latter
  should obviously use func instead of instruction_pointer).

3. FETCH_FUNC_NAME(file_offset, type) can do

   struct xxx *info = (void*)current-utask-vaddr;
   void *addr = data + info-bp_addr - info-tu-offset;

   return FETCH_FUNC_NAME(memory, type)(regs, aaddr, dest);

4. Now, the only change we need in parse_probe_arg(@) is that
   it should use either FETCH_MTD_memory or FETCH_MTD_file_offset
   depending on arg[0] == '+'.

   And we do not need to pass void *prive to parse_probe_arg().

 What do you think? One again, I can be easily wrong, I didn't read the
 code yet.

You are absolutely right.

I thought we need a fetch_param anyway if we will add support for
cross-fetch later.  But I won't insist it strongly, I can delay it to
later work and make current code simpler if you want. :)


  static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs 
 *regs)
  {
  struct trace_uprobe *tu;
 +struct uprobe_task *utask;
  int ret = 0;
  
  tu = container_of(con, struct trace_uprobe, consumer);
  tu-nhit++;
  
 +utask = current-utask;
 +if (utask == NULL)
 +return UPROBE_HANDLER_REMOVE;

 Hmm, why? The previous change ensures -utask is not NULL? If we hit
 NULL we have a bug, we should not remove this uprobe.

Yes, I just want to be defensive. :)

So do you suggest to add BUG_ON()?  And can I convert or remove a
similar check in uprobes.c:pre_ssout() too?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-26 Thread Namhyung Kim 
From: Namhyung Kim 

Enable to fetch data from a file offset.  Currently it only supports
fetching from same binary uprobe set.  It'll translate the file offset
to a proper virtual address in the process.

The syntax is "@+OFFSET" as it does similar to normal memory fetching
(@ADDR) which does no address translation.

To do it, change fifth argument to receive 'void *priv' and make it
set only for uprobes so that we can determine the arg is for kprobes
or uprobes easily.

Suggested-by: Oleg Nesterov 
Cc: Masami Hiramatsu 
Cc: Srikar Dronamraju 
Cc: Oleg Nesterov 
Cc: zhangwei(Jovi) 
Cc: Arnaldo Carvalho de Melo 
Signed-off-by: Namhyung Kim 
---
 Documentation/trace/uprobetracer.txt |  1 +
 kernel/trace/trace_kprobe.c  | 10 +-
 kernel/trace/trace_probe.c   | 32 +++-
 kernel/trace/trace_probe.h   |  9 -
 kernel/trace/trace_uprobe.c  | 36 +++-
 5 files changed, 80 insertions(+), 8 deletions(-)

diff --git a/Documentation/trace/uprobetracer.txt 
b/Documentation/trace/uprobetracer.txt
index 6e5cff263e2b..f1cf9a34ad9d 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -32,6 +32,7 @@ Synopsis of uprobe_tracer
   FETCHARGS : Arguments. Each probe can have up to 128 args.
%REG : Fetch register REG
@ADDR   : Fetch memory at ADDR (ADDR should be in userspace)
+   @+OFFSET: Fetch memory at OFFSET (OFFSET from same file as PATH)
$stackN : Fetch Nth entry of stack (N >= 0)
$stack  : Fetch stack address.
$retval : Fetch return value.(*)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5c4f72d45b97..300fd0f8b2a9 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -239,6 +239,14 @@ DEFINE_BASIC_FETCH_FUNCS(symbol)
 DEFINE_FETCH_symbol(string)
 DEFINE_FETCH_symbol(string_size)
 
+/* kprobes don't support file_offset fetch methods */
+#define fetch_file_offset_u8   NULL
+#define fetch_file_offset_u16  NULL
+#define fetch_file_offset_u32  NULL
+#define fetch_file_offset_u64  NULL
+#define fetch_file_offset_string   NULL
+#define fetch_file_offset_string_size  NULL
+
 /* Fetch type information table */
 const struct fetch_type kprobes_fetch_type_table[] = {
/* Special types */
@@ -758,7 +766,7 @@ static int create_trace_kprobe(int argc, char **argv)
 
/* Parse fetch argument */
ret = traceprobe_parse_probe_arg(arg, >p.size, parg,
-   is_return, true);
+   is_return, NULL);
if (ret) {
pr_info("Parse error at argument[%d]. (%d)\n", i, ret);
goto error;
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 1be603d45811..bd16aed615b2 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -348,13 +348,14 @@ static int parse_probe_vars(char *arg, const struct 
fetch_type *t,
 
 /* Recursive argument parser */
 static int parse_probe_arg(char *arg, const struct fetch_type *t,
-struct fetch_param *f, bool is_return, bool is_kprobe)
+struct fetch_param *f, bool is_return, void *priv)
 {
const struct fetch_type *ftbl;
unsigned long param;
long offset;
char *tmp;
int ret = 0;
+   bool is_kprobe = (priv == NULL);
 
ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
BUG_ON(ftbl == NULL);
@@ -373,7 +374,7 @@ static int parse_probe_arg(char *arg, const struct 
fetch_type *t,
}
break;
 
-   case '@':   /* memory or symbol */
+   case '@':   /* memory, file-offset or symbol */
if (isdigit(arg[1])) {
ret = kstrtoul(arg + 1, 0, );
if (ret)
@@ -381,6 +382,26 @@ static int parse_probe_arg(char *arg, const struct 
fetch_type *t,
 
f->fn = t->fetch[FETCH_MTD_memory];
f->data = (void *)param;
+   } else if (arg[1] == '+') {
+   struct file_offset_fetch_param *foprm;
+
+   /* kprobes don't support file offsets */
+   if (is_kprobe)
+   return -EINVAL;
+
+   ret = kstrtol(arg + 2, 0, );
+   if (ret)
+   break;
+
+   foprm = kzalloc(sizeof(*foprm), GFP_KERNEL);
+   if (!foprm)
+   return -ENOMEM;
+
+   foprm->tu = priv;
+   foprm->offset = offset;
+
+   f->fn = t->fetch[FETCH_MTD_file_offset];
+   f->data = foprm;
} else {

[PATCH/RFC 17/17] tracing/uprobes: Add @+file_offset fetch method

2013-11-26 Thread Namhyung Kim 
From: Namhyung Kim namhyung@lge.com

Enable to fetch data from a file offset.  Currently it only supports
fetching from same binary uprobe set.  It'll translate the file offset
to a proper virtual address in the process.

The syntax is @+OFFSET as it does similar to normal memory fetching
(@ADDR) which does no address translation.

To do it, change fifth argument to receive 'void *priv' and make it
set only for uprobes so that we can determine the arg is for kprobes
or uprobes easily.

Suggested-by: Oleg Nesterov o...@redhat.com
Cc: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Srikar Dronamraju sri...@linux.vnet.ibm.com
Cc: Oleg Nesterov o...@redhat.com
Cc: zhangwei(Jovi) jovi.zhang...@huawei.com
Cc: Arnaldo Carvalho de Melo a...@ghostprotocols.net
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
 Documentation/trace/uprobetracer.txt |  1 +
 kernel/trace/trace_kprobe.c  | 10 +-
 kernel/trace/trace_probe.c   | 32 +++-
 kernel/trace/trace_probe.h   |  9 -
 kernel/trace/trace_uprobe.c  | 36 +++-
 5 files changed, 80 insertions(+), 8 deletions(-)

diff --git a/Documentation/trace/uprobetracer.txt 
b/Documentation/trace/uprobetracer.txt
index 6e5cff263e2b..f1cf9a34ad9d 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -32,6 +32,7 @@ Synopsis of uprobe_tracer
   FETCHARGS : Arguments. Each probe can have up to 128 args.
%REG : Fetch register REG
@ADDR   : Fetch memory at ADDR (ADDR should be in userspace)
+   @+OFFSET: Fetch memory at OFFSET (OFFSET from same file as PATH)
$stackN : Fetch Nth entry of stack (N = 0)
$stack  : Fetch stack address.
$retval : Fetch return value.(*)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5c4f72d45b97..300fd0f8b2a9 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -239,6 +239,14 @@ DEFINE_BASIC_FETCH_FUNCS(symbol)
 DEFINE_FETCH_symbol(string)
 DEFINE_FETCH_symbol(string_size)
 
+/* kprobes don't support file_offset fetch methods */
+#define fetch_file_offset_u8   NULL
+#define fetch_file_offset_u16  NULL
+#define fetch_file_offset_u32  NULL
+#define fetch_file_offset_u64  NULL
+#define fetch_file_offset_string   NULL
+#define fetch_file_offset_string_size  NULL
+
 /* Fetch type information table */
 const struct fetch_type kprobes_fetch_type_table[] = {
/* Special types */
@@ -758,7 +766,7 @@ static int create_trace_kprobe(int argc, char **argv)
 
/* Parse fetch argument */
ret = traceprobe_parse_probe_arg(arg, tp-p.size, parg,
-   is_return, true);
+   is_return, NULL);
if (ret) {
pr_info(Parse error at argument[%d]. (%d)\n, i, ret);
goto error;
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 1be603d45811..bd16aed615b2 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -348,13 +348,14 @@ static int parse_probe_vars(char *arg, const struct 
fetch_type *t,
 
 /* Recursive argument parser */
 static int parse_probe_arg(char *arg, const struct fetch_type *t,
-struct fetch_param *f, bool is_return, bool is_kprobe)
+struct fetch_param *f, bool is_return, void *priv)
 {
const struct fetch_type *ftbl;
unsigned long param;
long offset;
char *tmp;
int ret = 0;
+   bool is_kprobe = (priv == NULL);
 
ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
BUG_ON(ftbl == NULL);
@@ -373,7 +374,7 @@ static int parse_probe_arg(char *arg, const struct 
fetch_type *t,
}
break;
 
-   case '@':   /* memory or symbol */
+   case '@':   /* memory, file-offset or symbol */
if (isdigit(arg[1])) {
ret = kstrtoul(arg + 1, 0, param);
if (ret)
@@ -381,6 +382,26 @@ static int parse_probe_arg(char *arg, const struct 
fetch_type *t,
 
f-fn = t-fetch[FETCH_MTD_memory];
f-data = (void *)param;
+   } else if (arg[1] == '+') {
+   struct file_offset_fetch_param *foprm;
+
+   /* kprobes don't support file offsets */
+   if (is_kprobe)
+   return -EINVAL;
+
+   ret = kstrtol(arg + 2, 0, offset);
+   if (ret)
+   break;
+
+   foprm = kzalloc(sizeof(*foprm), GFP_KERNEL);
+   if (!foprm)
+   return -ENOMEM;
+
+   foprm-tu = priv;
+