[for-next][PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2014-01-02 Thread Steven Rostedt 
From: Namhyung Kim 

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

Acked-by: Oleg Nesterov 
Acked-by: Masami Hiramatsu 
Cc: Srikar Dronamraju 
Cc: zhangwei(Jovi) 
Cc: Arnaldo Carvalho de Melo 
Signed-off-by: Namhyung Kim 
---
 kernel/trace/trace_kprobe.c | 20 ++
 kernel/trace/trace_probe.c  | 65 ++---
 kernel/trace/trace_probe.h  | 53 
 kernel/trace/trace_uprobe.c | 20 ++
 4 files changed, 119 insertions(+), 39 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index c9ffdaf..fe3f00c 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,26 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
+   sizeof(u32), 1, "__data_loc char[]"),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
+   string_size, sizeof(u32), 0, "u32"),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+
+   ASSIGN_FETCH_TYPE_END
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9e..541036e 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
-   sizeof(u32), 1, "__data_loc char[]"),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
-   string_size, sizeof(u32), 0, "u32"),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16, 0),
-   ASSIGN_FETCH_TYPE(u32, u32, 0),
-   ASSIGN_FETCH_TYPE(u64, u64, 0),
-   ASSIGN_FETCH_TYPE(s8,  u8,  1),
-   ASSIGN_FETCH_TYPE(s16, u16, 1),
-   ASSIGN_FETCH_TYPE(s32, u32, 1),
-   ASSIGN_FETCH_TYPE(s64, u64, 1),
-};
-
-static const struct fetch_type *find_fetch_type(const char *type)
+static const struct fetch_type *find_fetch_type(const char *type,
+   const struct fetch_type *ftbl)
 {
int i;
 
@@ -398,21 +376,22 @@ static const struct fetch_type *find_fetch_type(const 
char *type)
 
switch (bs) {
case 8:
-   return find_fetch_type("u8");
+   return find_fetch_type("u8", ftbl);
case 16:
-   return find_fetch_type("u16");
+   return find_fetch_type("u16", ftbl);
case 32:
-   return find_fetch_type("u32");
+   return find_fetch_type("u32", ftbl);
case 64:
-   return find_fetch_type("u64");
+   return find_fetch_type("u64", ftbl);
default:
goto fail;
}
}
 
-   for (i = 0; i <

[for-next][PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2014-01-02 Thread Steven Rostedt 
From: Namhyung Kim namhyung@lge.com

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

Acked-by: Oleg Nesterov o...@redhat.com
Acked-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Srikar Dronamraju sri...@linux.vnet.ibm.com
Cc: zhangwei(Jovi) jovi.zhang...@huawei.com
Cc: Arnaldo Carvalho de Melo a...@ghostprotocols.net
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
 kernel/trace/trace_kprobe.c | 20 ++
 kernel/trace/trace_probe.c  | 65 ++---
 kernel/trace/trace_probe.h  | 53 
 kernel/trace/trace_uprobe.c | 20 ++
 4 files changed, 119 insertions(+), 39 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index c9ffdaf..fe3f00c 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,26 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
+   sizeof(u32), 1, __data_loc char[]),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
+   string_size, sizeof(u32), 0, u32),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+
+   ASSIGN_FETCH_TYPE_END
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9e..541036e 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, %Ld)
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = \\\%s\\\;
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
-   sizeof(u32), 1, __data_loc char[]),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
-   string_size, sizeof(u32), 0, u32),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16, 0),
-   ASSIGN_FETCH_TYPE(u32, u32, 0),
-   ASSIGN_FETCH_TYPE(u64, u64, 0),
-   ASSIGN_FETCH_TYPE(s8,  u8,  1),
-   ASSIGN_FETCH_TYPE(s16, u16, 1),
-   ASSIGN_FETCH_TYPE(s32, u32, 1),
-   ASSIGN_FETCH_TYPE(s64, u64, 1),
-};
-
-static const struct fetch_type *find_fetch_type(const char *type)
+static const struct fetch_type *find_fetch_type(const char *type,
+   const struct fetch_type *ftbl)
 {
int i;
 
@@ -398,21 +376,22 @@ static const struct fetch_type *find_fetch_type(const 
char *type)
 
switch (bs) {
case 8:
-   return find_fetch_type(u8);
+   return find_fetch_type(u8, ftbl);
case 16:
-   return find_fetch_type(u16);
+   return find_fetch_type(u16, ftbl);
case 32:
-   return find_fetch_type(u32);
+   return find_fetch_type(u32, ftbl);
case 64:
-   return find_fetch_type(u64);
+   return find_fetch_type(u64, ftbl);
default:

[PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-15 Thread Namhyung Kim 
From: Namhyung Kim 

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

Acked-by: Masami Hiramatsu 
Cc: Srikar Dronamraju 
Cc: Oleg Nesterov 
Cc: zhangwei(Jovi) 
Cc: Arnaldo Carvalho de Melo 
Signed-off-by: Namhyung Kim 
---
 kernel/trace/trace_kprobe.c | 20 ++
 kernel/trace/trace_probe.c  | 65 ++---
 kernel/trace/trace_probe.h  | 53 
 kernel/trace/trace_uprobe.c | 20 ++
 4 files changed, 119 insertions(+), 39 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 62d6c961bbce..23c6c3feff82 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,26 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
+   sizeof(u32), 1, "__data_loc char[]"),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
+   string_size, sizeof(u32), 0, "u32"),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+
+   ASSIGN_FETCH_TYPE_END
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9eaa2ac..541036ec7392 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
-   sizeof(u32), 1, "__data_loc char[]"),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
-   string_size, sizeof(u32), 0, "u32"),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16, 0),
-   ASSIGN_FETCH_TYPE(u32, u32, 0),
-   ASSIGN_FETCH_TYPE(u64, u64, 0),
-   ASSIGN_FETCH_TYPE(s8,  u8,  1),
-   ASSIGN_FETCH_TYPE(s16, u16, 1),
-   ASSIGN_FETCH_TYPE(s32, u32, 1),
-   ASSIGN_FETCH_TYPE(s64, u64, 1),
-};
-
-static const struct fetch_type *find_fetch_type(const char *type)
+static const struct fetch_type *find_fetch_type(const char *type,
+   const struct fetch_type *ftbl)
 {
int i;
 
@@ -398,21 +376,22 @@ static const struct fetch_type *find_fetch_type(const 
char *type)
 
switch (bs) {
case 8:
-   return find_fetch_type("u8");
+   return find_fetch_type("u8", ftbl);
case 16:
-   return find_fetch_type("u16");
+   return find_fetch_type("u16", ftbl);
case 32:
-   return find_fetch_type("u32");
+   return find_fetch_type("u32", ftbl);
case 64:
-   return find_fetch_type("u64");
+   return find_fetch_type("u64", ftbl);
default:
goto fail;
}
}
 
