From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
Oleg Nesterov <oleg@redhat.com>,
Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
Hyeoncheol Lee <cheol.lee@lge.com>,
"zhangwei(Jovi)" <jovi.zhangwei@huawei.com>,
Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
Hemant Kumar <hkshaw@linux.vnet.ibm.com>,
LKML <linux-kernel@vger.kernel.org>,
Namhyung Kim <namhyung.kim@lge.com>
Subject: Re: [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table
Date: Tue, 10 Dec 2013 18:14:03 +0900 [thread overview]
Message-ID: <52A6DB5B.7000409@hitachi.com> (raw)
In-Reply-To: <87d2l5jp1d.fsf@sejong.aot.lge.com>
(2013/12/10 13:41), Namhyung Kim wrote:
>>From 9f7e24f9d97440a015d7fa6562bce462fcf1f230 Mon Sep 17 00:00:00 2001
> From: Namhyung Kim <namhyung.kim@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.pt@hitachi.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: zhangwei(Jovi) <jovi.zhangwei@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
This looks OK for me ;)
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@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 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; ftbl[i].name; i++) {
> + if (strcmp(type, ftbl[i].name) == 0)
> + return &ftbl[i];
> + }
>
> fail:
> return NULL;
> @@ -426,16 +405,17 @@ static __kprobes void fetch_stack_address(struct pt_regs *regs,
> }
>
> static fetch_func_t get_fetch_size_function(const struct fetch_type *type,
> - fetch_func_t orig_fn)
> + fetch_func_t orig_fn,
> + const struct fetch_type *ftbl)
> {
> int i;
>
> - if (type != &fetch_type_table[FETCH_TYPE_STRING])
> + if (type != &ftbl[FETCH_TYPE_STRING])
> return NULL; /* Only string type needs size function */
>
> for (i = 0; i < FETCH_MTD_END; i++)
> if (type->fetch[i] == orig_fn)
> - return fetch_type_table[FETCH_TYPE_STRSIZE].fetch[i];
> + return ftbl[FETCH_TYPE_STRSIZE].fetch[i];
>
> WARN_ON(1); /* This should not happen */
>
> @@ -504,12 +484,14 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
> 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;
> + BUG_ON(ftbl == NULL);
>
> /* Until uprobe_events supports only reg arguments */
> if (!is_kprobe && arg[0] != '%')
> @@ -568,7 +550,7 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
> struct deref_fetch_param *dprm;
> const struct fetch_type *t2;
>
> - t2 = find_fetch_type(NULL);
> + t2 = find_fetch_type(NULL, ftbl);
> *tmp = '\0';
> dprm = kzalloc(sizeof(struct deref_fetch_param), GFP_KERNEL);
>
> @@ -637,9 +619,13 @@ static int __parse_bitfield_probe_arg(const char *bf,
> int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
> struct probe_arg *parg, bool is_return, bool is_kprobe)
> {
> + const struct fetch_type *ftbl;
> const char *t;
> int ret;
>
> + ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
> + BUG_ON(ftbl == NULL);
> +
> if (strlen(arg) > MAX_ARGSTR_LEN) {
> pr_info("Argument is too long.: %s\n", arg);
> return -ENOSPC;
> @@ -654,7 +640,7 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
> arg[t - parg->comm] = '\0';
> t++;
> }
> - parg->type = find_fetch_type(t);
> + parg->type = find_fetch_type(t, ftbl);
> if (!parg->type) {
> pr_info("Unsupported type: %s\n", t);
> return -EINVAL;
> @@ -668,7 +654,8 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
>
> if (ret >= 0) {
> parg->fetch_size.fn = get_fetch_size_function(parg->type,
> - parg->fetch.fn);
> + parg->fetch.fn,
> + ftbl);
> parg->fetch_size.data = parg->fetch.data;
> }
>
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 7c6be146f444..5b77798d1130 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -126,24 +126,70 @@ struct fetch_param {
> void *data;
> };
>
> +/* For defining macros, define string/string_size types */
> +typedef u32 string;
> +typedef u32 string_size;
> +
> #define PRINT_TYPE_FUNC_NAME(type) print_type_##type
> #define PRINT_TYPE_FMT_NAME(type) print_type_format_##type
>
> /* Printing in basic type function template */
> -#define DECLARE_BASIC_PRINT_TYPE_FUNC(type, fmt) \
> +#define DECLARE_BASIC_PRINT_TYPE_FUNC(type) \
> __kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s, \
> const char *name, \
> void *data, void *ent); \
> -extern const char PRINT_TYPE_FMT_NAME(type)[];
> -
> -DECLARE_BASIC_PRINT_TYPE_FUNC(u8 , "%x")
> -DECLARE_BASIC_PRINT_TYPE_FUNC(u16, "%x")
> -DECLARE_BASIC_PRINT_TYPE_FUNC(u32, "%x")
> -DECLARE_BASIC_PRINT_TYPE_FUNC(u64, "%Lx")
> -DECLARE_BASIC_PRINT_TYPE_FUNC(s8, "%d")
> -DECLARE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
> -DECLARE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
> -DECLARE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")
> +extern const char PRINT_TYPE_FMT_NAME(type)[]
> +
> +DECLARE_BASIC_PRINT_TYPE_FUNC(u8);
> +DECLARE_BASIC_PRINT_TYPE_FUNC(u16);
> +DECLARE_BASIC_PRINT_TYPE_FUNC(u32);
> +DECLARE_BASIC_PRINT_TYPE_FUNC(u64);
> +DECLARE_BASIC_PRINT_TYPE_FUNC(s8);
> +DECLARE_BASIC_PRINT_TYPE_FUNC(s16);
> +DECLARE_BASIC_PRINT_TYPE_FUNC(s32);
> +DECLARE_BASIC_PRINT_TYPE_FUNC(s64);
> +DECLARE_BASIC_PRINT_TYPE_FUNC(string);
> +
> +#define FETCH_FUNC_NAME(method, type) fetch_##method##_##type
> +
> +/* Declare macro for basic types */
> +#define DECLARE_FETCH_FUNC(method, type) \
> +extern void FETCH_FUNC_NAME(method, type)(struct pt_regs *regs, \
> + void *data, void *dest)
> +
> +#define DECLARE_BASIC_FETCH_FUNCS(method) \
> +DECLARE_FETCH_FUNC(method, u8); \
> +DECLARE_FETCH_FUNC(method, u16); \
> +DECLARE_FETCH_FUNC(method, u32); \
> +DECLARE_FETCH_FUNC(method, u64)
> +
> +DECLARE_BASIC_FETCH_FUNCS(reg);
> +#define fetch_reg_string NULL
> +#define fetch_reg_string_size NULL
> +
> +DECLARE_BASIC_FETCH_FUNCS(stack);
> +#define fetch_stack_string NULL
> +#define fetch_stack_string_size NULL
> +
> +DECLARE_BASIC_FETCH_FUNCS(retval);
> +#define fetch_retval_string NULL
> +#define fetch_retval_string_size NULL
> +
> +DECLARE_BASIC_FETCH_FUNCS(memory);
> +DECLARE_FETCH_FUNC(memory, string);
> +DECLARE_FETCH_FUNC(memory, string_size);
> +
> +DECLARE_BASIC_FETCH_FUNCS(symbol);
> +DECLARE_FETCH_FUNC(symbol, string);
> +DECLARE_FETCH_FUNC(symbol, string_size);
> +
> +DECLARE_BASIC_FETCH_FUNCS(deref);
> +DECLARE_FETCH_FUNC(deref, string);
> +DECLARE_FETCH_FUNC(deref, string_size);
> +
> +DECLARE_BASIC_FETCH_FUNCS(bitfield);
> +#define fetch_bitfield_string NULL
> +#define fetch_bitfield_string_size NULL
>
> /* Default (unsigned long) fetch type */
> #define __DEFAULT_FETCH_TYPE(t) u##t
> @@ -175,9 +221,17 @@ ASSIGN_FETCH_FUNC(bitfield, ftype), \
> #define ASSIGN_FETCH_TYPE(ptype, ftype, sign) \
> __ASSIGN_FETCH_TYPE(#ptype, ptype, ftype, sizeof(ftype), sign, #ptype)
>
> +#define ASSIGN_FETCH_TYPE_END {}
> +
> #define FETCH_TYPE_STRING 0
> #define FETCH_TYPE_STRSIZE 1
>
> +/*
> + * Fetch type information table.
> + * It's declared as a weak symbol due to conditional compilation.
> + */
> +extern __weak const struct fetch_type kprobes_fetch_type_table[];
> +extern __weak const struct fetch_type uprobes_fetch_type_table[];
>
> struct probe_arg {
> struct fetch_param fetch;
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index b233d9cb1216..2c60925ea073 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -74,6 +74,26 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs);
> static int uretprobe_dispatcher(struct uprobe_consumer *con,
> unsigned long func, struct pt_regs *regs);
>
> +/* Fetch type information table */
> +const struct fetch_type uprobes_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
> +};
> +
> static inline void init_trace_uprobe_filter(struct trace_uprobe_filter *filter)
> {
> rwlock_init(&filter->rwlock);
>
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
next prev parent reply other threads:[~2013-12-10 9:14 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-09 6:19 [PATCHSET 00/17] tracing/uprobes: Add support for more fetch methods (v8) Namhyung Kim
2013-12-09 6:19 ` [PATCH 01/17] tracing/uprobes: Fix documentation of uprobe registration syntax Namhyung Kim
2013-12-09 6:19 ` [PATCH 02/17] tracing/probes: Fix basic print type functions Namhyung Kim
2013-12-09 6:19 ` [PATCH 03/17] tracing/kprobes: Factor out struct trace_probe Namhyung Kim
2013-12-09 6:19 ` [PATCH 04/17] tracing/uprobes: Convert to " Namhyung Kim
2013-12-09 6:19 ` [PATCH 05/17] tracing/kprobes: Move common functions to trace_probe.h Namhyung Kim
2013-12-09 6:19 ` [PATCH 06/17] tracing/probes: Integrate duplicate set_print_fmt() Namhyung Kim
2013-12-09 6:19 ` [PATCH 07/17] tracing/probes: Move fetch function helpers to trace_probe.h Namhyung Kim
2013-12-09 15:12 ` Masami Hiramatsu
2013-12-09 6:19 ` [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table Namhyung Kim
2013-12-09 15:09 ` Masami Hiramatsu
2013-12-10 1:05 ` Namhyung Kim
2013-12-10 4:41 ` Namhyung Kim
2013-12-10 9:14 ` Masami Hiramatsu [this message]
2013-12-09 6:19 ` [PATCH 09/17] tracing/probes: Implement 'stack' fetch method for uprobes Namhyung Kim
2013-12-09 6:19 ` [PATCH 10/17] tracing/probes: Move 'symbol' fetch method to kprobes Namhyung Kim
2013-12-10 10:12 ` Masami Hiramatsu
2013-12-11 1:05 ` Namhyung Kim
2013-12-11 1:26 ` Masami Hiramatsu
2013-12-09 6:19 ` [PATCH 11/17] tracing/probes: Add fetch{,_size} member into deref fetch method Namhyung Kim
2013-12-09 6:20 ` [PATCH 12/17] tracing/probes: Implement 'memory' fetch method for uprobes Namhyung Kim
2013-12-10 11:00 ` Masami Hiramatsu
2013-12-11 1:15 ` Namhyung Kim
2013-12-11 1:27 ` Masami Hiramatsu
2013-12-09 6:20 ` [PATCH 13/17] tracing/uprobes: Pass 'is_return' to traceprobe_parse_probe_arg() Namhyung Kim
2013-12-09 15:11 ` Masami Hiramatsu
2013-12-09 6:20 ` [PATCH 14/17] tracing/uprobes: Fetch args before reserving a ring buffer Namhyung Kim
2013-12-09 6:20 ` [PATCH 15/17] tracing/uprobes: Add support for full argument access methods Namhyung Kim
2013-12-09 6:20 ` [PATCH 16/17] uprobes: Allocate ->utask before handler_chain() for tracing handlers Namhyung Kim
2013-12-10 10:41 ` Masami Hiramatsu
2013-12-10 15:57 ` Oleg Nesterov
2013-12-11 1:30 ` Namhyung Kim
2013-12-11 1:43 ` Masami Hiramatsu
2013-12-11 18:11 ` Oleg Nesterov
2013-12-12 5:55 ` Masami Hiramatsu
2013-12-12 19:46 ` Oleg Nesterov
2013-12-13 1:57 ` Masami Hiramatsu
2013-12-13 1:58 ` Masami Hiramatsu
2013-12-09 6:20 ` [PATCH 17/17] tracing/uprobes: Add @+file_offset fetch method Namhyung Kim
-- strict thread matches above, loose matches on Subject: below --
2013-12-16 4:31 [PATCHSET 00/17] tracing/uprobes: Add support for more fetch methods (v9) Namhyung Kim
2013-12-16 4:32 ` [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table Namhyung Kim
2013-11-27 6:19 [PATCHSET 00/17] tracing/uprobes: Add support for more fetch methods (v7) Namhyung Kim
2013-11-27 6:19 ` [PATCH 08/17] tracing/probes: Split [ku]probes_fetch_type_table Namhyung Kim
2013-12-02 17:04 ` Oleg Nesterov
2013-12-02 17:09 ` Oleg Nesterov
2013-12-03 2:24 ` Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52A6DB5B.7000409@hitachi.com \
--to=masami.hiramatsu.pt@hitachi.com \
--cc=acme@ghostprotocols.net \
--cc=cheol.lee@lge.com \
--cc=hkshaw@linux.vnet.ibm.com \
--cc=jovi.zhangwei@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=namhyung.kim@lge.com \
--cc=namhyung@kernel.org \
--cc=oleg@redhat.com \
--cc=rostedt@goodmis.org \
--cc=srikar@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).