Re: [PATCH 3/5 v2] tracing: Automatically mount tracefs on debugfs/tracing

[Steven Rostedt]

On Sat, 24 Jan 2015 11:00:41 +0800
Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:

> > +		if (traced->d_op) {
> > +			/*
> > +			 * FIXME:
> > +			 * Currently debugfs sets the d_op by a
> > side-effect
> > +			 * of calling simple_lookup(). Normally,
> > we should
> > +			 * never change d_op of a dentry, but as
> > this is
> > +			 * happening at boot up and shouldn't be
> > racing with
> > +			 * any other users, this should be OK. But
> > it is still
> > +			 * a hack, and needs to be properly done.
> > +			 */
> > +			trace_ops = *traced->d_op;
> > +			trace_ops.d_automount = trace_automount;
> > +			traced->d_flags |= DCACHE_NEED_AUTOMOUNT;
> > +			traced->d_op = &trace_ops;
> > +		} else {
> > +			/* Ideally, this is what should happen */
> > +			trace_ops = simple_dentry_operations;
> > +			trace_ops.d_automount = trace_automount;
> > +			d_set_d_op(traced, &trace_ops);
> 
> How will this else block run if debugfs is setting d_op in the
> debugfs_create_dir() call?

It wont; I put the else block there to show what we would like to do.
And would hopefully work if debugfs ever changed.


> 
> What really do you want to do here, just automount a filesystem on
> debugfs?  If so, can't we just add a new debugfs call to do that?

We could add a call to debugfs to do that. Would you prefer that? From
talking with Al, it sounds to me that changing d_ops on the fly is very
racy. Adding a call in debugfs sounds like it would be open for other
users to do the same and do so while the system is running. Would that
be wise?

Doing the automount here is guaranteed not to happen after system boot
up, or when there might be users of debugfs while the ops changes.

-- Steve


-- 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/