-   for (i = 0; i <

[PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-15 Thread Namhyung Kim 
From: Namhyung Kim namhyung@lge.com

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

Acked-by: 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
---
 kernel/trace/trace_kprobe.c | 20 ++
 kernel/trace/trace_probe.c  | 65 ++---
 kernel/trace/trace_probe.h  | 53 
 kernel/trace/trace_uprobe.c | 20 ++
 4 files changed, 119 insertions(+), 39 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 62d6c961bbce..23c6c3feff82 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,26 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
+   sizeof(u32), 1, __data_loc char[]),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
+   string_size, sizeof(u32), 0, u32),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+
+   ASSIGN_FETCH_TYPE_END
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9eaa2ac..541036ec7392 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, %Ld)
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = \\\%s\\\;
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
-   sizeof(u32), 1, __data_loc char[]),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
-   string_size, sizeof(u32), 0, u32),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16, 0),
-   ASSIGN_FETCH_TYPE(u32, u32, 0),
-   ASSIGN_FETCH_TYPE(u64, u64, 0),
-   ASSIGN_FETCH_TYPE(s8,  u8,  1),
-   ASSIGN_FETCH_TYPE(s16, u16, 1),
-   ASSIGN_FETCH_TYPE(s32, u32, 1),
-   ASSIGN_FETCH_TYPE(s64, u64, 1),
-};
-
-static const struct fetch_type *find_fetch_type(const char *type)
+static const struct fetch_type *find_fetch_type(const char *type,
+   const struct fetch_type *ftbl)
 {
int i;
 
@@ -398,21 +376,22 @@ static const struct fetch_type *find_fetch_type(const 
char *type)
 
switch (bs) {
case 8:
-   return find_fetch_type(u8);
+   return find_fetch_type(u8, ftbl);
case 16:
-   return find_fetch_type(u16);
+   return find_fetch_type(u16, ftbl);
case 32:
-   return find_fetch_type(u32);
+   return find_fetch_type(u32, ftbl);
case 64:
-   return find_fetch_type(u64);
+   return find_fetch_type(u64, ftbl);

Re: [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-10 Thread Masami Hiramatsu 
(2013/12/10 13:41), Namhyung Kim wrote:
>>From 9f7e24f9d97440a015d7fa6562bce462fcf1f230 Mon Sep 17 00:00:00 2001
> From: Namhyung Kim 
> Date: Tue, 26 Nov 2013 14:56:28 +0900
> Subject: [PATCH v8.1 08/17] tracing/probes: Split [ku]probes_fetch_type_table
> 
> Use separate fetch_type_table for kprobes and uprobes.  It currently
> shares all fetch methods but some of them will be implemented
> differently later.
> 
> This is not to break build if [ku]probes is configured alone (like
> !CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
> to the table declaration so that it can be safely omitted when it
> configured out.
> 
> Cc: Masami Hiramatsu 
> Cc: Srikar Dronamraju 
> Cc: Oleg Nesterov 
> Cc: zhangwei(Jovi) 
> Cc: Arnaldo Carvalho de Melo 
> Signed-off-by: Namhyung Kim 

This looks OK for me ;)

Acked-by: Masami Hiramatsu 

Thank you!

> ---
>  kernel/trace/trace_kprobe.c | 20 
>  kernel/trace/trace_probe.c  | 65 --
>  kernel/trace/trace_probe.h  | 76 
> ++---
>  kernel/trace/trace_uprobe.c | 20 
>  4 files changed, 131 insertions(+), 50 deletions(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 62d6c961bbce..23c6c3feff82 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -88,6 +88,26 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
> pt_regs *regs);
>  static int kretprobe_dispatcher(struct kretprobe_instance *ri,
>   struct pt_regs *regs);
>  
> +/* Fetch type information table */
> +const struct fetch_type kprobes_fetch_type_table[] = {
> + /* Special types */
> + [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
> + sizeof(u32), 1, "__data_loc char[]"),
> + [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
> + string_size, sizeof(u32), 0, "u32"),
> + /* Basic types */
> + ASSIGN_FETCH_TYPE(u8,  u8,  0),
> + ASSIGN_FETCH_TYPE(u16, u16, 0),
> + ASSIGN_FETCH_TYPE(u32, u32, 0),
> + ASSIGN_FETCH_TYPE(u64, u64, 0),
> + ASSIGN_FETCH_TYPE(s8,  u8,  1),
> + ASSIGN_FETCH_TYPE(s16, u16, 1),
> + ASSIGN_FETCH_TYPE(s32, u32, 1),
> + ASSIGN_FETCH_TYPE(s64, u64, 1),
> +
> + ASSIGN_FETCH_TYPE_END
> +};
> +
>  /*
>   * Allocate new trace_probe and initialize it (including kprobes).
>   */
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index c26bc9eaa2ac..541036ec7392 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
>  DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
>  DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
>  
> -/* For defining macros, define string/string_size types */
> -typedef u32 string;
> -typedef u32 string_size;
> -
>  /* Print type function for string type */
>  __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
> const char *name,
> @@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
> *s,
>  
>  const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";
>  
> -#define FETCH_FUNC_NAME(method, type)fetch_##method##_##type
>  /*
>   * Define macro for basic types - we don't need to define s* types, because
>   * we have to care only about bitwidth at recording time.
> @@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
> *data)
>   kfree(data);
>  }
>  
> -/* Fetch type information table */
> -static const struct fetch_type fetch_type_table[] = {
> - /* Special types */
> - [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
> - sizeof(u32), 1, "__data_loc char[]"),
> - [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
> - string_size, sizeof(u32), 0, "u32"),
> - /* Basic types */
> - ASSIGN_FETCH_TYPE(u8,  u8,  0),
> - ASSIGN_FETCH_TYPE(u16, u16, 0),
> - ASSIGN_FETCH_TYPE(u32, u32, 0),
> - ASSIGN_FETCH_TYPE(u64, u64, 0),
> - ASSIGN_FETCH_TYPE(s8,  u8,  1),
> - ASSIGN_FETCH_TYPE(s16, u16, 1),
> - ASSIGN_FETCH_TYPE(s32, u32, 1),
> - ASSIGN_FETCH_TYPE(s64, u64, 1),
> -};
> -
> -static const struct fetch_type *find_fetch_type(const char *type)
> +static const struct fetch_type *find_fetch_type(const char *type,
> + const struct fetch_type *ftbl)
>  {
>   int i;
>  
> @@ -398,21 +376,22 @@ static const struct fetch_type *find_fetch_type(const 
> char *type)
>  
>   switch (bs) {
>   case 8:
> - return find_fetch_type("u8");
> + return find_fetch_type("u8", ftbl);
>   case 16:
> - return find_fetch_type("u16");
> +

Re: [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-10 Thread Masami Hiramatsu 
(2013/12/10 13:41), Namhyung Kim wrote:
From 9f7e24f9d97440a015d7fa6562bce462fcf1f230 Mon Sep 17 00:00:00 2001
 From: Namhyung Kim namhyung@lge.com
 Date: Tue, 26 Nov 2013 14:56:28 +0900
 Subject: [PATCH v8.1 08/17] tracing/probes: Split [ku]probes_fetch_type_table
 
 Use separate fetch_type_table for kprobes and uprobes.  It currently
 shares all fetch methods but some of them will be implemented
 differently later.
 
 This is not to break build if [ku]probes is configured alone (like
 !CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
 to the table declaration so that it can be safely omitted when it
 configured out.
 
 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

This looks OK for me ;)

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

Thank you!

 ---
  kernel/trace/trace_kprobe.c | 20 
  kernel/trace/trace_probe.c  | 65 --
  kernel/trace/trace_probe.h  | 76 
 ++---
  kernel/trace/trace_uprobe.c | 20 
  4 files changed, 131 insertions(+), 50 deletions(-)
 
 diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
 index 62d6c961bbce..23c6c3feff82 100644
 --- a/kernel/trace/trace_kprobe.c
 +++ b/kernel/trace/trace_kprobe.c
 @@ -88,6 +88,26 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
 pt_regs *regs);
  static int kretprobe_dispatcher(struct kretprobe_instance *ri,
   struct pt_regs *regs);
  
 +/* Fetch type information table */
 +const struct fetch_type kprobes_fetch_type_table[] = {
 + /* Special types */
 + [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
 + sizeof(u32), 1, __data_loc char[]),
 + [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
 + string_size, sizeof(u32), 0, u32),
 + /* Basic types */
 + ASSIGN_FETCH_TYPE(u8,  u8,  0),
 + ASSIGN_FETCH_TYPE(u16, u16, 0),
 + ASSIGN_FETCH_TYPE(u32, u32, 0),
 + ASSIGN_FETCH_TYPE(u64, u64, 0),
 + ASSIGN_FETCH_TYPE(s8,  u8,  1),
 + ASSIGN_FETCH_TYPE(s16, u16, 1),
 + ASSIGN_FETCH_TYPE(s32, u32, 1),
 + ASSIGN_FETCH_TYPE(s64, u64, 1),
 +
 + ASSIGN_FETCH_TYPE_END
 +};
 +
  /*
   * Allocate new trace_probe and initialize it (including kprobes).
   */
 diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
 index c26bc9eaa2ac..541036ec7392 100644
 --- a/kernel/trace/trace_probe.c
 +++ b/kernel/trace/trace_probe.c
 @@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, %d)
  DEFINE_BASIC_PRINT_TYPE_FUNC(s32, %d)
  DEFINE_BASIC_PRINT_TYPE_FUNC(s64, %Ld)
  
 -/* For defining macros, define string/string_size types */
 -typedef u32 string;
 -typedef u32 string_size;
 -
  /* Print type function for string type */
  __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
 const char *name,
 @@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
 *s,
  
  const char PRINT_TYPE_FMT_NAME(string)[] = \\\%s\\\;
  
 -#define FETCH_FUNC_NAME(method, type)fetch_##method##_##type
  /*
   * Define macro for basic types - we don't need to define s* types, because
   * we have to care only about bitwidth at recording time.
 @@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
 *data)
   kfree(data);
  }
  
 -/* Fetch type information table */
 -static const struct fetch_type fetch_type_table[] = {
 - /* Special types */
 - [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
 - sizeof(u32), 1, __data_loc char[]),
 - [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
 - string_size, sizeof(u32), 0, u32),
 - /* Basic types */
 - ASSIGN_FETCH_TYPE(u8,  u8,  0),
 - ASSIGN_FETCH_TYPE(u16, u16, 0),
 - ASSIGN_FETCH_TYPE(u32, u32, 0),
 - ASSIGN_FETCH_TYPE(u64, u64, 0),
 - ASSIGN_FETCH_TYPE(s8,  u8,  1),
 - ASSIGN_FETCH_TYPE(s16, u16, 1),
 - ASSIGN_FETCH_TYPE(s32, u32, 1),
 - ASSIGN_FETCH_TYPE(s64, u64, 1),
 -};
 -
 -static const struct fetch_type *find_fetch_type(const char *type)
 +static const struct fetch_type *find_fetch_type(const char *type,
 + const struct fetch_type *ftbl)
  {
   int i;
  
 @@ -398,21 +376,22 @@ static const struct fetch_type *find_fetch_type(const 
 char *type)
  
   switch (bs) {
   case 8:
 - return find_fetch_type(u8);
 + return find_fetch_type(u8, ftbl);
   case 16:
 - return

Re: [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-09 Thread Namhyung Kim 
On Tue, 10 Dec 2013 10:05:40 +0900, Namhyung Kim wrote:
> On Tue, 10 Dec 2013 00:09:36 +0900, Masami Hiramatsu wrote:
>> (2013/12/09 15:19), Namhyung Kim wrote:
>>> @@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
>>> char *type)
>>> -   for (i = 0; i < ARRAY_SIZE(fetch_type_table); i++)
>>> -   if (strcmp(type, fetch_type_table[i].name) == 0)
>>> -   return _type_table[i];
>>> +   for (i = 0; i < NR_FETCH_TYPES; i++)
>>
>> Hmm, I consider this should use correct length of given array. Thus,
>> I'd like recommend you to do;
>> giving the size of fetch_type_table, or define *probe_fetch_type_table with
>> NR_FETCH_TYPES size, or introduce a gatekeeper, e.g. special terminator entry
>> for initializing.
>
> Ah, okay.  So I'd like to go with the gatekeeper approach since I added
> the NR_FETCH_TYPES only for iterating the table so no need to keep the
> size itself.  And it might be better for additional change in the
> future.

So here is the new patch:


>From 9f7e24f9d97440a015d7fa6562bce462fcf1f230 Mon Sep 17 00:00:00 2001
From: Namhyung Kim 
Date: Tue, 26 Nov 2013 14:56:28 +0900
Subject: [PATCH v8.1 08/17] tracing/probes: Split [ku]probes_fetch_type_table

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

Cc: Masami Hiramatsu 
Cc: Srikar Dronamraju 
Cc: Oleg Nesterov 
Cc: zhangwei(Jovi) 
Cc: Arnaldo Carvalho de Melo 
Signed-off-by: Namhyung Kim 
---
 kernel/trace/trace_kprobe.c | 20 
 kernel/trace/trace_probe.c  | 65 --
 kernel/trace/trace_probe.h  | 76 ++---
 kernel/trace/trace_uprobe.c | 20 
 4 files changed, 131 insertions(+), 50 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 62d6c961bbce..23c6c3feff82 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,26 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
+   sizeof(u32), 1, "__data_loc char[]"),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
+   string_size, sizeof(u32), 0, "u32"),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+
+   ASSIGN_FETCH_TYPE_END
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9eaa2ac..541036ec7392 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
-   sizeof(u32), 1, "__data_loc char[]"),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
-   string_size, sizeof(u32), 0, "u32"),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16, 0),
-   ASSIGN_FETCH_TYPE(u32, u32, 0),
-   ASSIGN_FETCH_TYPE(u64, u64, 0),
-

Re: [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-09 Thread Namhyung Kim 
Hi Masami,

On Tue, 10 Dec 2013 00:09:36 +0900, Masami Hiramatsu wrote:
> (2013/12/09 15:19), Namhyung Kim wrote:
>> @@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
>> char *type)
>>  
>>  switch (bs) {
>>  case 8:
>> -return find_fetch_type("u8");
>> +return find_fetch_type("u8", ftbl);
>>  case 16:
>> -return find_fetch_type("u16");
>> +return find_fetch_type("u16", ftbl);
>>  case 32:
>> -return find_fetch_type("u32");
>> +return find_fetch_type("u32", ftbl);
>>  case 64:
>> -return find_fetch_type("u64");
>> +return find_fetch_type("u64", ftbl);
>>  default:
>>  goto fail;
>>  }
>>  }
>>  
>> -for (i = 0; i < ARRAY_SIZE(fetch_type_table); i++)
>> -if (strcmp(type, fetch_type_table[i].name) == 0)
>> -return _type_table[i];
>> +for (i = 0; i < NR_FETCH_TYPES; i++)
>
> Hmm, I consider this should use correct length of given array. Thus,
> I'd like recommend you to do;
> giving the size of fetch_type_table, or define *probe_fetch_type_table with
> NR_FETCH_TYPES size, or introduce a gatekeeper, e.g. special terminator entry
> for initializing.

