Re: [PATCH RESEND] tracing: Make tracing work when debugfs is not compiled or initialized.

Steven Rostedt
2015-11-04T15:03:43Z

On Wed, 4 Nov 2015 09:11:18 +0800
Jiaxing Wang <hello.wjx@gmail.com> wrote:

> Currently tracing_init_dentry() returns -ENODEV when debugfs is not
> initialized, which causes tracefs not populated with tracing files and
> directories, so we will get an empty directory even after we manually
> mount tracefs.
>
> We can make tracing_init_dentry() return NULL as long as tracefs
> is initialized and get a populated tracefs.
>
> We also need to make global_trace.dir not NULL in order to pass the checks
> in tracing_get_dentry() and add_tracer_options().
>
> Also added stub debugfs_create_automount() for when debugfs is not
> configured in.
>
> Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com>
> ---
> include/linux/debugfs.h | 8 ++++++++
> kernel/trace/Kconfig | 1 -
> kernel/trace/trace.c | 29 +++++++++++++++++------------
> 3 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
> index 9beb636..b42ef88 100644
> --- a/include/linux/debugfs.h
> +++ b/include/linux/debugfs.h
> @@ -160,6 +160,14 @@ static inline struct dentry *debugfs_create_symlink(const char *name,
> return ERR_PTR(-ENODEV);
> }
>
> +static inline struct dentry *debugfs_create_automount(const char *name,
> + struct dentry *parent,
> + struct vfsmount *(*f)(void *),
> + void *data)
> +{
> + return ERR_PTR(-ENODEV);
> +}

This part needs an Acked-by from Greg KH.

> +
> static inline void debugfs_remove(struct dentry *dentry)
> { }
>
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 1153c43..59f6377f 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -95,7 +95,6 @@ config RING_BUFFER_ALLOW_SWAP
>
> config TRACING
> bool
> - select DEBUG_FS
> select RING_BUFFER
> select STACKTRACE if STACKTRACE_SUPPORT
> select TRACEPOINTS
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 6e79408..6dd064e 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6706,6 +6706,8 @@ static struct vfsmount *trace_automount(void *ingore)
> return mnt;
> }
>
> +#define TRACE_TOP_DIR_ENTRY ((struct dentry *)1)
> +
> /**
> * tracing_init_dentry - initialize top level trace array
> *
> @@ -6716,27 +6718,30 @@ static struct vfsmount *trace_automount(void *ingore)
> struct dentry *tracing_init_dentry(void)
> {
> struct trace_array *tr = &global_trace;
> + struct dentry *traced;
>
> /* The top level trace array uses NULL as parent */
> if (tr->dir)
> return NULL;
>
> - if (WARN_ON(!debugfs_initialized()))
> + if (WARN_ON(!tracefs_initialized()))
> return ERR_PTR(-ENODEV);
>
> - /*
> - * As there may still be users that expect the tracing
> - * files to exist in debugfs/tracing, we must automount
> - * the tracefs file system there, so older tools still
> - * work with the newer kerenl.
> - */
> - tr->dir = debugfs_create_automount("tracing", NULL,
> - trace_automount, NULL);
> - if (!tr->dir) {
> - pr_warn_once("Could not create debugfs directory 'tracing'\n");
> - return ERR_PTR(-ENOMEM);
> + if (debugfs_initialized()) {
> + /*
> + * As there may still be users that expect the tracing
> + * files to exist in debugfs/tracing, we must automount
> + * the tracefs file system there, so older tools still
> + * work with the newer kerenl.
> + */
> + traced = debugfs_create_automount("tracing", NULL,
> + trace_automount, NULL);
> + if (!traced)
> + pr_warn_once("Could not create debugfs directory 'tracing'\n");

This should return a warning, and please keep the tr->dir instead of
this new traced variable.

> }
>
> + tr->dir = TRACE_TOP_DIR_ENTRY;
> +

Also, no need to add this, because if debugfs is not initialize, then
tr->dir would be ERR_PTR(-ENODEV), which still works as tr->dir is not
NULL.

-- Steve

> return NULL;
> }
>