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

From: Steven Rostedt
Date: Mon Jan 26 2015 - 19:33:10 EST


On Mon, 26 Jan 2015 21:40:20 +0000
Al Viro <viro@xxxxxxxxxxxxxxxxxx> 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
--
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/