Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1260669 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/3] tracing: Make tracing work when debugfs is not  compiled or initialized. Steven Rostedt <rostedt@goodmis.org> - 2015-11-02 15:20 +0100
    [PATCH RESEND] tracing: Make tracing work when debugfs is not compiled or initialized. Jiaxing Wang <hello.wjx@gmail.com> - 2015-11-04 02:20 +0100
      Re: [PATCH RESEND] tracing: Make tracing work when debugfs is not  compiled or initialized. Steven Rostedt <rostedt@goodmis.org> - 2015-11-04 16:10 +0100
        Re: [PATCH RESEND] tracing: Make tracing work when debugfs is not  compiled or initialized. Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-11-04 20:00 +0100
          Re: [PATCH RESEND] tracing: Make tracing work when debugfs is not  compiled or initialized. Steven Rostedt <rostedt@goodmis.org> - 2015-11-04 21:40 +0100
        Re: [PATCH RESEND] tracing: Make tracing work when debugfs is not  compiled or initialized. Jiaxing Wang <hello.wjx@gmail.com> - 2015-11-05 06:30 +0100
          Re: [PATCH RESEND] tracing: Make tracing work when debugfs is not  compiled or initialized. Steven Rostedt <rostedt@goodmis.org> - 2015-11-05 17:00 +0100
            [PATCH] tracing: Make tracing work when debugfs is not configured in Jiaxing Wang <hello.wjx@gmail.com> - 2015-11-06 09:10 +0100
            Re: [PATCH RESEND] tracing: Make tracing work when debugfs is not  compiled or initialized. Jiaxing Wang <hello.wjx@gmail.com> - 2015-11-06 09:10 +0100

#1260669 — Re: [PATCH 2/3] tracing: Make tracing work when debugfs is not compiled or initialized.

On Mon, 19 Oct 2015 09:54:16 +0800
Jiaxing Wang <hello.wjx@gmail.com> wrote:

> Sorry for the last patch, please use this to add stub for
> debugfs_create_automount().

Can you fold this into patch number two and resubmit that patch.

Thanks,

-- Steve

> 
> >From b3b877d8d9fd9795ea1055042039a272e47f4dc5 Mon Sep 17 00:00:00 2001
> From: Jiaxing Wang <hello.wjx@gmail.com>
> Date: Mon, 19 Oct 2015 09:46:12 +0800
> Subject: [PATCH] debugfs: Add stub function for debugfs_create_automount().
> 
> Add stub for debugfs_create_automount() for when debugfs is not configured
> in.
> 
> Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com>
> ---
>  include/linux/debugfs.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> 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);
> +}
> +
>  static inline void debugfs_remove(struct dentry *dentry)
>  { }
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

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);
+}
+
 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");
 	}
 
+	tr->dir = TRACE_TOP_DIR_ENTRY;
+
 	return NULL;
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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;
>  }
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Wed, Nov 04, 2015 at 10:03:39AM -0500, Steven Rostedt wrote:
> 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.

The debugfs change should be split out into a separate patch, which I'll
be glad to take through my tree, it isn't dependant on the tracing
code at all.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Wed, 4 Nov 2015 10:54:42 -0800
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Wed, Nov 04, 2015 at 10:03:39AM -0500, Steven Rostedt wrote:
> > 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.
> 
> The debugfs change should be split out into a separate patch, which I'll
> be glad to take through my tree, it isn't dependant on the tracing
> code at all.
> 

I guess the tracing code is dependent on that change. As the tracing
code will then be compiled without the select DEBUGFS, and will fail to
compile if that change is not there.

That said. I think it's best to split it up, and have the debugfs
change go through your tree, and the other part go through mine without
the remove of the "select DEBUGFS". After both are in mainline, then we
can remove the select statement.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Wed, Nov 04, 2015 at 10:03:39AM -0500, Steven Rostedt wrote:
> 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.
Do you mean return ERR_PTR(-ENOMEM); when debugfs_create_automount()
return NULL?
As long as tracefs is initialized, we can make tracing_init_dentry() return
NULL even if the debugfs automount point is not created(), and tracefs can
still be populated. If tracing_init_dentry() returns error in this case,
the caller of tracing_init_dentry() will not populate tracefs.
> 
> >  	}
> >  
> > +	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.
If we accept debugfs_create_automount() return NULL, tr->dir can still
be NULL if we do tr->dir = debugfs_create_automount().
> 
> -- Steve
> 
> >  	return NULL;
> >  }
> >  
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Thu, 5 Nov 2015 13:23:01 +0800
Jiaxing Wang <hello.wjx@gmail.com> wrote:

