* [PATCH 0/6] uprobes/tracing: cleanups and minor fixes
@ 2013-01-31 19:17 Oleg Nesterov
2013-01-31 19:18 ` [PATCH 1/6] uprobes/tracing: Fix dentry/mount leak in create_trace_uprobe() Oleg Nesterov
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Oleg Nesterov @ 2013-01-31 19:17 UTC (permalink / raw)
To: Ingo Molnar, Srikar Dronamraju, Steven Rostedt
Cc: Anton Arapov, Frank Eigler, Josh Stone, Masami Hiramatsu,
Suzuki K. Poulose, linux-kernel
Hello.
Cleanups and minor fixes in trace_uprobe.c. This is also preparation
for the next changes.
Please review, I am going to add this to "oleg/misc uprobes/core" tree
unless someone objects.
Oleg.
kernel/trace/trace_probe.h | 1 -
kernel/trace/trace_uprobe.c | 57 ++++++++++++++++---------------------------
2 files changed, 21 insertions(+), 37 deletions(-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/6] uprobes/tracing: Fix dentry/mount leak in create_trace_uprobe()
2013-01-31 19:17 [PATCH 0/6] uprobes/tracing: cleanups and minor fixes Oleg Nesterov
@ 2013-01-31 19:18 ` Oleg Nesterov
2013-01-31 19:18 ` [PATCH 2/6] uprobes/tracing: Fully initialize uprobe_trace_consumer before uprobe_register() Oleg Nesterov
` (4 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Oleg Nesterov @ 2013-01-31 19:18 UTC (permalink / raw)
To: Ingo Molnar, Srikar Dronamraju, Steven Rostedt
Cc: Anton Arapov, Frank Eigler, Josh Stone, Masami Hiramatsu,
Suzuki K. Poulose, linux-kernel
create_trace_uprobe() does kern_path() to find ->d_inode, but forgets
to do path_put(). We can do this right after igrab().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
kernel/trace/trace_uprobe.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 7d5b407..6563f00 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -253,12 +253,13 @@ static int create_trace_uprobe(int argc, char **argv)
if (ret)
goto fail_address_parse;
+ inode = igrab(path.dentry->d_inode);
+ path_put(&path);
+
ret = kstrtoul(arg, 0, &offset);
if (ret)
goto fail_address_parse;
- inode = igrab(path.dentry->d_inode);
-
argc -= 2;
argv += 2;
--
1.5.5.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/6] uprobes/tracing: Fully initialize uprobe_trace_consumer before uprobe_register()
2013-01-31 19:17 [PATCH 0/6] uprobes/tracing: cleanups and minor fixes Oleg Nesterov
2013-01-31 19:18 ` [PATCH 1/6] uprobes/tracing: Fix dentry/mount leak in create_trace_uprobe() Oleg Nesterov
@ 2013-01-31 19:18 ` Oleg Nesterov
2013-01-31 19:18 ` [PATCH 3/6] uprobes/tracing: Ensure inode != NULL in create_trace_uprobe() Oleg Nesterov
` (3 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Oleg Nesterov @ 2013-01-31 19:18 UTC (permalink / raw)
To: Ingo Molnar, Srikar Dronamraju, Steven Rostedt
Cc: Anton Arapov, Frank Eigler, Josh Stone, Masami Hiramatsu,
Suzuki K. Poulose, linux-kernel
probe_event_enable() does uprobe_register() and only after that sets
utc->tu and tu->consumer/flags. This can race with uprobe_dispatcher()
which can miss these assignments or see them out of order. Nothing
really bad can happen, but this doesn't look clean/safe.
And this does not allow to use uprobe_consumer->filter() we are going
to add, it is called by uprobe_register() and it needs utc->tu.
Change this code to initialize everything before uprobe_register(), and
reset tu->consumer/flags if it fails. We can't race with event_disable(),
the caller holds event_mutex, and if we could the code would be wrong
anyway.
In fact I think uprobe_trace_consumer should die, it buys nothing but
complicates the code. We can simply add uprobe_consumer into trace_uprobe.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
kernel/trace/trace_uprobe.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 6563f00..7b75949 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -547,17 +547,18 @@ static int probe_event_enable(struct trace_uprobe *tu, int flag)
return -EINTR;
utc->cons.handler = uprobe_dispatcher;
+ utc->tu = tu;
+ tu->consumer = utc;
+ tu->flags |= flag;
+
ret = uprobe_register(tu->inode, tu->offset, &utc->cons);
if (ret) {
+ tu->consumer = NULL;
+ tu->flags &= ~flag;
kfree(utc);
- return ret;
}
- tu->flags |= flag;
- utc->tu = tu;
- tu->consumer = utc;
-
- return 0;
+ return ret;
}
static void probe_event_disable(struct trace_uprobe *tu, int flag)
--
1.5.5.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/6] uprobes/tracing: Ensure inode != NULL in create_trace_uprobe()
2013-01-31 19:17 [PATCH 0/6] uprobes/tracing: cleanups and minor fixes Oleg Nesterov
2013-01-31 19:18 ` [PATCH 1/6] uprobes/tracing: Fix dentry/mount leak in create_trace_uprobe() Oleg Nesterov
2013-01-31 19:18 ` [PATCH 2/6] uprobes/tracing: Fully initialize uprobe_trace_consumer before uprobe_register() Oleg Nesterov
@ 2013-01-31 19:18 ` Oleg Nesterov
2013-02-04 10:48 ` Srikar Dronamraju
2013-01-31 19:18 ` [PATCH 4/6] uprobes/tracing: Introduce is_trace_uprobe_enabled() Oleg Nesterov
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Oleg Nesterov @ 2013-01-31 19:18 UTC (permalink / raw)
To: Ingo Molnar, Srikar Dronamraju, Steven Rostedt
Cc: Anton Arapov, Frank Eigler, Josh Stone, Masami Hiramatsu,
Suzuki K. Poulose, linux-kernel
probe_event_enable/disable() check tu->inode != NULL at the start.
This is ugly, if igrab() can fail create_trace_uprobe() should not
succeed and "postpone" the failure.
Note: alloc_uprobe() should probably check igrab() != NULL as well.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/trace/trace_uprobe.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 7b75949..f02bbec 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -255,6 +255,8 @@ static int create_trace_uprobe(int argc, char **argv)
inode = igrab(path.dentry->d_inode);
path_put(&path);
+ if (!inode)
+ goto fail_address_parse;
ret = kstrtoul(arg, 0, &offset);
if (ret)
@@ -539,7 +541,7 @@ static int probe_event_enable(struct trace_uprobe *tu, int flag)
struct uprobe_trace_consumer *utc;
int ret = 0;
- if (!tu->inode || tu->consumer)
+ if (tu->consumer)
return -EINTR;
utc = kzalloc(sizeof(struct uprobe_trace_consumer), GFP_KERNEL);
@@ -563,7 +565,7 @@ static int probe_event_enable(struct trace_uprobe *tu, int flag)
static void probe_event_disable(struct trace_uprobe *tu, int flag)
{
- if (!tu->inode || !tu->consumer)
+ if (!tu->consumer)
return;
uprobe_unregister(tu->inode, tu->offset, &tu->consumer->cons);
--
1.5.5.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/6] uprobes/tracing: Introduce is_trace_uprobe_enabled()
2013-01-31 19:17 [PATCH 0/6] uprobes/tracing: cleanups and minor fixes Oleg Nesterov
` (2 preceding siblings ...)
2013-01-31 19:18 ` [PATCH 3/6] uprobes/tracing: Ensure inode != NULL in create_trace_uprobe() Oleg Nesterov
@ 2013-01-31 19:18 ` Oleg Nesterov
2013-02-04 16:12 ` Srikar Dronamraju
2013-01-31 19:18 ` [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe Oleg Nesterov
2013-01-31 19:18 ` [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit Oleg Nesterov
5 siblings, 1 reply; 18+ messages in thread
From: Oleg Nesterov @ 2013-01-31 19:18 UTC (permalink / raw)
To: Ingo Molnar, Srikar Dronamraju, Steven Rostedt
Cc: Anton Arapov, Frank Eigler, Josh Stone, Masami Hiramatsu,
Suzuki K. Poulose, linux-kernel
probe_event_enable/disable() check tu->consumer != NULL to avoid the
wrong uprobe_register/unregister().
We are going to kill this pointer and "struct uprobe_trace_consumer",
so we add the new helper, is_trace_uprobe_enabled(), which can rely
on TP_FLAG_TRACE/TP_FLAG_PROFILE instead.
Note: the current logic doesn't look optimal, it is not clear why
TP_FLAG_TRACE/TP_FLAG_PROFILE are mutually exclusive, we will probably
change this later.
Also kill the unused TP_FLAG_UPROBE.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/trace/trace_probe.h | 1 -
kernel/trace/trace_uprobe.c | 9 +++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 9337086..5c7e09d 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -66,7 +66,6 @@
#define TP_FLAG_TRACE 1
#define TP_FLAG_PROFILE 2
#define TP_FLAG_REGISTERED 4
-#define TP_FLAG_UPROBE 8
/* data_rloc: data relative location, compatible with u32 */
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index f02bbec..947379a 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -536,12 +536,17 @@ partial:
return TRACE_TYPE_PARTIAL_LINE;
}
+static inline bool is_trace_uprobe_enabled(struct trace_uprobe *tu)
+{
+ return tu->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE);
+}
+
static int probe_event_enable(struct trace_uprobe *tu, int flag)
{
struct uprobe_trace_consumer *utc;
int ret = 0;
- if (tu->consumer)
+ if (is_trace_uprobe_enabled(tu))
return -EINTR;
utc = kzalloc(sizeof(struct uprobe_trace_consumer), GFP_KERNEL);
@@ -565,7 +570,7 @@ static int probe_event_enable(struct trace_uprobe *tu, int flag)
static void probe_event_disable(struct trace_uprobe *tu, int flag)
{
- if (!tu->consumer)
+ if (!is_trace_uprobe_enabled(tu))
return;
uprobe_unregister(tu->inode, tu->offset, &tu->consumer->cons);
--
1.5.5.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe
2013-01-31 19:17 [PATCH 0/6] uprobes/tracing: cleanups and minor fixes Oleg Nesterov
` (3 preceding siblings ...)
2013-01-31 19:18 ` [PATCH 4/6] uprobes/tracing: Introduce is_trace_uprobe_enabled() Oleg Nesterov
@ 2013-01-31 19:18 ` Oleg Nesterov
2013-02-04 16:59 ` Srikar Dronamraju
2013-02-11 9:19 ` Srikar Dronamraju
2013-01-31 19:18 ` [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit Oleg Nesterov
5 siblings, 2 replies; 18+ messages in thread
From: Oleg Nesterov @ 2013-01-31 19:18 UTC (permalink / raw)
To: Ingo Molnar, Srikar Dronamraju, Steven Rostedt
Cc: Anton Arapov, Frank Eigler, Josh Stone, Masami Hiramatsu,
Suzuki K. Poulose, linux-kernel
trace_uprobe->consumer and "struct uprobe_trace_consumer" add the
unnecessary indirection and complicate the code for no reason.
This patch simply embeds uprobe_consumer into "struct trace_uprobe",
all other changes only fix the compilation errors.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/trace/trace_uprobe.c | 35 ++++++-----------------------------
1 files changed, 6 insertions(+), 29 deletions(-)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 947379a..55cdc14 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -31,17 +31,11 @@
/*
* uprobe event core functions
*/
-struct trace_uprobe;
-struct uprobe_trace_consumer {
- struct uprobe_consumer cons;
- struct trace_uprobe *tu;
-};
-
struct trace_uprobe {
struct list_head list;
struct ftrace_event_class class;
struct ftrace_event_call call;
- struct uprobe_trace_consumer *consumer;
+ struct uprobe_consumer consumer;
struct inode *inode;
char *filename;
unsigned long offset;
@@ -92,6 +86,7 @@ alloc_trace_uprobe(const char *group, const char *event, int nargs)
goto error;
INIT_LIST_HEAD(&tu->list);
+ tu->consumer.handler = uprobe_dispatcher;
return tu;
error:
@@ -543,27 +538,15 @@ static inline bool is_trace_uprobe_enabled(struct trace_uprobe *tu)
static int probe_event_enable(struct trace_uprobe *tu, int flag)
{
- struct uprobe_trace_consumer *utc;
int ret = 0;
if (is_trace_uprobe_enabled(tu))
return -EINTR;
- utc = kzalloc(sizeof(struct uprobe_trace_consumer), GFP_KERNEL);
- if (!utc)
- return -EINTR;
-
- utc->cons.handler = uprobe_dispatcher;
- utc->tu = tu;
- tu->consumer = utc;
tu->flags |= flag;
-
- ret = uprobe_register(tu->inode, tu->offset, &utc->cons);
- if (ret) {
- tu->consumer = NULL;
+ ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
+ if (ret)
tu->flags &= ~flag;
- kfree(utc);
- }
return ret;
}
@@ -573,10 +556,8 @@ static void probe_event_disable(struct trace_uprobe *tu, int flag)
if (!is_trace_uprobe_enabled(tu))
return;
- uprobe_unregister(tu->inode, tu->offset, &tu->consumer->cons);
+ uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
tu->flags &= ~flag;
- kfree(tu->consumer);
- tu->consumer = NULL;
}
static int uprobe_event_define_fields(struct ftrace_event_call *event_call)
@@ -714,13 +695,9 @@ int trace_uprobe_register(struct ftrace_event_call *event, enum trace_reg type,
static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
{
- struct uprobe_trace_consumer *utc;
struct trace_uprobe *tu;
- utc = container_of(con, struct uprobe_trace_consumer, cons);
- tu = utc->tu;
- if (!tu || tu->consumer != utc)
- return 0;
+ tu = container_of(con, struct trace_uprobe, consumer);
if (tu->flags & TP_FLAG_TRACE)
uprobe_trace_func(tu, regs);
--
1.5.5.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit
2013-01-31 19:17 [PATCH 0/6] uprobes/tracing: cleanups and minor fixes Oleg Nesterov
` (4 preceding siblings ...)
2013-01-31 19:18 ` [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe Oleg Nesterov
@ 2013-01-31 19:18 ` Oleg Nesterov
2013-02-04 11:17 ` Srikar Dronamraju
2013-02-11 9:19 ` Srikar Dronamraju
5 siblings, 2 replies; 18+ messages in thread
From: Oleg Nesterov @ 2013-01-31 19:18 UTC (permalink / raw)
To: Ingo Molnar, Srikar Dronamraju, Steven Rostedt
Cc: Anton Arapov, Frank Eigler, Josh Stone, Masami Hiramatsu,
Suzuki K. Poulose, linux-kernel
Move tu->nhit++ from uprobe_trace_func() to uprobe_dispatcher().
->nhit counts how many time we hit the breakpoint inserted by this
uprobe, we do not want to loose this info if uprobe was enabled by
sys_perf_event_open().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/trace/trace_uprobe.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 55cdc14..0a9a8de 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -473,8 +473,6 @@ static void uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
unsigned long irq_flags;
struct ftrace_event_call *call = &tu->call;
- tu->nhit++;
-
local_save_flags(irq_flags);
pc = preempt_count();
@@ -698,6 +696,7 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
struct trace_uprobe *tu;
tu = container_of(con, struct trace_uprobe, consumer);
+ tu->nhit++;
if (tu->flags & TP_FLAG_TRACE)
uprobe_trace_func(tu, regs);
--
1.5.5.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] uprobes/tracing: Ensure inode != NULL in create_trace_uprobe()
2013-01-31 19:18 ` [PATCH 3/6] uprobes/tracing: Ensure inode != NULL in create_trace_uprobe() Oleg Nesterov
@ 2013-02-04 10:48 ` Srikar Dronamraju
0 siblings, 0 replies; 18+ messages in thread
From: Srikar Dronamraju @ 2013-02-04 10:48 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Steven Rostedt, Anton Arapov, Frank Eigler,
Josh Stone, Masami Hiramatsu, Suzuki K. Poulose, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:22]:
> probe_event_enable/disable() check tu->inode != NULL at the start.
> This is ugly, if igrab() can fail create_trace_uprobe() should not
> succeed and "postpone" the failure.
>
> Note: alloc_uprobe() should probably check igrab() != NULL as well.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> kernel/trace/trace_uprobe.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 7b75949..f02bbec 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -255,6 +255,8 @@ static int create_trace_uprobe(int argc, char **argv)
>
> inode = igrab(path.dentry->d_inode);
> path_put(&path);
> + if (!inode)
> + goto fail_address_parse;
>
> ret = kstrtoul(arg, 0, &offset);
> if (ret)
> @@ -539,7 +541,7 @@ static int probe_event_enable(struct trace_uprobe *tu, int flag)
> struct uprobe_trace_consumer *utc;
> int ret = 0;
>
> - if (!tu->inode || tu->consumer)
> + if (tu->consumer)
> return -EINTR;
>
> utc = kzalloc(sizeof(struct uprobe_trace_consumer), GFP_KERNEL);
> @@ -563,7 +565,7 @@ static int probe_event_enable(struct trace_uprobe *tu, int flag)
>
> static void probe_event_disable(struct trace_uprobe *tu, int flag)
> {
> - if (!tu->inode || !tu->consumer)
> + if (!tu->consumer)
> return;
>
> uprobe_unregister(tu->inode, tu->offset, &tu->consumer->cons);
> --
> 1.5.5.1
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit
2013-01-31 19:18 ` [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit Oleg Nesterov
@ 2013-02-04 11:17 ` Srikar Dronamraju
2013-02-04 15:18 ` Oleg Nesterov
2013-02-11 9:19 ` Srikar Dronamraju
1 sibling, 1 reply; 18+ messages in thread
From: Srikar Dronamraju @ 2013-02-04 11:17 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Steven Rostedt, Anton Arapov, Frank Eigler,
Josh Stone, Masami Hiramatsu, Suzuki K. Poulose, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:32]:
> Move tu->nhit++ from uprobe_trace_func() to uprobe_dispatcher().
>
> ->nhit counts how many time we hit the breakpoint inserted by this
> uprobe, we do not want to loose this info if uprobe was enabled by
> sys_perf_event_open().
>
Though I dont see a problem with this change, It seems unnecessary for
me.
Info from nhits is mostly for /sys/kernel/debug/tracing/uprobe_profile
I am not sure how sys_perf_event_open() is making use of this?
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
> kernel/trace/trace_uprobe.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 55cdc14..0a9a8de 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -473,8 +473,6 @@ static void uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
> unsigned long irq_flags;
> struct ftrace_event_call *call = &tu->call;
>
> - tu->nhit++;
> -
> local_save_flags(irq_flags);
> pc = preempt_count();
>
> @@ -698,6 +696,7 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
> struct trace_uprobe *tu;
>
> tu = container_of(con, struct trace_uprobe, consumer);
> + tu->nhit++;
>
> if (tu->flags & TP_FLAG_TRACE)
> uprobe_trace_func(tu, regs);
> --
> 1.5.5.1
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit
2013-02-04 11:17 ` Srikar Dronamraju
@ 2013-02-04 15:18 ` Oleg Nesterov
2013-02-04 16:26 ` Srikar Dronamraju
0 siblings, 1 reply; 18+ messages in thread
From: Oleg Nesterov @ 2013-02-04 15:18 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Ingo Molnar, Steven Rostedt, Anton Arapov, Frank Eigler,
Josh Stone, Masami Hiramatsu, Suzuki K. Poulose, linux-kernel
On 02/04, Srikar Dronamraju wrote:
>
> * Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:32]:
>
> > Move tu->nhit++ from uprobe_trace_func() to uprobe_dispatcher().
> >
> > ->nhit counts how many time we hit the breakpoint inserted by this
> > uprobe, we do not want to loose this info if uprobe was enabled by
> > sys_perf_event_open().
> >
>
> Though I dont see a problem with this change, It seems unnecessary for
> me.
>
> Info from nhits is mostly for /sys/kernel/debug/tracing/uprobe_profile
It is only for uprobe_profile, yes, and it is useful. Why should we hide
this info if this uprobe is used by perf?
> I am not sure how sys_perf_event_open() is making use of this?
I hope I'll send the final series today. From the changelog of the patch
which actually turns the filtering on:
Testing:
# perf probe -x /lib/libc.so.6 syscall
# perl -e 'syscall -1 while 1' &
[1] 530
# perf record -e probe_libc:syscall perl -e 'syscall -1 for 1..10; sleep 1'
# perf report --show-total-period
100.00% 10 perl libc-2.8.so [.] syscall
Before this patch:
# cat /sys/kernel/debug/tracing/uprobe_profile
/lib/libc.so.6 syscall 79291
A huge ->nrhit == 79291 reflects the fact that the background process
530 constantly hits this breakpoint too, even if doesn't contribute to
the output.
After the patch:
# cat /sys/kernel/debug/tracing/uprobe_profile
/lib/libc.so.6 syscall 10
This shows that only the target process was punished by int3.
Oleg.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] uprobes/tracing: Introduce is_trace_uprobe_enabled()
2013-01-31 19:18 ` [PATCH 4/6] uprobes/tracing: Introduce is_trace_uprobe_enabled() Oleg Nesterov
@ 2013-02-04 16:12 ` Srikar Dronamraju
0 siblings, 0 replies; 18+ messages in thread
From: Srikar Dronamraju @ 2013-02-04 16:12 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Steven Rostedt, Anton Arapov, Frank Eigler,
Josh Stone, Masami Hiramatsu, Suzuki K. Poulose, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:25]:
> probe_event_enable/disable() check tu->consumer != NULL to avoid the
> wrong uprobe_register/unregister().
>
> We are going to kill this pointer and "struct uprobe_trace_consumer",
> so we add the new helper, is_trace_uprobe_enabled(), which can rely
> on TP_FLAG_TRACE/TP_FLAG_PROFILE instead.
>
> Note: the current logic doesn't look optimal, it is not clear why
> TP_FLAG_TRACE/TP_FLAG_PROFILE are mutually exclusive, we will probably
> change this later.
>
> Also kill the unused TP_FLAG_UPROBE.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> kernel/trace/trace_probe.h | 1 -
> kernel/trace/trace_uprobe.c | 9 +++++++--
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 9337086..5c7e09d 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -66,7 +66,6 @@
> #define TP_FLAG_TRACE 1
> #define TP_FLAG_PROFILE 2
> #define TP_FLAG_REGISTERED 4
> -#define TP_FLAG_UPROBE 8
>
>
> /* data_rloc: data relative location, compatible with u32 */
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index f02bbec..947379a 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -536,12 +536,17 @@ partial:
> return TRACE_TYPE_PARTIAL_LINE;
> }
>
> +static inline bool is_trace_uprobe_enabled(struct trace_uprobe *tu)
> +{
> + return tu->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE);
> +}
> +
> static int probe_event_enable(struct trace_uprobe *tu, int flag)
> {
> struct uprobe_trace_consumer *utc;
> int ret = 0;
>
> - if (tu->consumer)
> + if (is_trace_uprobe_enabled(tu))
> return -EINTR;
>
> utc = kzalloc(sizeof(struct uprobe_trace_consumer), GFP_KERNEL);
> @@ -565,7 +570,7 @@ static int probe_event_enable(struct trace_uprobe *tu, int flag)
>
> static void probe_event_disable(struct trace_uprobe *tu, int flag)
> {
> - if (!tu->consumer)
> + if (!is_trace_uprobe_enabled(tu))
> return;
>
> uprobe_unregister(tu->inode, tu->offset, &tu->consumer->cons);
> --
> 1.5.5.1
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit
2013-02-04 15:18 ` Oleg Nesterov
@ 2013-02-04 16:26 ` Srikar Dronamraju
2013-02-04 16:34 ` Steven Rostedt
0 siblings, 1 reply; 18+ messages in thread
From: Srikar Dronamraju @ 2013-02-04 16:26 UTC (permalink / raw)
To: Oleg Nesterov, Steven Rostedt, Masami Hiramatsu
Cc: Ingo Molnar, Anton Arapov, Frank Eigler, Josh Stone,
Suzuki K. Poulose, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2013-02-04 16:18:50]:
> On 02/04, Srikar Dronamraju wrote:
> >
> > * Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:32]:
> >
> > > Move tu->nhit++ from uprobe_trace_func() to uprobe_dispatcher().
> > >
> > > ->nhit counts how many time we hit the breakpoint inserted by this
> > > uprobe, we do not want to loose this info if uprobe was enabled by
> > > sys_perf_event_open().
> > >
> >
> > Though I dont see a problem with this change, It seems unnecessary for
> > me.
> >
> > Info from nhits is mostly for /sys/kernel/debug/tracing/uprobe_profile
>
> It is only for uprobe_profile, yes, and it is useful. Why should we hide
> this info if this uprobe is used by perf?
Fine with me.
Steve, Masami, Do you have comments/suggestions on this.
(Since kprobe_profile just accounts for kprobetracer and doesnt account
for perf record.)
May we should make a similar change in kprobetracer to keep things
similar.
--
Thanks and Regards
Srikar Dronamraju
>
> > I am not sure how sys_perf_event_open() is making use of this?
>
> I hope I'll send the final series today. From the changelog of the patch
> which actually turns the filtering on:
>
> Testing:
>
> # perf probe -x /lib/libc.so.6 syscall
>
> # perl -e 'syscall -1 while 1' &
> [1] 530
>
> # perf record -e probe_libc:syscall perl -e 'syscall -1 for 1..10; sleep 1'
>
> # perf report --show-total-period
> 100.00% 10 perl libc-2.8.so [.] syscall
>
> Before this patch:
>
> # cat /sys/kernel/debug/tracing/uprobe_profile
> /lib/libc.so.6 syscall 79291
>
> A huge ->nrhit == 79291 reflects the fact that the background process
> 530 constantly hits this breakpoint too, even if doesn't contribute to
> the output.
>
> After the patch:
>
> # cat /sys/kernel/debug/tracing/uprobe_profile
> /lib/libc.so.6 syscall 10
>
> This shows that only the target process was punished by int3.
>
> Oleg.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit
2013-02-04 16:26 ` Srikar Dronamraju
@ 2013-02-04 16:34 ` Steven Rostedt
0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2013-02-04 16:34 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Oleg Nesterov, Masami Hiramatsu, Ingo Molnar, Anton Arapov,
Frank Eigler, Josh Stone, Suzuki K. Poulose, linux-kernel
On Mon, 2013-02-04 at 21:56 +0530, Srikar Dronamraju wrote:
> * Oleg Nesterov <oleg@redhat.com> [2013-02-04 16:18:50]:
>
> > On 02/04, Srikar Dronamraju wrote:
> > >
> > > * Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:32]:
> > >
> > > > Move tu->nhit++ from uprobe_trace_func() to uprobe_dispatcher().
> > > >
> > > > ->nhit counts how many time we hit the breakpoint inserted by this
> > > > uprobe, we do not want to loose this info if uprobe was enabled by
> > > > sys_perf_event_open().
> > > >
> > >
> > > Though I dont see a problem with this change, It seems unnecessary for
> > > me.
> > >
> > > Info from nhits is mostly for /sys/kernel/debug/tracing/uprobe_profile
> >
> > It is only for uprobe_profile, yes, and it is useful. Why should we hide
> > this info if this uprobe is used by perf?
>
> Fine with me.
>
> Steve, Masami, Do you have comments/suggestions on this.
I have no problem with the change.
> (Since kprobe_profile just accounts for kprobetracer and doesnt account
> for perf record.)
> May we should make a similar change in kprobetracer to keep things
> similar.
I'm fine either way.
-- Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe
2013-01-31 19:18 ` [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe Oleg Nesterov
@ 2013-02-04 16:59 ` Srikar Dronamraju
2013-02-04 17:20 ` Oleg Nesterov
2013-02-11 9:19 ` Srikar Dronamraju
1 sibling, 1 reply; 18+ messages in thread
From: Srikar Dronamraju @ 2013-02-04 16:59 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Steven Rostedt, Anton Arapov, Frank Eigler,
Josh Stone, Masami Hiramatsu, Suzuki K. Poulose, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:29]:
> trace_uprobe->consumer and "struct uprobe_trace_consumer" add the
> unnecessary indirection and complicate the code for no reason.
>
> This patch simply embeds uprobe_consumer into "struct trace_uprobe",
> all other changes only fix the compilation errors.
>
I know this patch doesnt change the current behaviour.
We dont handle two concurrent perf record sessions for the same user
space probe. Since both sessons share the same trace_uprobe and hence
share the same consumer. Initially I had thought of having a chain in
uprobe_trace_consumer. However we dont get have enough information at
the probe_event_disable() time to detect which consumer to delete Hence
I dropped the idea of having a list of consumers attached to the
trace_uprobe.
However with allowing prefiltering, we need to have ability to
distinguish consumers. The idea of embedding uprobe_consumer within
trace_uprobe, may make the problem even more tougher to solve.
Should we document this as a TODO?
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
> kernel/trace/trace_uprobe.c | 35 ++++++-----------------------------
> 1 files changed, 6 insertions(+), 29 deletions(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 947379a..55cdc14 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -31,17 +31,11 @@
> /*
> * uprobe event core functions
> */
> -struct trace_uprobe;
> -struct uprobe_trace_consumer {
> - struct uprobe_consumer cons;
> - struct trace_uprobe *tu;
> -};
> -
> struct trace_uprobe {
> struct list_head list;
> struct ftrace_event_class class;
> struct ftrace_event_call call;
> - struct uprobe_trace_consumer *consumer;
> + struct uprobe_consumer consumer;
> struct inode *inode;
> char *filename;
> unsigned long offset;
> @@ -92,6 +86,7 @@ alloc_trace_uprobe(const char *group, const char *event, int nargs)
> goto error;
>
> INIT_LIST_HEAD(&tu->list);
> + tu->consumer.handler = uprobe_dispatcher;
> return tu;
>
> error:
> @@ -543,27 +538,15 @@ static inline bool is_trace_uprobe_enabled(struct trace_uprobe *tu)
>
> static int probe_event_enable(struct trace_uprobe *tu, int flag)
> {
> - struct uprobe_trace_consumer *utc;
> int ret = 0;
>
> if (is_trace_uprobe_enabled(tu))
> return -EINTR;
>
> - utc = kzalloc(sizeof(struct uprobe_trace_consumer), GFP_KERNEL);
> - if (!utc)
> - return -EINTR;
> -
> - utc->cons.handler = uprobe_dispatcher;
> - utc->tu = tu;
> - tu->consumer = utc;
> tu->flags |= flag;
> -
> - ret = uprobe_register(tu->inode, tu->offset, &utc->cons);
> - if (ret) {
> - tu->consumer = NULL;
> + ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
> + if (ret)
> tu->flags &= ~flag;
> - kfree(utc);
> - }
>
> return ret;
> }
> @@ -573,10 +556,8 @@ static void probe_event_disable(struct trace_uprobe *tu, int flag)
> if (!is_trace_uprobe_enabled(tu))
> return;
>
> - uprobe_unregister(tu->inode, tu->offset, &tu->consumer->cons);
> + uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
> tu->flags &= ~flag;
> - kfree(tu->consumer);
> - tu->consumer = NULL;
> }
>
> static int uprobe_event_define_fields(struct ftrace_event_call *event_call)
> @@ -714,13 +695,9 @@ int trace_uprobe_register(struct ftrace_event_call *event, enum trace_reg type,
>
> static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
> {
> - struct uprobe_trace_consumer *utc;
> struct trace_uprobe *tu;
>
> - utc = container_of(con, struct uprobe_trace_consumer, cons);
> - tu = utc->tu;
> - if (!tu || tu->consumer != utc)
> - return 0;
> + tu = container_of(con, struct trace_uprobe, consumer);
>
> if (tu->flags & TP_FLAG_TRACE)
> uprobe_trace_func(tu, regs);
> --
> 1.5.5.1
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe
2013-02-04 16:59 ` Srikar Dronamraju
@ 2013-02-04 17:20 ` Oleg Nesterov
2013-02-11 9:18 ` Srikar Dronamraju
0 siblings, 1 reply; 18+ messages in thread
From: Oleg Nesterov @ 2013-02-04 17:20 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Ingo Molnar, Steven Rostedt, Anton Arapov, Frank Eigler,
Josh Stone, Masami Hiramatsu, Suzuki K. Poulose, linux-kernel
On 02/04, Srikar Dronamraju wrote:
>
> * Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:29]:
>
> > trace_uprobe->consumer and "struct uprobe_trace_consumer" add the
> > unnecessary indirection and complicate the code for no reason.
> >
> > This patch simply embeds uprobe_consumer into "struct trace_uprobe",
> > all other changes only fix the compilation errors.
>
> I know this patch doesnt change the current behaviour.
Yes, and it makes the code simpler.
> We dont handle two concurrent perf record sessions for the same user
> space probe. Since both sessons share the same trace_uprobe and hence
> share the same consumer.
We do? I am testing the patches I am going to send, and I specially
tried to verify that 2 concurent sessions with different/same filtering
constraints work fine.
Or I misunderstood what you meant...
> Initially I had thought of having a chain in
> uprobe_trace_consumer. However we dont get have enough information at
> the probe_event_disable() time to detect which consumer to delete Hence
> I dropped the idea of having a list of consumers attached to the
> trace_uprobe.
You know, until recently I knew absolutely nothing about kernel/events/
and kernel/trace/. Not that I really understand this code now, I can
be easily wrong.
But so far I think that a chain of multiple consumers makes no sense.
Each consumer->handler() will use the same call->perf_events list, this
will only complicate the code for no reason.
> However with allowing prefiltering, we need to have ability to
> distinguish consumers.
Certainly not. Please see the patches I am going to send.
Oleg.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe
2013-02-04 17:20 ` Oleg Nesterov
@ 2013-02-11 9:18 ` Srikar Dronamraju
0 siblings, 0 replies; 18+ messages in thread
From: Srikar Dronamraju @ 2013-02-11 9:18 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Steven Rostedt, Anton Arapov, Frank Eigler,
Josh Stone, Masami Hiramatsu, Suzuki K. Poulose, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2013-02-04 18:20:10]:
>
> But so far I think that a chain of multiple consumers makes no sense.
> Each consumer->handler() will use the same call->perf_events list, this
> will only complicate the code for no reason.
>
> > However with allowing prefiltering, we need to have ability to
> > distinguish consumers.
>
> Certainly not. Please see the patches I am going to send.
>
Yeah, Looking at the next patchset made me realize this.
thanks
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe
2013-01-31 19:18 ` [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe Oleg Nesterov
2013-02-04 16:59 ` Srikar Dronamraju
@ 2013-02-11 9:19 ` Srikar Dronamraju
1 sibling, 0 replies; 18+ messages in thread
From: Srikar Dronamraju @ 2013-02-11 9:19 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Steven Rostedt, Anton Arapov, Frank Eigler,
Josh Stone, Masami Hiramatsu, Suzuki K. Poulose, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:29]:
> trace_uprobe->consumer and "struct uprobe_trace_consumer" add the
> unnecessary indirection and complicate the code for no reason.
>
> This patch simply embeds uprobe_consumer into "struct trace_uprobe",
> all other changes only fix the compilation errors.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> kernel/trace/trace_uprobe.c | 35 ++++++-----------------------------
> 1 files changed, 6 insertions(+), 29 deletions(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 947379a..55cdc14 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -31,17 +31,11 @@
> /*
> * uprobe event core functions
> */
> -struct trace_uprobe;
> -struct uprobe_trace_consumer {
> - struct uprobe_consumer cons;
> - struct trace_uprobe *tu;
> -};
> -
> struct trace_uprobe {
> struct list_head list;
> struct ftrace_event_class class;
> struct ftrace_event_call call;
> - struct uprobe_trace_consumer *consumer;
> + struct uprobe_consumer consumer;
> struct inode *inode;
> char *filename;
> unsigned long offset;
> @@ -92,6 +86,7 @@ alloc_trace_uprobe(const char *group, const char *event, int nargs)
> goto error;
>
> INIT_LIST_HEAD(&tu->list);
> + tu->consumer.handler = uprobe_dispatcher;
> return tu;
>
> error:
> @@ -543,27 +538,15 @@ static inline bool is_trace_uprobe_enabled(struct trace_uprobe *tu)
>
> static int probe_event_enable(struct trace_uprobe *tu, int flag)
> {
> - struct uprobe_trace_consumer *utc;
> int ret = 0;
>
> if (is_trace_uprobe_enabled(tu))
> return -EINTR;
>
> - utc = kzalloc(sizeof(struct uprobe_trace_consumer), GFP_KERNEL);
> - if (!utc)
> - return -EINTR;
> -
> - utc->cons.handler = uprobe_dispatcher;
> - utc->tu = tu;
> - tu->consumer = utc;
> tu->flags |= flag;
> -
> - ret = uprobe_register(tu->inode, tu->offset, &utc->cons);
> - if (ret) {
> - tu->consumer = NULL;
> + ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
> + if (ret)
> tu->flags &= ~flag;
> - kfree(utc);
> - }
>
> return ret;
> }
> @@ -573,10 +556,8 @@ static void probe_event_disable(struct trace_uprobe *tu, int flag)
> if (!is_trace_uprobe_enabled(tu))
> return;
>
> - uprobe_unregister(tu->inode, tu->offset, &tu->consumer->cons);
> + uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
> tu->flags &= ~flag;
> - kfree(tu->consumer);
> - tu->consumer = NULL;
> }
>
> static int uprobe_event_define_fields(struct ftrace_event_call *event_call)
> @@ -714,13 +695,9 @@ int trace_uprobe_register(struct ftrace_event_call *event, enum trace_reg type,
>
> static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
> {
> - struct uprobe_trace_consumer *utc;
> struct trace_uprobe *tu;
>
> - utc = container_of(con, struct uprobe_trace_consumer, cons);
> - tu = utc->tu;
> - if (!tu || tu->consumer != utc)
> - return 0;
> + tu = container_of(con, struct trace_uprobe, consumer);
>
> if (tu->flags & TP_FLAG_TRACE)
> uprobe_trace_func(tu, regs);
> --
> 1.5.5.1
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit
2013-01-31 19:18 ` [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit Oleg Nesterov
2013-02-04 11:17 ` Srikar Dronamraju
@ 2013-02-11 9:19 ` Srikar Dronamraju
1 sibling, 0 replies; 18+ messages in thread
From: Srikar Dronamraju @ 2013-02-11 9:19 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Ingo Molnar, Steven Rostedt, Anton Arapov, Frank Eigler,
Josh Stone, Masami Hiramatsu, Suzuki K. Poulose, linux-kernel
* Oleg Nesterov <oleg@redhat.com> [2013-01-31 20:18:32]:
> Move tu->nhit++ from uprobe_trace_func() to uprobe_dispatcher().
>
> ->nhit counts how many time we hit the breakpoint inserted by this
> uprobe, we do not want to loose this info if uprobe was enabled by
> sys_perf_event_open().
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> kernel/trace/trace_uprobe.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 55cdc14..0a9a8de 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -473,8 +473,6 @@ static void uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
> unsigned long irq_flags;
> struct ftrace_event_call *call = &tu->call;
>
> - tu->nhit++;
> -
> local_save_flags(irq_flags);
> pc = preempt_count();
>
> @@ -698,6 +696,7 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
> struct trace_uprobe *tu;
>
> tu = container_of(con, struct trace_uprobe, consumer);
> + tu->nhit++;
>
> if (tu->flags & TP_FLAG_TRACE)
> uprobe_trace_func(tu, regs);
> --
> 1.5.5.1
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-02-11 9:22 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-31 19:17 [PATCH 0/6] uprobes/tracing: cleanups and minor fixes Oleg Nesterov
2013-01-31 19:18 ` [PATCH 1/6] uprobes/tracing: Fix dentry/mount leak in create_trace_uprobe() Oleg Nesterov
2013-01-31 19:18 ` [PATCH 2/6] uprobes/tracing: Fully initialize uprobe_trace_consumer before uprobe_register() Oleg Nesterov
2013-01-31 19:18 ` [PATCH 3/6] uprobes/tracing: Ensure inode != NULL in create_trace_uprobe() Oleg Nesterov
2013-02-04 10:48 ` Srikar Dronamraju
2013-01-31 19:18 ` [PATCH 4/6] uprobes/tracing: Introduce is_trace_uprobe_enabled() Oleg Nesterov
2013-02-04 16:12 ` Srikar Dronamraju
2013-01-31 19:18 ` [PATCH 5/6] uprobes/tracing: Kill uprobe_trace_consumer, embed uprobe_consumer into trace_uprobe Oleg Nesterov
2013-02-04 16:59 ` Srikar Dronamraju
2013-02-04 17:20 ` Oleg Nesterov
2013-02-11 9:18 ` Srikar Dronamraju
2013-02-11 9:19 ` Srikar Dronamraju
2013-01-31 19:18 ` [PATCH 6/6] uprobes/perf: Always increment trace_uprobe->nhit Oleg Nesterov
2013-02-04 11:17 ` Srikar Dronamraju
2013-02-04 15:18 ` Oleg Nesterov
2013-02-04 16:26 ` Srikar Dronamraju
2013-02-04 16:34 ` Steven Rostedt
2013-02-11 9:19 ` Srikar Dronamraju
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).