Ah, okay.  So I'd like to go with the gatekeeper approach since I added
the NR_FETCH_TYPES only for iterating the table so no need to keep the
size itself.  And it might be better for additional change in the
future.

Thanks,
Namhyung

>
>> +/* Fetch type information table */
>> +const struct fetch_type uprobes_fetch_type_table[] = {
>
> For example, here if I see the NR_FETCH_TYPES in above [], I can ensure
> the table has at least that size (and if there is a space, it will be
> filled with zero, in that case, we just need to add a checking
> fetch_type_table[i].name != NULL).
>
> Thank you,
--
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 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-09 Thread Masami Hiramatsu 
(2013/12/09 15:19), Namhyung Kim wrote:
> @@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
> char *type)
>  
>   switch (bs) {
>   case 8:
> - return find_fetch_type("u8");
> + return find_fetch_type("u8", ftbl);
>   case 16:
> - return find_fetch_type("u16");
> + return find_fetch_type("u16", ftbl);
>   case 32:
> - return find_fetch_type("u32");
> + return find_fetch_type("u32", ftbl);
>   case 64:
> - return find_fetch_type("u64");
> + return find_fetch_type("u64", ftbl);
>   default:
>   goto fail;
>   }
>   }
>  
> - for (i = 0; i < ARRAY_SIZE(fetch_type_table); i++)
> - if (strcmp(type, fetch_type_table[i].name) == 0)
> - return _type_table[i];
> + for (i = 0; i < NR_FETCH_TYPES; i++)

