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

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 26 Jan 2015 21:40:20 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:


> Is the following correct?
> 	* only one directory in the entire tree (/instances) allows mkdir/rmdir.

yes

> 	* ftrace_trace_arrays always starts with global_trace and the rest
> is in 1-to-1 correspondence with subdirectories of /instances.

pretty much.

> 	* tr->name is NULL for global_trace and non-NULL for everything
> else.

yes.

> 	* all modifications of that list are happening from mkdir/rmdir

yes.

> 	* in the end of ->mkdir tr->dir set to the dentry of our subdirectory,
> and it's non-NULL (or trace_array creation simply fails)

yes.

> 	* global_array.dir is set to magical value ((struct dentry *)1) upon

This is new for this patch series. Currently, global_array.dir points
to the dentry of /sys/kernel/debug/tracing. 

> the first call of tracing_init_dentry().  Prior to that it's NULL.  BTW, may
> I politely inquire what the fuck are those contortions in
> tracing_init_dentry_tr() about?  Looks like a stunningly convoluted way
> to trigger that automount point creation early in tracer_init_tracefs().
> Why not do that right there explicitly?

Yeah, that could be cleaned up. Before the tracefs code, it made much
more sense to keep that as a single function. Now that global_array.dir
is treated differently as the subdirs, it does make sense to have
global_arry.dir initialized in a separate function.

I'll update my patch series to do this.


> 
> What are mount options doing?  I mean, sure, you set the mode/uid/gid
> of root.  Which is not modifiable anyway...  And AFAICS that's all those
> options are affecting.

You mean in tracefs.c/inode.c? I have no idea, they are there from
debugfs cut and paste ;-)

> 
> AFAICS, nothing ever removes files in debugfs root.  Right?  If so, you don't

I'm not sure about that. Well, not from tracing or from userspace. But
I believe things can be removed from debugfs root with module unload.
But are you talking about tracefs root?

> need the games with simple_pin_fs() - they are pointless in such situation
> anyway.  Just do tracefs_mount = kern_mount(&trace_fs_type); in tracefs_init()
> and be done with that; lose the tracefs_mount_count and all calls of
> simple_{pin,release}_fs().

OK, cool. Because I had no idea what hey were used for anyway. ;-)

> 
> While we are at it, what the hell is tracefs_file_operations about?  Looks
> like some bastard offspring of /dev/null, but I don't see anything that would
> use it...

Hehe, again, this is what debugfs did and I just brought it over
thinking I needed it.

Thanks,

-- Steve