[PATCH 0/3] Some tracing fixes.

[PATCH 0/3] Some tracing fixes.

[Jiaxing Wang]

In short:
  The first patch update instance_rmdir() to use tracefs_remove_recursive.
  The second one make tracing work when debugfs is not initialized, currently
we get an empty directory after mounting tracefs manually.
  The third one makes it possible to specify tracer specific options from
kernel parameter, like the following:
  
  ftrace=function ftrace_filter=kfree trace_options=func_stack_trace

Jiaxing Wang (3):
  tracing: Update instance_rmdir() to use tracefs_remove_recursive.
  tracing: Make tracing work when debugfs is not compiled or
    initialized.
  tracing: Apply tracer specific options from kernel command line.

 kernel/trace/Kconfig |  1 -
 kernel/trace/trace.c | 72 +++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 52 insertions(+), 21 deletions(-)

-- 
2.1.4

[PATCH 1/3] tracing: Update instance_rmdir() to use tracefs_remove_recursive.

[Jiaxing Wang]

Update instancd_rmdir to use tracefs_remove_recursive instead of
debugfs_remove_recursive.This was left in the transition from debugfs
to tracefs.

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

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6e79408..69f9754 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6602,7 +6602,7 @@ static int instance_rmdir(const char *name)
 	tracing_set_nop(tr);
 	event_trace_del_tracer(tr);
 	ftrace_destroy_function_files(tr);
-	debugfs_remove_recursive(tr->dir);
+	tracefs_remove_recursive(tr->dir);
 	free_trace_buffers(tr);
 
 	kfree(tr->name);
-- 
2.1.4

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

[Jiaxing Wang]

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().

Signed-off-by: Jiaxing Wang <hello.wjx@gmail.com>
---
 kernel/trace/Kconfig |  1 -
 kernel/trace/trace.c | 29 +++++++++++++++++------------
 2 files changed, 17 insertions(+), 13 deletions(-)

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 69f9754..2d3042f 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

[PATCH 3/3] tracing: Apply tracer specific options from kernel command line.

[Jiaxing Wang]

Currently, the trace_options parameter is only applied in
tracer_alloc_buffers() when global_trace.current_trace is nop_trace,
so a tracer specific option will not be applied even when the specific
tracer is also enabled from kernel command line. For example, the
'func_stack_trace' option can't be enabled with the following kernel
parameter:

  ftrace=function ftrace_filter=kfree trace_options=func_stack_trace

We can enable tracer specific options by simply apply the options again
if the specific tracer is also supplied from command line and started
in register_tracer().

To keep trace_boot_options_buf from overwritten by strsep() and strstrip()
and can be parsed again, a copy is made for them to work on.

Also make register_tracer() be __init to access the __init data, and
in fact register_tracer is only called from __init code.

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

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2d3042f..d42af59 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1204,13 +1204,15 @@ static inline int run_tracer_selftest(struct tracer *type)
 }
 #endif /* CONFIG_FTRACE_STARTUP_TEST */
 
+static int __init apply_trace_boot_options(void);
+
 /**
  * register_tracer - register a tracer with the ftrace system.
  * @type - the plugin for the tracer
  *
  * Register a new plugin tracer.
  */
-int register_tracer(struct tracer *type)
+int __init register_tracer(struct tracer *type)
 {
 	struct tracer *t;
 	int ret = 0;
@@ -1268,6 +1270,9 @@ int register_tracer(struct tracer *type)
 	/* Do we want this tracer to start on bootup? */
 	tracing_set_tracer(&global_trace, type->name);
 	default_bootup_tracer = NULL;
+
+	apply_trace_boot_options();
+
 	/* disable other selftests, since this will break it. */
 	tracing_selftest_disabled = true;
 #ifdef CONFIG_FTRACE_STARTUP_TEST
@@ -3603,6 +3608,33 @@ static int trace_set_options(struct trace_array *tr, char *option)
 	return ret;
 }
 