Hmm, I consider this should use correct length of given array. Thus,
I'd like recommend you to do;
giving the size of fetch_type_table, or define *probe_fetch_type_table with
NR_FETCH_TYPES size, or introduce a gatekeeper, e.g. special terminator entry
for initializing.

> +/* Fetch type information table */
> +const struct fetch_type uprobes_fetch_type_table[] = {

For example, here if I see the NR_FETCH_TYPES in above [], I can ensure
the table has at least that size (and if there is a space, it will be
filled with zero, in that case, we just need to add a checking
fetch_type_table[i].name != NULL).

Thank you,

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


--
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 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-09 Thread Masami Hiramatsu 
(2013/12/09 15:19), Namhyung Kim wrote:
 @@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
 char *type)
  
   switch (bs) {
   case 8:
 - return find_fetch_type(u8);
 + return find_fetch_type(u8, ftbl);
   case 16:
 - return find_fetch_type(u16);
 + return find_fetch_type(u16, ftbl);
   case 32:
 - return find_fetch_type(u32);
 + return find_fetch_type(u32, ftbl);
   case 64:
 - return find_fetch_type(u64);
 + return find_fetch_type(u64, ftbl);
   default:
   goto fail;
   }
   }
  
 - for (i = 0; i  ARRAY_SIZE(fetch_type_table); i++)
 - if (strcmp(type, fetch_type_table[i].name) == 0)
 - return fetch_type_table[i];
 + for (i = 0; i  NR_FETCH_TYPES; i++)

