Re: [PATCH 00/16 v3] tracing: Add new file system tracefs

[Steven Rostedt]

On Tue, 27 Jan 2015 04:34:21 +0000
Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:


> Frankly, the first impression is that you have trace_types_lock way too
> high in the hierarchy - you are taking it well outside of the level where
> you start walking through ftrace_trace_arrays...

Actually, it is originally used for modifying of tr->current_trace

This was before having multiple buffers. I just used the same lock to
protect the trace arrays. Note, I have patches to make a separate mutex
for that, but it just seems to make things a little more complex, so I
haven't pushed it too much. Yeah, that trace_types_lock is basically
tracing's BKL.

> 
> What else is it used for?  You seem to use it protect trace_array refcounts,
> but you use them in a very odd way...  What happens if lookup for a file
> in that tree loses CPU just as it enters trace_array_get() (e.g. on the
> same trace_types_lock), at which point rm .../instances/<whatever> comes,
> sees that tr->ref is still zero, kicks the sucker out of the list and
> frees it, immediately followed by mkdir creating a new instace?  If the
> allocation of the new trace_array happens to reuse the just-freed one,
> your trace_array_get() will actually succeed, won't it?
> 
> It's not fatal (after all, everything will work as if you opened the similar
> file in new instance in the first place), but it looks rather bogus...

I'm actually aware of this possibility. But as you said, it's not
fatal, and very unlikely to happen.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/