+static int __init apply_trace_boot_options(void)
+{
+	char *option;
+	char *buf;
+	char *str;
+	size_t len;
+
+	if (trace_boot_options) {
+		len = strlen(trace_boot_options);
+
+		buf = str = kmalloc(len + 1, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+
+		memcpy(buf, trace_boot_options, len + 1);
+
+		while (str) {
+			option = strsep(&str, ",");
+			trace_set_options(&global_trace, option);
+		}
+
+		kfree(buf);
+	}
+
+	return 0;
+}
+
 static ssize_t
 tracing_trace_options_write(struct file *filp, const char __user *ubuf,
 			size_t cnt, loff_t *ppos)
@@ -7153,12 +7185,7 @@ __init static int tracer_alloc_buffers(void)
 	INIT_LIST_HEAD(&global_trace.events);
 	list_add(&global_trace.list, &ftrace_trace_arrays);
 
-	while (trace_boot_options) {
-		char *option;
-
-		option = strsep(&trace_boot_options, ",");
-		trace_set_options(&global_trace, option);
-	}
+	apply_trace_boot_options();
 
 	register_snapshot_cmd();
 
-- 
2.1.4

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]

Hi Jiaxing,

[auto build test ERROR on tip/perf/core -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Jiaxing-Wang/Some-tracing-fixes/20151018-200252
config: arm-omap2plus_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All error/warnings (new ones prefixed by >>):

   kernel/trace/trace.c: In function 'tracing_init_dentry':
>> kernel/trace/trace.c:6737:3: error: implicit declaration of function 'debugfs_create_automount' [-Werror=implicit-function-declaration]
      traced = debugfs_create_automount("tracing", NULL,
      ^
>> kernel/trace/trace.c:6737:10: warning: assignment makes pointer from integer without a cast
      traced = debugfs_create_automount("tracing", NULL,
             ^
   cc1: some warnings being treated as errors

vim +/debugfs_create_automount +6737 kernel/trace/trace.c

  6731			/*
  6732			 * As there may still be users that expect the tracing
  6733			 * files to exist in debugfs/tracing, we must automount
  6734			 * the tracefs file system there, so older tools still
  6735			 * work with the newer kerenl.
  6736			 */
> 6737			traced = debugfs_create_automount("tracing", NULL,
  6738							   trace_automount, NULL);
  6739			if (!traced)
  6740				pr_warn_once("Could not create debugfs directory 'tracing'\n");

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24643 bytes --]

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

[Jiaxing Wang]

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

>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)
 { }
 
-- 
2.1.4

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

[Steven Rostedt]

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)
>  { }
>  

Re: [PATCH 3/3] tracing: Apply tracer specific options from kernel command line.

[Steven Rostedt]

On Sun, 18 Oct 2015 19:58:10 +0800
Jiaxing Wang <hello.wjx@gmail.com> wrote:


> +static int __init apply_trace_boot_options(void)
> +{
> +	char *option;
> +	char *buf;
> +	char *str;
> +	size_t len;
> +
> +	if (trace_boot_options) {
> +		len = strlen(trace_boot_options);
> +
> +		buf = str = kmalloc(len + 1, GFP_KERNEL);
> +		if (!buf)
> +			return -ENOMEM;
> +
> +		memcpy(buf, trace_boot_options, len + 1);
> +
> +		while (str) {
> +			option = strsep(&str, ",");
> +			trace_set_options(&global_trace, option);
> +		}
> +
> +		kfree(buf);

Instead of allocating a buffer, do what I do in early_enable_events()
(see trace_events.c), where I put back the ',' after processing.

-- Steve

> +	}
> +
> +	return 0;
> +}
> +
>  static ssize_t
>  tracing_trace_options_write(struct file *filp, const char __user *ubuf,
>  			size_t cnt, loff_t *ppos)
> @@ -7153,12 +7185,7 @@ __init static int tracer_alloc_buffers(void)
>  	INIT_LIST_HEAD(&global_trace.events);
>  	list_add(&global_trace.list, &ftrace_trace_arrays);
>  
> -	while (trace_boot_options) {
> -		char *option;
> -
> -		option = strsep(&trace_boot_options, ",");
> -		trace_set_options(&global_trace, option);
> -	}
> +	apply_trace_boot_options();
>  
>  	register_snapshot_cmd();
>  

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

[Jiaxing Wang]

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

[PATCH v2] tracing: Apply tracer specific options from kernel command line.

[Jiaxing Wang]