Hmm, I consider this should use correct length of given array. Thus,
I'd like recommend you to do;
giving the size of fetch_type_table, or define *probe_fetch_type_table with
NR_FETCH_TYPES size, or introduce a gatekeeper, e.g. special terminator entry
for initializing.

 +/* Fetch type information table */
 +const struct fetch_type uprobes_fetch_type_table[] = {

For example, here if I see the NR_FETCH_TYPES in above [], I can ensure
the table has at least that size (and if there is a space, it will be
filled with zero, in that case, we just need to add a checking
fetch_type_table[i].name != NULL).

Thank you,

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


--
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 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-09 Thread Namhyung Kim 
Hi Masami,

On Tue, 10 Dec 2013 00:09:36 +0900, Masami Hiramatsu wrote:
 (2013/12/09 15:19), Namhyung Kim wrote:
 @@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
 char *type)
  
  switch (bs) {
  case 8:
 -return find_fetch_type(u8);
 +return find_fetch_type(u8, ftbl);
  case 16:
 -return find_fetch_type(u16);
 +return find_fetch_type(u16, ftbl);
  case 32:
 -return find_fetch_type(u32);
 +return find_fetch_type(u32, ftbl);
  case 64:
 -return find_fetch_type(u64);
 +return find_fetch_type(u64, ftbl);
  default:
  goto fail;
  }
  }
  
 -for (i = 0; i  ARRAY_SIZE(fetch_type_table); i++)
 -if (strcmp(type, fetch_type_table[i].name) == 0)
 -return fetch_type_table[i];
 +for (i = 0; i  NR_FETCH_TYPES; i++)

 Hmm, I consider this should use correct length of given array. Thus,
 I'd like recommend you to do;
 giving the size of fetch_type_table, or define *probe_fetch_type_table with
 NR_FETCH_TYPES size, or introduce a gatekeeper, e.g. special terminator entry
 for initializing.

Ah, okay.  So I'd like to go with the gatekeeper approach since I added
the NR_FETCH_TYPES only for iterating the table so no need to keep the
size itself.  And it might be better for additional change in the
future.

Thanks,
Namhyung


 +/* Fetch type information table */
 +const struct fetch_type uprobes_fetch_type_table[] = {

 For example, here if I see the NR_FETCH_TYPES in above [], I can ensure
 the table has at least that size (and if there is a space, it will be
 filled with zero, in that case, we just need to add a checking
 fetch_type_table[i].name != NULL).

 Thank you,
--
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 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-09 Thread Namhyung Kim 
On Tue, 10 Dec 2013 10:05:40 +0900, Namhyung Kim wrote:
 On Tue, 10 Dec 2013 00:09:36 +0900, Masami Hiramatsu wrote:
 (2013/12/09 15:19), Namhyung Kim wrote:
 @@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
 char *type)
 -   for (i = 0; i  ARRAY_SIZE(fetch_type_table); i++)
 -   if (strcmp(type, fetch_type_table[i].name) == 0)
 -   return fetch_type_table[i];
 +   for (i = 0; i  NR_FETCH_TYPES; i++)

 Hmm, I consider this should use correct length of given array. Thus,
 I'd like recommend you to do;
 giving the size of fetch_type_table, or define *probe_fetch_type_table with
 NR_FETCH_TYPES size, or introduce a gatekeeper, e.g. special terminator entry
 for initializing.

 Ah, okay.  So I'd like to go with the gatekeeper approach since I added
 the NR_FETCH_TYPES only for iterating the table so no need to keep the
 size itself.  And it might be better for additional change in the
 future.

So here is the new patch:


From 9f7e24f9d97440a015d7fa6562bce462fcf1f230 Mon Sep 17 00:00:00 2001
From: Namhyung Kim namhyung@lge.com
Date: Tue, 26 Nov 2013 14:56:28 +0900
Subject: [PATCH v8.1 08/17] tracing/probes: Split [ku]probes_fetch_type_table

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

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
---
 kernel/trace/trace_kprobe.c | 20 
 kernel/trace/trace_probe.c  | 65 --
 kernel/trace/trace_probe.h  | 76 ++---
 kernel/trace/trace_uprobe.c | 20 
 4 files changed, 131 insertions(+), 50 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 62d6c961bbce..23c6c3feff82 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,26 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
+   sizeof(u32), 1, __data_loc char[]),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
+   string_size, sizeof(u32), 0, u32),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+
+   ASSIGN_FETCH_TYPE_END
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9eaa2ac..541036ec7392 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, %Ld)
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = \\\%s\\\;
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
-   sizeof(u32), 1, __data_loc char[]),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
-   string_size, sizeof(u32), 0, u32),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16,

[PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-08 Thread Namhyung Kim 
From: Namhyung Kim 

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

Cc: Masami Hiramatsu 
Cc: Srikar Dronamraju 
Cc: Oleg Nesterov 
Cc: zhangwei(Jovi) 
Cc: Arnaldo Carvalho de Melo 
Signed-off-by: Namhyung Kim 
---
 kernel/trace/trace_kprobe.c | 18 +++
 kernel/trace/trace_probe.c  | 64 +++---
 kernel/trace/trace_probe.h  | 76 ++---
 kernel/trace/trace_uprobe.c | 18 +++
 4 files changed, 126 insertions(+), 50 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 62d6c961bbce..d00ee5ce6ccc 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,24 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
+   sizeof(u32), 1, "__data_loc char[]"),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
+   string_size, sizeof(u32), 0, "u32"),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9eaa2ac..68b00a214fcc 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
-   sizeof(u32), 1, "__data_loc char[]"),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
-   string_size, sizeof(u32), 0, "u32"),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16, 0),
-   ASSIGN_FETCH_TYPE(u32, u32, 0),
-   ASSIGN_FETCH_TYPE(u64, u64, 0),
-   ASSIGN_FETCH_TYPE(s8,  u8,  1),
-   ASSIGN_FETCH_TYPE(s16, u16, 1),
-   ASSIGN_FETCH_TYPE(s32, u32, 1),
-   ASSIGN_FETCH_TYPE(s64, u64, 1),
-};
-
-static const struct fetch_type *find_fetch_type(const char *type)
+static const struct fetch_type *find_fetch_type(const char *type,
+   const struct fetch_type *ftbl)
 {
int i;
 
@@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
char *type)
 
