* [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot
@ 2013-12-22 3:07 Steven Rostedt
2013-12-22 3:08 ` [PATCH 1/2] tracing: Add generic tracing_lseek() function Steven Rostedt
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Steven Rostedt @ 2013-12-22 3:07 UTC (permalink / raw
To: linux-kernel; +Cc: Tom Zanussi, Paul E. McKenney
Tom,
This is the changes I made to fix the reports that Fengguang's kbuild test bot
found. I folded in your change that fixes the bug with -ENODEV used in kernel.h.
Paul,
I've Cc'd you because the second patch is fixing up RCU notation. The filter
is protect by rcu_sched, and the first hunk of the patch is performed
within a rcu_read_lock_sched(), and the second hunk is done on the write
side. The tmp variable holds the old value, the value is updated, and
then the new value gets updated. I emailed you so that you can verify
that this all works.
Thanks!
-- Steve
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
ftrace/core
Head SHA1: 05741e383228a79eeec83644181255d2171f9f7c
Steven Rostedt (Red Hat) (2):
tracing: Add generic tracing_lseek() function
tracing: Fix rcu handling of event_trigger_data filter field
----
include/linux/ftrace.h | 2 --
kernel/trace/ftrace.c | 25 ++++++-------------------
kernel/trace/trace.c | 14 +++++++++-----
kernel/trace/trace.h | 4 +++-
kernel/trace/trace_events_trigger.c | 10 ++++++----
kernel/trace/trace_stack.c | 2 +-
6 files changed, 25 insertions(+), 32 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] tracing: Add generic tracing_lseek() function
2013-12-22 3:07 [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot Steven Rostedt
@ 2013-12-22 3:08 ` Steven Rostedt
2013-12-22 3:08 ` [PATCH 2/2] tracing: Fix rcu handling of event_trigger_data filter field Steven Rostedt
2013-12-23 3:13 ` [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot Tom Zanussi
2 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2013-12-22 3:08 UTC (permalink / raw
To: linux-kernel; +Cc: Tom Zanussi, Paul E. McKenney, kbuild test robot
[-- Attachment #1: 0001-tracing-Add-generic-tracing_lseek-function.patch --]
[-- Type: text/plain, Size: 6484 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Trace event triggers added a lseek that uses the ftrace_filter_lseek()
function. Unfortunately, when function tracing is not configured in
that function is not defined and the kernel fails to build.
This is the second time that function was added to a file ops and
it broke the build due to requiring special config dependencies.
Make a generic tracing_lseek() that all the tracing utilities may
use.
Also, modify the old ftrace_filter_lseek() to return 0 instead of
1 on WRONLY. Not sure why it was a 1 as that does not make sense.
This also changes the old tracing_seek() to modify the file pos
pointer on WRONLY as well.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace.h | 2 --
kernel/trace/ftrace.c | 25 ++++++-------------------
kernel/trace/trace.c | 14 +++++++++-----
kernel/trace/trace.h | 2 ++
kernel/trace/trace_events_trigger.c | 2 +-
kernel/trace/trace_stack.c | 2 +-
6 files changed, 19 insertions(+), 28 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 31ea4b4..f4233b1 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -570,8 +570,6 @@ static inline int
ftrace_regex_release(struct inode *inode, struct file *file) { return -ENODEV; }
#endif /* CONFIG_DYNAMIC_FTRACE */
-loff_t ftrace_filter_lseek(struct file *file, loff_t offset, int whence);
-
/* totally disable ftrace - can not re-enable after this */
void ftrace_kill(void);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0e9f9ea..531ffa6 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1082,19 +1082,6 @@ static __init void ftrace_profile_debugfs(struct dentry *d_tracer)
static struct pid * const ftrace_swapper_pid = &init_struct_pid;
-loff_t
-ftrace_filter_lseek(struct file *file, loff_t offset, int whence)
-{
- loff_t ret;
-
- if (file->f_mode & FMODE_READ)
- ret = seq_lseek(file, offset, whence);
- else
- file->f_pos = ret = 1;
-
- return ret;
-}
-
#ifdef CONFIG_DYNAMIC_FTRACE
#ifndef CONFIG_FTRACE_MCOUNT_RECORD
@@ -2739,7 +2726,7 @@ static void ftrace_filter_reset(struct ftrace_hash *hash)
* routine, you can use ftrace_filter_write() for the write
* routine if @flag has FTRACE_ITER_FILTER set, or
* ftrace_notrace_write() if @flag has FTRACE_ITER_NOTRACE set.
- * ftrace_filter_lseek() should be used as the lseek routine, and
+ * tracing_lseek() should be used as the lseek routine, and
* release must call ftrace_regex_release().
*/
int
@@ -3767,7 +3754,7 @@ static const struct file_operations ftrace_filter_fops = {
.open = ftrace_filter_open,
.read = seq_read,
.write = ftrace_filter_write,
- .llseek = ftrace_filter_lseek,
+ .llseek = tracing_lseek,
.release = ftrace_regex_release,
};
@@ -3775,7 +3762,7 @@ static const struct file_operations ftrace_notrace_fops = {
.open = ftrace_notrace_open,
.read = seq_read,
.write = ftrace_notrace_write,
- .llseek = ftrace_filter_lseek,
+ .llseek = tracing_lseek,
.release = ftrace_regex_release,
};
@@ -4038,7 +4025,7 @@ static const struct file_operations ftrace_graph_fops = {
.open = ftrace_graph_open,
.read = seq_read,
.write = ftrace_graph_write,
- .llseek = ftrace_filter_lseek,
+ .llseek = tracing_lseek,
.release = ftrace_graph_release,
};
@@ -4046,7 +4033,7 @@ static const struct file_operations ftrace_graph_notrace_fops = {
.open = ftrace_graph_notrace_open,
.read = seq_read,
.write = ftrace_graph_write,
- .llseek = ftrace_filter_lseek,
+ .llseek = tracing_lseek,
.release = ftrace_graph_release,
};
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
@@ -4719,7 +4706,7 @@ static const struct file_operations ftrace_pid_fops = {
.open = ftrace_pid_open,
.write = ftrace_pid_write,
.read = seq_read,
- .llseek = ftrace_filter_lseek,
+ .llseek = tracing_lseek,
.release = ftrace_pid_release,
};
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 59bf5b5..e32a2f4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3183,19 +3183,23 @@ tracing_write_stub(struct file *filp, const char __user *ubuf,
return count;
}
-static loff_t tracing_seek(struct file *file, loff_t offset, int origin)
+loff_t tracing_lseek(struct file *file, loff_t offset, int whence)
{
+ int ret;
+
if (file->f_mode & FMODE_READ)
- return seq_lseek(file, offset, origin);
+ ret = seq_lseek(file, offset, whence);
else
- return 0;
+ file->f_pos = ret = 0;
+
+ return ret;
}
static const struct file_operations tracing_fops = {
.open = tracing_open,
.read = seq_read,
.write = tracing_write_stub,
- .llseek = tracing_seek,
+ .llseek = tracing_lseek,
.release = tracing_release,
};
@@ -4940,7 +4944,7 @@ static const struct file_operations snapshot_fops = {
.open = tracing_snapshot_open,
.read = seq_read,
.write = tracing_snapshot_write,
- .llseek = tracing_seek,
+ .llseek = tracing_lseek,
.release = tracing_snapshot_release,
};
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 433bfc5..0380cab 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -588,6 +588,8 @@ void tracing_start_sched_switch_record(void);
int register_tracer(struct tracer *type);
int is_tracing_stopped(void);
+loff_t tracing_lseek(struct file *file, loff_t offset, int whence);
+
extern cpumask_var_t __read_mostly tracing_buffer_mask;
#define for_each_tracing_cpu(cpu) \
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index f5b3f78..12ac8a5 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -281,7 +281,7 @@ const struct file_operations event_trigger_fops = {
.open = event_trigger_open,
.read = seq_read,
.write = event_trigger_write,
- .llseek = ftrace_filter_lseek,
+ .llseek = tracing_lseek,
.release = event_trigger_release,
};
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index b20428c..e6be585 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -382,7 +382,7 @@ static const struct file_operations stack_trace_filter_fops = {
.open = stack_trace_filter_open,
.read = seq_read,
.write = ftrace_filter_write,
- .llseek = ftrace_filter_lseek,
+ .llseek = tracing_lseek,
.release = ftrace_regex_release,
};
--
1.8.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] tracing: Fix rcu handling of event_trigger_data filter field
2013-12-22 3:07 [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot Steven Rostedt
2013-12-22 3:08 ` [PATCH 1/2] tracing: Add generic tracing_lseek() function Steven Rostedt
@ 2013-12-22 3:08 ` Steven Rostedt
2014-01-09 1:30 ` Paul E. McKenney
2013-12-23 3:13 ` [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot Tom Zanussi
2 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2013-12-22 3:08 UTC (permalink / raw
To: linux-kernel; +Cc: Tom Zanussi, Paul E. McKenney, kbuild test robot
[-- Attachment #1: 0002-tracing-Fix-rcu-handling-of-event_trigger_data-filte.patch --]
[-- Type: text/plain, Size: 2288 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
The filter field of the event_trigger_data structure is protected under
RCU sched locks. It was not annotated as such, and after doing so,
sparse pointed out several locations that required fix ups.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace.h | 2 +-
kernel/trace/trace_events_trigger.c | 8 +++++---
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 0380cab..02b592f 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1057,7 +1057,7 @@ struct event_trigger_data {
int ref;
struct event_trigger_ops *ops;
struct event_command *cmd_ops;
- struct event_filter *filter;
+ struct event_filter __rcu *filter;
char *filter_str;
void *private_data;
struct list_head list;
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 12ac8a5..f6dd115 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -67,6 +67,7 @@ event_triggers_call(struct ftrace_event_file *file, void *rec)
{
struct event_trigger_data *data;
enum event_trigger_type tt = ETT_NONE;
+ struct event_filter *filter;
if (list_empty(&file->triggers))
return tt;
@@ -76,7 +77,8 @@ event_triggers_call(struct ftrace_event_file *file, void *rec)
data->ops->func(data);
continue;
}
- if (data->filter && !filter_match_preds(data->filter, rec))
+ filter = rcu_dereference(data->filter);
+ if (filter && !filter_match_preds(filter, rec))
continue;
if (data->cmd_ops->post_trigger) {
tt |= data->cmd_ops->trigger_type;
@@ -703,7 +705,7 @@ static int set_trigger_filter(char *filter_str,
if (ret)
goto out;
assign:
- tmp = data->filter;
+ tmp = rcu_access_pointer(data->filter);
rcu_assign_pointer(data->filter, filter);
@@ -719,7 +721,7 @@ static int set_trigger_filter(char *filter_str,
if (filter_str) {
data->filter_str = kstrdup(filter_str, GFP_KERNEL);
if (!data->filter_str) {
- free_event_filter(data->filter);
+ free_event_filter(rcu_access_pointer(data->filter));
data->filter = NULL;
ret = -ENOMEM;
}
--
1.8.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot
2013-12-22 3:07 [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot Steven Rostedt
2013-12-22 3:08 ` [PATCH 1/2] tracing: Add generic tracing_lseek() function Steven Rostedt
2013-12-22 3:08 ` [PATCH 2/2] tracing: Fix rcu handling of event_trigger_data filter field Steven Rostedt
@ 2013-12-23 3:13 ` Tom Zanussi
2014-01-02 17:11 ` Steven Rostedt
2 siblings, 1 reply; 6+ messages in thread
From: Tom Zanussi @ 2013-12-23 3:13 UTC (permalink / raw
To: Steven Rostedt; +Cc: linux-kernel, Paul E. McKenney
Hi Steve,
On Sat, 2013-12-21 at 22:07 -0500, Steven Rostedt wrote:
> Tom,
>
> This is the changes I made to fix the reports that Fengguang's kbuild test bot
> found. I folded in your change that fixes the bug with -ENODEV used in kernel.h.
>
These look fine to me, and I didn't see any problems after running
through my normal event trigger testing with them applied.
So you can add my Acked-by and/or Tested-by..
Thanks for writing these improved versions!
Tom
> Paul,
>
> I've Cc'd you because the second patch is fixing up RCU notation. The filter
> is protect by rcu_sched, and the first hunk of the patch is performed
> within a rcu_read_lock_sched(), and the second hunk is done on the write
> side. The tmp variable holds the old value, the value is updated, and
> then the new value gets updated. I emailed you so that you can verify
> that this all works.
>
> Thanks!
>
> -- Steve
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
> ftrace/core
>
> Head SHA1: 05741e383228a79eeec83644181255d2171f9f7c
>
>
> Steven Rostedt (Red Hat) (2):
> tracing: Add generic tracing_lseek() function
> tracing: Fix rcu handling of event_trigger_data filter field
>
> ----
> include/linux/ftrace.h | 2 --
> kernel/trace/ftrace.c | 25 ++++++-------------------
> kernel/trace/trace.c | 14 +++++++++-----
> kernel/trace/trace.h | 4 +++-
> kernel/trace/trace_events_trigger.c | 10 ++++++----
> kernel/trace/trace_stack.c | 2 +-
> 6 files changed, 25 insertions(+), 32 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot
2013-12-23 3:13 ` [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot Tom Zanussi
@ 2014-01-02 17:11 ` Steven Rostedt
0 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2014-01-02 17:11 UTC (permalink / raw
To: Tom Zanussi; +Cc: linux-kernel, Paul E. McKenney
On Sun, 22 Dec 2013 21:13:02 -0600
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:
> Hi Steve,
>
> On Sat, 2013-12-21 at 22:07 -0500, Steven Rostedt wrote:
> > Tom,
> >
> > This is the changes I made to fix the reports that Fengguang's kbuild test bot
> > found. I folded in your change that fixes the bug with -ENODEV used in kernel.h.
> >
>
> These look fine to me, and I didn't see any problems after running
> through my normal event trigger testing with them applied.
>
> So you can add my Acked-by and/or Tested-by..
>
> Thanks for writing these improved versions!
Bah, I forgot to add these to the change logs before pushing to my
for-next branch. I hate to rebase, but I also would like these tags
too.
Hmm, I may just rebase anyway. We still have a couple of weeks before
the merge window opens.
-- Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] tracing: Fix rcu handling of event_trigger_data filter field
2013-12-22 3:08 ` [PATCH 2/2] tracing: Fix rcu handling of event_trigger_data filter field Steven Rostedt
@ 2014-01-09 1:30 ` Paul E. McKenney
0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2014-01-09 1:30 UTC (permalink / raw
To: Steven Rostedt; +Cc: linux-kernel, Tom Zanussi, kbuild test robot
On Sat, Dec 21, 2013 at 10:08:01PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
>
> The filter field of the event_trigger_data structure is protected under
> RCU sched locks. It was not annotated as such, and after doing so,
> sparse pointed out several locations that required fix ups.
>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Looks good to me!
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
> kernel/trace/trace.h | 2 +-
> kernel/trace/trace_events_trigger.c | 8 +++++---
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 0380cab..02b592f 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -1057,7 +1057,7 @@ struct event_trigger_data {
> int ref;
> struct event_trigger_ops *ops;
> struct event_command *cmd_ops;
> - struct event_filter *filter;
> + struct event_filter __rcu *filter;
> char *filter_str;
> void *private_data;
> struct list_head list;
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index 12ac8a5..f6dd115 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -67,6 +67,7 @@ event_triggers_call(struct ftrace_event_file *file, void *rec)
> {
> struct event_trigger_data *data;
> enum event_trigger_type tt = ETT_NONE;
> + struct event_filter *filter;
>
> if (list_empty(&file->triggers))
> return tt;
> @@ -76,7 +77,8 @@ event_triggers_call(struct ftrace_event_file *file, void *rec)
> data->ops->func(data);
> continue;
> }
> - if (data->filter && !filter_match_preds(data->filter, rec))
> + filter = rcu_dereference(data->filter);
> + if (filter && !filter_match_preds(filter, rec))
> continue;
> if (data->cmd_ops->post_trigger) {
> tt |= data->cmd_ops->trigger_type;
> @@ -703,7 +705,7 @@ static int set_trigger_filter(char *filter_str,
> if (ret)
> goto out;
> assign:
> - tmp = data->filter;
> + tmp = rcu_access_pointer(data->filter);
>
> rcu_assign_pointer(data->filter, filter);
>
> @@ -719,7 +721,7 @@ static int set_trigger_filter(char *filter_str,
> if (filter_str) {
> data->filter_str = kstrdup(filter_str, GFP_KERNEL);
> if (!data->filter_str) {
> - free_event_filter(data->filter);
> + free_event_filter(rcu_access_pointer(data->filter));
> data->filter = NULL;
> ret = -ENOMEM;
> }
> --
> 1.8.4.3
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-09 22:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-22 3:07 [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot Steven Rostedt
2013-12-22 3:08 ` [PATCH 1/2] tracing: Add generic tracing_lseek() function Steven Rostedt
2013-12-22 3:08 ` [PATCH 2/2] tracing: Fix rcu handling of event_trigger_data filter field Steven Rostedt
2014-01-09 1:30 ` Paul E. McKenney
2013-12-23 3:13 ` [PATCH 0/2] tracing: Fixes to event_triggers patches found by Fengguang's test bot Tom Zanussi
2014-01-02 17:11 ` Steven Rostedt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.