Currently, the trace_options parameter is only applied in
tracer_alloc_buffers() when global_trace.current_trace is nop_trace,
so a tracer specific option will not be applied even when the specific
tracer is also enabled from kernel command line. For example, the
'func_stack_trace' option can't be enabled with the following kernel
parameter:

  ftrace=function ftrace_filter=kfree trace_options=func_stack_trace

We can enable tracer specific options by simply apply the options again
if the specific tracer is also supplied from command line and started
in register_tracer().

To make trace_boot_options_buf can be parsed again, a comma and a space
is put back if they were replaced by strsep and strstrip respectively.

Also make register_tracer() be __init to access the __init data, and
in fact register_tracer is only called from __init code.

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

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 6dd064e..689886c 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -214,12 +214,10 @@ __setup("alloc_snapshot", boot_alloc_snapshot);
 
 
 static char trace_boot_options_buf[MAX_TRACER_SIZE] __initdata;
-static char *trace_boot_options __initdata;
 
 static int __init set_trace_boot_options(char *str)
 {
 	strlcpy(trace_boot_options_buf, str, MAX_TRACER_SIZE);
-	trace_boot_options = trace_boot_options_buf;
 	return 0;
 }
 __setup("trace_options=", set_trace_boot_options);
@@ -1204,13 +1202,15 @@ static inline int run_tracer_selftest(struct tracer *type)
 }
 #endif /* CONFIG_FTRACE_STARTUP_TEST */
 
+static void __init apply_trace_boot_options(void);
+
 /**
  * register_tracer - register a tracer with the ftrace system.
  * @type - the plugin for the tracer
  *
  * Register a new plugin tracer.
  */
-int register_tracer(struct tracer *type)
+int __init register_tracer(struct tracer *type)
 {
 	struct tracer *t;
 	int ret = 0;
@@ -1268,6 +1268,9 @@ int register_tracer(struct tracer *type)
 	/* Do we want this tracer to start on bootup? */
 	tracing_set_tracer(&global_trace, type->name);
 	default_bootup_tracer = NULL;
+
+	apply_trace_boot_options();
+
 	/* disable other selftests, since this will break it. */
 	tracing_selftest_disabled = true;
 #ifdef CONFIG_FTRACE_STARTUP_TEST
@@ -3577,6 +3580,7 @@ static int trace_set_options(struct trace_array *tr, char *option)
 	int neg = 0;
 	int ret = -ENODEV;
 	int i;
+	size_t orig_len = strlen(option);
 
 	cmp = strstrip(option);
 
@@ -3600,9 +3604,37 @@ static int trace_set_options(struct trace_array *tr, char *option)
 
 	mutex_unlock(&trace_types_lock);
 
+	/*
+	 * If the first trailing whitespace is replaced with '\0' by strstrip,
+	 * turn it back into a space.
+	 */
+	if (orig_len > strlen(option))
+		option[strlen(option)] = ' ';
+
 	return ret;
 }
 
+static void __init apply_trace_boot_options(void)
+{
+	char *buf = trace_boot_options_buf;
+	char *option;
+
+	while (true) {
+		option = strsep(&buf, ",");
+
+		if (!option)
+			break;
+		if (!*option)
+			continue;
+
+		trace_set_options(&global_trace, option);
+
+		/* Put back the comma to allow this to be called again */
+		if (buf)
+			*(buf - 1) = ',';
+	}
+}
+
 static ssize_t
 tracing_trace_options_write(struct file *filp, const char __user *ubuf,
 			size_t cnt, loff_t *ppos)
@@ -7153,12 +7185,7 @@ __init static int tracer_alloc_buffers(void)
 	INIT_LIST_HEAD(&global_trace.events);
 	list_add(&global_trace.list, &ftrace_trace_arrays);
 
-	while (trace_boot_options) {
-		char *option;
-
-		option = strsep(&trace_boot_options, ",");
-		trace_set_options(&global_trace, option);
-	}
+	apply_trace_boot_options();
 
 	register_snapshot_cmd();
 
-- 
2.1.4

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

[Steven Rostedt]

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

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

[Greg Kroah-Hartman]

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

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

[Steven Rostedt]

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

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

[Jiaxing Wang]

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

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

[Steven Rostedt]

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

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

[Jiaxing Wang]

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/

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

[Jiaxing Wang]

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