switch (bs) {
case 8:
-   return find_fetch_type("u8");
+   return find_fetch_type("u8", ftbl);
case 16:
-   return find_fetch_type("u16");
+   return find_fetch_type("u16", ftbl);
case 32:
-   return find_fetch_type("u32");
+   return find_fetch_type("u32", ftbl);
case 64:
-   return find_fetch_type("u64");
+   return find_fetch_type("u64", ftbl);
default:
goto fail;
}
}
 
-   for (i = 0; i < ARRAY_SIZE(fetch_type_table); i++)
-   if

[PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-08 Thread Namhyung Kim 
From: Namhyung Kim namhyung@lge.com

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

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
---
 kernel/trace/trace_kprobe.c | 18 +++
 kernel/trace/trace_probe.c  | 64 +++---
 kernel/trace/trace_probe.h  | 76 ++---
 kernel/trace/trace_uprobe.c | 18 +++
 4 files changed, 126 insertions(+), 50 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 62d6c961bbce..d00ee5ce6ccc 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,24 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
+   sizeof(u32), 1, __data_loc char[]),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
+   string_size, sizeof(u32), 0, u32),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9eaa2ac..68b00a214fcc 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, %Ld)
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = \\\%s\\\;
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
-   sizeof(u32), 1, __data_loc char[]),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
-   string_size, sizeof(u32), 0, u32),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16, 0),
-   ASSIGN_FETCH_TYPE(u32, u32, 0),
-   ASSIGN_FETCH_TYPE(u64, u64, 0),
-   ASSIGN_FETCH_TYPE(s8,  u8,  1),
-   ASSIGN_FETCH_TYPE(s16, u16, 1),
-   ASSIGN_FETCH_TYPE(s32, u32, 1),
-   ASSIGN_FETCH_TYPE(s64, u64, 1),
-};
-
-static const struct fetch_type *find_fetch_type(const char *type)
+static const struct fetch_type *find_fetch_type(const char *type,
+   const struct fetch_type *ftbl)
 {
int i;
 
@@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
char *type)
 
