Re: [for-next][PATCH 3/7] tracefs: Add new tracefs file system

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 9 Feb 2015 14:56:19 +0900
Namhyung Kim <namhyung@kernel.org> wrote:

> Hi Steve,
> 
> On Wed, Feb 04, 2015 at 09:34:23AM -0500, Steven Rostedt wrote:
> > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > 
> > Add a separate file system to handle the tracing directory. Currently it
> > is part of debugfs, but that is starting to show its limits.
> > 
> > One thing is that in order to access the tracing infrastructure, you need
> > to mount debugfs. As that includes debugging from all sorts of sub systems
> > in the kernel, it is not considered advisable to mount such an all
> > encompassing debugging system.
> > 
> > Having the tracing system in its own file systems gives access to the
> > tracing sub system without needing to include all other systems.
> > 
> > Another problem with tracing using the debugfs system is that the
> > instances use mkdir to create sub buffers. debugfs does not support mkdir
> > from userspace so to implement it, special hacks were used. By controlling
> > the file system that the tracing infrastructure uses, this can be properly
> > done without hacks.
> > 
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> > ---
> 
> [SNIP]
> > +/**
> > + * tracefs_create_file - create a file in the tracefs filesystem
> > + * @name: a pointer to a string containing the name of the file to create.
> > + * @mode: the permission that the file should have.
> > + * @parent: a pointer to the parent dentry for this file.  This should be a
> > + *          directory dentry if set.  If this parameter is NULL, then the
> > + *          file will be created in the root of the tracefs filesystem.
> > + * @data: a pointer to something that the caller will want to get to later
> > + *        on.  The inode.i_private pointer will point to this value on
> > + *        the open() call.
> > + * @fops: a pointer to a struct file_operations that should be used for
> > + *        this file.
> > + *
> > + * This is the basic "create a file" function for tracefs.  It allows for a
> > + * wide range of flexibility in creating a file, or a directory (if you want
> > + * to create a directory, the tracefs_create_dir() function is
> > + * recommended to be used instead.)
> > + *
> > + * This function will return a pointer to a dentry if it succeeds.  This
> > + * pointer must be passed to the tracefs_remove() function when the file is
> > + * to be removed (no automatic cleanup happens if your module is unloaded,
> > + * you are responsible here.)  If an error occurs, %NULL will be returned.
> > + *
> > + * If tracefs is not enabled in the kernel, the value -%ENODEV will be
> > + * returned.
> 
> I cannot find where it returns -ENODEV.  AFAICS we cannot call tracefs
> functions if tracing was not enabled.

Ah, originally I had it like debugfs, where these functions were stubs
that would return ENODEV when TRACING was not configured. But then, I
decided that nothing should be calling these functions when tracing is
not configured.

I'll nuke that comment.

> 
> 
> > + */
> > +struct dentry *tracefs_create_file(const char *name, umode_t mode,
> > +				   struct dentry *parent, void *data,
> > +				   const struct file_operations *fops)
> > +{
> > +	struct dentry *dentry;
> > +	struct inode *inode;
> > +
> > +	if (!(mode & S_IFMT))
> > +		mode |= S_IFREG;
> > +	BUG_ON(!S_ISREG(mode));
> > +	dentry = start_creating(name, parent);
> > +
> > +	if (IS_ERR(dentry))
> > +		return NULL;
> > +
> > +	inode = tracefs_get_inode(dentry->d_sb);
> > +	if (unlikely(!inode))
> > +		return failed_creating(dentry);
> > +
> > +	inode->i_mode = mode;
> > +	inode->i_fop = fops ? fops : &tracefs_file_operations;
> > +	inode->i_private = data;
> > +	d_instantiate(dentry, inode);
> > +	fsnotify_create(dentry->d_parent->d_inode, dentry);
> > +	return end_creating(dentry);
> > +}
> > +
> > +/**
> > + * tracefs_create_dir - create a directory in the tracefs filesystem
> > + * @name: a pointer to a string containing the name of the directory to
> > + *        create.
> > + * @parent: a pointer to the parent dentry for this file.  This should be a
> > + *          directory dentry if set.  If this parameter is NULL, then the
> > + *          directory will be created in the root of the tracefs filesystem.
> > + *
> > + * This function creates a directory in tracefs with the given name.
> > + *
> > + * This function will return a pointer to a dentry if it succeeds.  This
> > + * pointer must be passed to the tracefs_remove() function when the file is
> > + * to be removed. If an error occurs, %NULL will be returned.
> > + *
> > + * If tracing is not enabled in the kernel, the value -%ENODEV will be
> > + * returned.
> 
> Ditto.

And that one too.

Thanks,

-- Steve