Re: [PATCH 1/1] tracing: Simplify trace_array_get()

2013-07-21 Thread Oleg Nesterov
On 07/19, Oleg Nesterov wrote: > > On 07/19, Steven Rostedt wrote: > > > > Correct, but I don't see that as a major problem, do you? > > Neither me. I should have mentioned this. > > Except "might get a strange result" as you said. > > But I have to admit, I am still trying to find something really

Re: [PATCH 1/1] tracing: Simplify trace_array_get()

2013-07-19 Thread Oleg Nesterov
On 07/19, Steven Rostedt wrote: > > On Fri, 2013-07-19 at 19:20 +0200, Oleg Nesterov wrote: > > > Yes. But unless I missed something again this logic doesn't look exactly > > correct. Because it seems that trace_array_get() can succeed when it > > shoudn't. > > > > trace_array_get() can race with i

Re: [PATCH 1/1] tracing: Simplify trace_array_get()

2013-07-19 Thread Steven Rostedt
On Fri, 2013-07-19 at 13:35 -0400, Steven Rostedt wrote: > What would happen in that case, is that an event might be enabled or > disabled in another buffer instance. As that can only happen by the root > user, it would be the root user doing multiple things at the same time > to cause it. They mi

Re: [PATCH 1/1] tracing: Simplify trace_array_get()

2013-07-19 Thread Steven Rostedt
On Fri, 2013-07-19 at 19:20 +0200, Oleg Nesterov wrote: > Yes. But unless I missed something again this logic doesn't look exactly > correct. Because it seems that trace_array_get() can succeed when it > shoudn't. > > trace_array_get() can race with instance_delete() + new_instance_create(), > an

Re: [PATCH 1/1] tracing: Simplify trace_array_get()

2013-07-19 Thread Oleg Nesterov
On 07/19, Oleg Nesterov wrote: > > On 07/19, Steven Rostedt wrote: > > > > On Fri, 2013-07-19 at 17:51 +0200, Oleg Nesterov wrote: > > > mutex_lock(&trace_types_lock); > > > - list_for_each_entry(tr, &ftrace_trace_arrays, list) { > > > - if (tr == this_tr) { > > > - tr->re

Re: [PATCH 1/1] tracing: Simplify trace_array_get()

2013-07-19 Thread Oleg Nesterov
On 07/19, Steven Rostedt wrote: > > On Fri, 2013-07-19 at 17:51 +0200, Oleg Nesterov wrote: > > mutex_lock(&trace_types_lock); > > - list_for_each_entry(tr, &ftrace_trace_arrays, list) { > > - if (tr == this_tr) { > > - tr->ref++; > > - ret = 0; >

Re: [PATCH 1/1] tracing: Simplify trace_array_get()

2013-07-19 Thread Steven Rostedt
On Fri, 2013-07-19 at 17:51 +0200, Oleg Nesterov wrote: > trace_array_get() scans the global ftrace_trace_arrays list > to ensure that "this_tr" was not removed by instance_delete(). > > This looks a bit confusing, we can simply use list_empty() with > the same result. list_empty() == F can not be

[PATCH 1/1] tracing: Simplify trace_array_get()

2013-07-19 Thread Oleg Nesterov
trace_array_get() scans the global ftrace_trace_arrays list to ensure that "this_tr" was not removed by instance_delete(). This looks a bit confusing, we can simply use list_empty() with the same result. list_empty() == F can not be false positive, new_instance_create() does everything under the s