switch (bs) {
case 8:
-   return find_fetch_type(u8);
+   return find_fetch_type(u8, ftbl);
case 16:
-   return find_fetch_type(u16);
+   return find_fetch_type(u16, ftbl);
case 32:
-   return find_fetch_type(u32);
+   return find_fetch_type(u32, ftbl);
case 64:
-   return find_fetch_type(u64);
+   return find_fetch_type(u64, ftbl);
default:
goto

Re: [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-02 Thread Namhyung Kim 
Hi Oleg,

On Mon, 2 Dec 2013 18:09:52 +0100, Oleg Nesterov wrote:
> On 12/02, Oleg Nesterov wrote:
>>
>> On 11/27, Namhyung Kim wrote:
>> >
>> > Use separate fetch_type_table for kprobes and uprobes.  It currently
>> > shares all fetch methods but some of them will be implemented
>> > differently later.
>>
>> Hmm. This looks wrong, afaics...
>>
>> >  static int parse_probe_arg(char *arg, const struct fetch_type *t,
>> > struct fetch_param *f, bool is_return, bool is_kprobe)
>> >  {
>> > +  const struct fetch_type *ftbl;
>> >unsigned long param;
>> >long offset;
>> >char *tmp;
>> > -  int ret;
>> > +  int ret = 0;
>> >
>> > -  ret = 0;
>> > +  ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
>>
>> OK, but what if, say, CONFIG_KPROBE_EVENT && !CONFIG_UPROBE_EVENT ?
>> The kernel won't compile in this case?
>
> Ah, wait, probably I was wrong. I didn't noticee that this patch
> does
>
>   extern __weak const struct fetch_type kprobes_fetch_type_table[];
>   extern __weak const struct fetch_type uprobes_fetch_type_table[];
>
> Is it the reason for "weak" ?

Exactly!

>
> If yes, perhaps this deserves a comment or at least a note in the changelog.
> Or is there another reason?

Nope.  I'll add a comment and a note in the changelog too.  Please see the
new version below.

>
> I am wondering if this should always work, with any toolchain. I simply
> do not know what is the documented behaviour if a "weak" symbol is never
> defined.

It's something like a weak reference - if it couldn't find a definition
the ref would have value of 0 instead of error.  I'm not sure this is a
standard or documented behavior but it worked for a long time AFAIK so I
guess it's pretty compatible.

Btw I found this:

http://gcc.gnu.org/ml/gcc/1999-02n/msg01219.html

Thanks,
Namhyung


>From 510612b00f3e8c7c1322b3ce10be87296d9bce28 Mon Sep 17 00:00:00 2001
From: Namhyung Kim 
Date: Tue, 26 Nov 2013 14:56:28 +0900
Subject: [PATCH v8 08/17] tracing/probes: Split [ku]probes_fetch_type_table

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

Cc: Masami Hiramatsu 
Cc: Srikar Dronamraju 
Cc: Oleg Nesterov 
Cc: zhangwei(Jovi) 
Cc: Arnaldo Carvalho de Melo 
Signed-off-by: Namhyung Kim 
---
 kernel/trace/trace_kprobe.c | 18 +++
 kernel/trace/trace_probe.c  | 64 +++---
 kernel/trace/trace_probe.h  | 76 ++---
 kernel/trace/trace_uprobe.c | 18 +++
 4 files changed, 126 insertions(+), 50 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 62d6c961bbce..d00ee5ce6ccc 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,24 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
+   sizeof(u32), 1, "__data_loc char[]"),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
+   string_size, sizeof(u32), 0, "u32"),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9eaa2ac..68b00a214fcc 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types,

Re: [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-02 Thread Oleg Nesterov 
On 12/02, Oleg Nesterov wrote:
>
> On 11/27, Namhyung Kim wrote:
> >
> > Use separate fetch_type_table for kprobes and uprobes.  It currently
> > shares all fetch methods but some of them will be implemented
> > differently later.
>
> Hmm. This looks wrong, afaics...
>
> >  static int parse_probe_arg(char *arg, const struct fetch_type *t,
> >  struct fetch_param *f, bool is_return, bool is_kprobe)
> >  {
> > +   const struct fetch_type *ftbl;
> > unsigned long param;
> > long offset;
> > char *tmp;
> > -   int ret;
> > +   int ret = 0;
> >
> > -   ret = 0;
> > +   ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
>
> OK, but what if, say, CONFIG_KPROBE_EVENT && !CONFIG_UPROBE_EVENT ?
> The kernel won't compile in this case?

Ah, wait, probably I was wrong. I didn't noticee that this patch
does

extern __weak const struct fetch_type kprobes_fetch_type_table[];
extern __weak const struct fetch_type uprobes_fetch_type_table[];

Is it the reason for "weak" ?

If yes, perhaps this deserves a comment or at least a note in the changelog.
Or is there another reason?

I am wondering if this should always work, with any toolchain. I simply
do not know what is the documented behaviour if a "weak" symbol is never
defined.

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 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-02 Thread Oleg Nesterov 
On 11/27, Namhyung Kim wrote:
>
> Use separate fetch_type_table for kprobes and uprobes.  It currently
> shares all fetch methods but some of them will be implemented
> differently later.

Hmm. This looks wrong, afaics...

>  static int parse_probe_arg(char *arg, const struct fetch_type *t,
>struct fetch_param *f, bool is_return, bool is_kprobe)
>  {
> + const struct fetch_type *ftbl;
>   unsigned long param;
>   long offset;
>   char *tmp;
> - int ret;
> + int ret = 0;
>
> - ret = 0;
> + ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;

OK, but what if, say, CONFIG_KPROBE_EVENT && !CONFIG_UPROBE_EVENT ?
The kernel won't compile in this case?

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 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-02 Thread Oleg Nesterov 
On 11/27, Namhyung Kim wrote:

 Use separate fetch_type_table for kprobes and uprobes.  It currently
 shares all fetch methods but some of them will be implemented
 differently later.

Hmm. This looks wrong, afaics...

  static int parse_probe_arg(char *arg, const struct fetch_type *t,
struct fetch_param *f, bool is_return, bool is_kprobe)
  {
 + const struct fetch_type *ftbl;
   unsigned long param;
   long offset;
   char *tmp;
 - int ret;
 + int ret = 0;

 - ret = 0;
 + ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;

OK, but what if, say, CONFIG_KPROBE_EVENT  !CONFIG_UPROBE_EVENT ?
The kernel won't compile in this case?

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 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-02 Thread Oleg Nesterov 
On 12/02, Oleg Nesterov wrote:

 On 11/27, Namhyung Kim wrote:
 
  Use separate fetch_type_table for kprobes and uprobes.  It currently
  shares all fetch methods but some of them will be implemented
  differently later.

 Hmm. This looks wrong, afaics...

   static int parse_probe_arg(char *arg, const struct fetch_type *t,
   struct fetch_param *f, bool is_return, bool is_kprobe)
   {
  +   const struct fetch_type *ftbl;
  unsigned long param;
  long offset;
  char *tmp;
  -   int ret;
  +   int ret = 0;
 
  -   ret = 0;
  +   ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;

 OK, but what if, say, CONFIG_KPROBE_EVENT  !CONFIG_UPROBE_EVENT ?
 The kernel won't compile in this case?

Ah, wait, probably I was wrong. I didn't noticee that this patch
does

extern __weak const struct fetch_type kprobes_fetch_type_table[];
extern __weak const struct fetch_type uprobes_fetch_type_table[];

Is it the reason for weak ?

If yes, perhaps this deserves a comment or at least a note in the changelog.
Or is there another reason?

I am wondering if this should always work, with any toolchain. I simply
do not know what is the documented behaviour if a weak symbol is never
defined.

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 08/17] tracing/probes: Split [ku]probes_fetch_type_table

2013-12-02 Thread Namhyung Kim 
Hi Oleg,

On Mon, 2 Dec 2013 18:09:52 +0100, Oleg Nesterov wrote:
 On 12/02, Oleg Nesterov wrote:

 On 11/27, Namhyung Kim wrote:
 
  Use separate fetch_type_table for kprobes and uprobes.  It currently
  shares all fetch methods but some of them will be implemented
  differently later.

 Hmm. This looks wrong, afaics...

   static int parse_probe_arg(char *arg, const struct fetch_type *t,
  struct fetch_param *f, bool is_return, bool is_kprobe)
   {
  +  const struct fetch_type *ftbl;
 unsigned long param;
 long offset;
 char *tmp;
  -  int ret;
  +  int ret = 0;
 
  -  ret = 0;
  +  ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;

 OK, but what if, say, CONFIG_KPROBE_EVENT  !CONFIG_UPROBE_EVENT ?
 The kernel won't compile in this case?

 Ah, wait, probably I was wrong. I didn't noticee that this patch
 does

   extern __weak const struct fetch_type kprobes_fetch_type_table[];
   extern __weak const struct fetch_type uprobes_fetch_type_table[];

 Is it the reason for weak ?

Exactly!


 If yes, perhaps this deserves a comment or at least a note in the changelog.
 Or is there another reason?

Nope.  I'll add a comment and a note in the changelog too.  Please see the
new version below.


 I am wondering if this should always work, with any toolchain. I simply
 do not know what is the documented behaviour if a weak symbol is never
 defined.

It's something like a weak reference - if it couldn't find a definition
the ref would have value of 0 instead of error.  I'm not sure this is a
standard or documented behavior but it worked for a long time AFAIK so I
guess it's pretty compatible.

Btw I found this:

http://gcc.gnu.org/ml/gcc/1999-02n/msg01219.html

Thanks,
Namhyung


From 510612b00f3e8c7c1322b3ce10be87296d9bce28 Mon Sep 17 00:00:00 2001
From: Namhyung Kim namhyung@lge.com
Date: Tue, 26 Nov 2013 14:56:28 +0900
Subject: [PATCH v8 08/17] tracing/probes: Split [ku]probes_fetch_type_table

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT).  So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

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
---
 kernel/trace/trace_kprobe.c | 18 +++
 kernel/trace/trace_probe.c  | 64 +++---
 kernel/trace/trace_probe.h  | 76 ++---
 kernel/trace/trace_uprobe.c | 18 +++
 4 files changed, 126 insertions(+), 50 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 62d6c961bbce..d00ee5ce6ccc 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,24 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
+   sizeof(u32), 1, __data_loc char[]),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
+   string_size, sizeof(u32), 0, u32),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9eaa2ac..68b00a214fcc 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, %Ld)
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = \\\%s\\\;
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for

[PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

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

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

Cc: Masami Hiramatsu 
Cc: Srikar Dronamraju 
Cc: Oleg Nesterov 
Cc: zhangwei(Jovi) 
Cc: Arnaldo Carvalho de Melo 
Signed-off-by: Namhyung Kim 
---
 kernel/trace/trace_kprobe.c | 18 +++
 kernel/trace/trace_probe.c  | 64 ---
 kernel/trace/trace_probe.h  | 73 ++---
 kernel/trace/trace_uprobe.c | 18 +++
 4 files changed, 123 insertions(+), 50 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index a8129dfa36bb..0b3fccb910b4 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,24 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
+   sizeof(u32), 1, "__data_loc char[]"),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
+   string_size, sizeof(u32), 0, "u32"),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index a34e3fca4071..c76f4a38d50d 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
-   sizeof(u32), 1, "__data_loc char[]"),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
-   string_size, sizeof(u32), 0, "u32"),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16, 0),
-   ASSIGN_FETCH_TYPE(u32, u32, 0),
-   ASSIGN_FETCH_TYPE(u64, u64, 0),
-   ASSIGN_FETCH_TYPE(s8,  u8,  1),
-   ASSIGN_FETCH_TYPE(s16, u16, 1),
-   ASSIGN_FETCH_TYPE(s32, u32, 1),
-   ASSIGN_FETCH_TYPE(s64, u64, 1),
-};
-
-static const struct fetch_type *find_fetch_type(const char *type)
+static const struct fetch_type *find_fetch_type(const char *type,
+   const struct fetch_type *ftbl)
 {
int i;
 
@@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
char *type)
 