> > > -	/*
> > > -	 * 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.
> Do you mean return ERR_PTR(-ENOMEM); when debugfs_create_automount()
> return NULL?

Right.

> As long as tracefs is initialized, we can make tracing_init_dentry() return
> NULL even if the debugfs automount point is not created(), and tracefs can
> still be populated. If tracing_init_dentry() returns error in this case,
> the caller of tracing_init_dentry() will not populate tracefs.

But this is still a failure. tracing_init_dentry() now only mounts
tracefs on the debugfs/tracing directory. If it fails to do that when
debugfs is available, then it should fail, as it would break backward
compatibility with other tools.

If debugfs is not configured in, then it should set tr->dir to
whatever (ENOMEM is fine), and return NULL.


> > 
> > >  	}
> > >  
> > > +	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.
> If we accept debugfs_create_automount() return NULL, tr->dir can still
> be NULL if we do tr->dir = debugfs_create_automount().

What's wrong with that? This function is only to automount debugfs now.

Also, I think the first check should be:

	if (WARN_ON(!tracefs_initialized()) ||
	    (IS_ENABLED(CONFIG_DEBUGFS) &&
	     WARN_ON(!debugfs_initialized()))
		return ERR_PTR(-ENODEV);

Then we don't need the if (debugfs_initialized()) conditional.

-- Steve


> > 
> > -- Steve
> > 
> > >  	return NULL;
> > >  }
> > >  
> > 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1263819 — [PATCH] tracing: Make tracing work when debugfs is not configured in

Currently tracing_init_dentry() returns -ENODEV when debugfs is not
configured in, 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 if debugfs is not
configured in and can manually mount tracefs. But return -ENODEV
if debugfs is configured in but not initialized or failed to create
automount point as that would break backward compatibility with older
tools.

Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com>
---
 kernel/trace/trace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6e79408..c89296f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6721,7 +6721,9 @@ struct dentry *tracing_init_dentry(void)
 	if (tr->dir)
 		return NULL;
 
-	if (WARN_ON(!debugfs_initialized()))
+	if (WARN_ON(!tracefs_initialized()) ||
+		(IS_ENABLED(CONFIG_DEBUG_FS) &&
+		 WARN_ON(!debugfs_initialized())))
 		return ERR_PTR(-ENODEV);
 
 	/*
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

On Thu, Nov 05, 2015 at 10:54:32AM -0500, Steven Rostedt wrote:
> On Thu, 5 Nov 2015 13:23:01 +0800
> Jiaxing Wang <hello.wjx@gmail.com> wrote:
> 
> > > > -	/*
> > > > -	 * 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.
> > Do you mean return ERR_PTR(-ENOMEM); when debugfs_create_automount()
> > return NULL?
> 
> Right.
> 
> > As long as tracefs is initialized, we can make tracing_init_dentry() return
> > NULL even if the debugfs automount point is not created(), and tracefs can
> > still be populated. If tracing_init_dentry() returns error in this case,
> > the caller of tracing_init_dentry() will not populate tracefs.
> 
> But this is still a failure. tracing_init_dentry() now only mounts
> tracefs on the debugfs/tracing directory. If it fails to do that when
> debugfs is available, then it should fail, as it would break backward
> compatibility with other tools.
> 
> If debugfs is not configured in, then it should set tr->dir to
> whatever (ENOMEM is fine), and return NULL.
> 
> 
> > > 
> > > >  	}
> > > >  
> > > > +	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.
> > If we accept debugfs_create_automount() return NULL, tr->dir can still
> > be NULL if we do tr->dir = debugfs_create_automount().
> 
> What's wrong with that? This function is only to automount debugfs now.
> 
> Also, I think the first check should be:
> 
> 	if (WARN_ON(!tracefs_initialized()) ||
> 	    (IS_ENABLED(CONFIG_DEBUGFS) &&
> 	     WARN_ON(!debugfs_initialized()))
> 		return ERR_PTR(-ENODEV);
> 
> Then we don't need the if (debugfs_initialized()) conditional.

I will send you a new patch according to your suggestion, and if that
is OK, I will send a seperate patch to Greg KH to add stub for
debugfs_create_automount().

Thanks.
> 
> -- Steve
> 
> 
> > > 
> > > -- Steve
> > > 
> > > >  	return NULL;
> > > >  }
> > > >  
> > > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web