switch (bs) {
case 8:
-   return find_fetch_type("u8");
+   return find_fetch_type("u8", ftbl);
case 16:
-   return find_fetch_type("u16");
+   return find_fetch_type("u16", ftbl);
case 32:
-   return find_fetch_type("u32");
+   return find_fetch_type("u32", ftbl);
case 64:
-   return find_fetch_type("u64");
+   return find_fetch_type("u64", ftbl);
default:
goto fail;
}
}
 
-   for (i = 0; i < ARRAY_SIZE(fetch_type_table); i++)
-   if (strcmp(type, fetch_type_table[i].name) == 0)
-   return _type_table[i];
+   for (i = 0; i < NR_FETCH_TYPES; i++)
+   if (strcmp(type, ftbl[i].name) == 0)
+

[PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table

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

Use separate fetch_type_table for kprobes and uprobes.  It currently
shares all fetch methods but some of them will be implemented
differently later.

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
---
 kernel/trace/trace_kprobe.c | 18 +++
 kernel/trace/trace_probe.c  | 64 ---
 kernel/trace/trace_probe.h  | 73 ++---
 kernel/trace/trace_uprobe.c | 18 +++
 4 files changed, 123 insertions(+), 50 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index a8129dfa36bb..0b3fccb910b4 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,24 @@ static int kprobe_dispatcher(struct kprobe *kp, struct 
pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);
 
+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+   /* Special types */
+   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
+   sizeof(u32), 1, __data_loc char[]),
+   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
+   string_size, sizeof(u32), 0, u32),
+   /* Basic types */
+   ASSIGN_FETCH_TYPE(u8,  u8,  0),
+   ASSIGN_FETCH_TYPE(u16, u16, 0),
+   ASSIGN_FETCH_TYPE(u32, u32, 0),
+   ASSIGN_FETCH_TYPE(u64, u64, 0),
+   ASSIGN_FETCH_TYPE(s8,  u8,  1),
+   ASSIGN_FETCH_TYPE(s16, u16, 1),
+   ASSIGN_FETCH_TYPE(s32, u32, 1),
+   ASSIGN_FETCH_TYPE(s64, u64, 1),
+};
+
 /*
  * Allocate new trace_probe and initialize it (including kprobes).
  */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index a34e3fca4071..c76f4a38d50d 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, %d)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, %Ld)
 
-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
 /* Print type function for string type */
 __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
  const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq 
*s,
 
 const char PRINT_TYPE_FMT_NAME(string)[] = \\\%s\\\;
 
-#define FETCH_FUNC_NAME(method, type)  fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param 
*data)
kfree(data);
 }
 
-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
-   /* Special types */
-   [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE(string, string, string,
-   sizeof(u32), 1, __data_loc char[]),
-   [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE(string_size, u32,
-   string_size, sizeof(u32), 0, u32),
-   /* Basic types */
-   ASSIGN_FETCH_TYPE(u8,  u8,  0),
-   ASSIGN_FETCH_TYPE(u16, u16, 0),
-   ASSIGN_FETCH_TYPE(u32, u32, 0),
-   ASSIGN_FETCH_TYPE(u64, u64, 0),
-   ASSIGN_FETCH_TYPE(s8,  u8,  1),
-   ASSIGN_FETCH_TYPE(s16, u16, 1),
-   ASSIGN_FETCH_TYPE(s32, u32, 1),
-   ASSIGN_FETCH_TYPE(s64, u64, 1),
-};
-
-static const struct fetch_type *find_fetch_type(const char *type)
+static const struct fetch_type *find_fetch_type(const char *type,
+   const struct fetch_type *ftbl)
 {
int i;
 
@@ -398,21 +376,21 @@ static const struct fetch_type *find_fetch_type(const 
char *type)
 
switch (bs) {
case 8:
-   return find_fetch_type(u8);
+   return find_fetch_type(u8, ftbl);
case 16:
-   return find_fetch_type(u16);
+   return find_fetch_type(u16, ftbl);
case 32:
-   return find_fetch_type(u32);
+   return find_fetch_type(u32, ftbl);
case 64:
-   return find_fetch_type(u64);
+   return find_fetch_type(u64, ftbl);
default:
goto fail;
}
}
 
-   for (i = 0; i  ARRAY_SIZE(fetch_type_table); i++)
-   if (strcmp(type, fetch_type_table[i].name) == 0)
-   return fetch_type_table[i];
+