Some tracing fixes.

From b3b877d8d9fd9795ea1055042039a272e47f4dc5 Mon Sep 17 00:00:00 2001

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

Quoted 2 lines

From b3b877d8d9fd9795ea1055042039a272e47f4dc5 Mon Sep 17 00:00:00 2001

From: Jiaxing Wang <[email protected]>
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 <[email protected]>

---
 include/linux/debugfs.h | 8 ++++++++
 1 file changed, 8 insertions(+)

include/linux/debugfs.h +8 -0

index 9beb636..b42ef88 100644
@@ -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)
 { }
 

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 <[email protected]>

---
 include/linux/debugfs.h |  8 ++++++++
 kernel/trace/Kconfig    |  1 -
 kernel/trace/trace.c    | 29 +++++++++++++++++------------
 3 files changed, 25 insertions(+), 13 deletions(-)

include/linux/debugfs.h +0 -0

index 9beb636..b42ef88 100644

include/linux/debugfs.h +1 -0

+
 static inline void debugfs_remove(struct dentry *dentry)
 { }
 

kernel/trace/Kconfig +0 -1

index 1153c43..59f6377f 100644
@@ -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

kernel/trace/trace.c +5 -12

index 6e79408..6dd064e 100644
@@ -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()) {

kernel/trace/trace.c +0 -0

 	return NULL;
 }
 

Quoted 5 lines · On Wed, 4 Nov 2015 09:11:18 +0800 Jiaxing Wang <[email protected]> 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.

Quoted 8 lines

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.

Quoted 14 lines · Steven Rostedt

Quoted 11 lines · On Wed, 4 Nov 2015 09:11:18 +0800 Jiaxing Wang <[email protected]> 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().

Quoted 2 lines

Quoted 2 lines

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.

Quoted 26 lines · On Wed, 4 Nov 2015 09:11:18 +0800 Jiaxing Wang <[email protected]> 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 <[email protected]>

---
 include/linux/debugfs.h |  8 ++++++++
 kernel/trace/Kconfig    |  1 -
 kernel/trace/trace.c    | 29 +++++++++++++++++------------
 3 files changed, 25 insertions(+), 13 deletions(-)

include/linux/debugfs.h +0 -0

index 9beb636..b42ef88 100644

include/linux/debugfs.h static inline struct dentry *debugfs_create_symlink(const char *name, · line 169

Quoted 10 lines

include/linux/debugfs.h +7 -0

@@ -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.

Quoted 60 lines

include/linux/debugfs.h +1 -0

+
 static inline void debugfs_remove(struct dentry *dentry)
 { }
 

kernel/trace/Kconfig +0 -1

index 1153c43..59f6377f 100644
@@ -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

kernel/trace/trace.c +10 -12

index 6e79408..6dd064e 100644
@@ -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.

kernel/trace/trace.c static struct vfsmount *trace_automount(void *ingore) · line 6740

Quoted 5 lines

kernel/trace/trace.c +5 -0

+		 */
+		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.

kernel/trace/trace.c static struct vfsmount *trace_automount(void *ingore) · line 6744

Quoted 5 lines

kernel/trace/trace.c +2 -0

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

Quoted 3 lines

kernel/trace/trace.c +0 -0

 	return NULL;
 }
 

Quoted 17 lines

Quoted 17 lines

kernel/trace/trace.c +6 -11

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

Quoted 8 lines

kernel/trace/trace.c static struct vfsmount *trace_automount(void *ingore) · line 6740

Quoted 5 lines

kernel/trace/trace.c +5 -0

+		 */
+		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.

Quoted 9 lines

kernel/trace/trace.c static struct vfsmount *trace_automount(void *ingore) · line 6744

Quoted 5 lines

kernel/trace/trace.c +2 -0

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

Quoted 7 lines

-- Steve

Quoted 3 lines

return NULL;
}

Quoted 25 lines · On Thu, 5 Nov 2015 13:23:01 +0800 Jiaxing Wang <[email protected]> wrote:

Quoted 17 lines

Quoted 17 lines

kernel/trace/trace.c +6 -11

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

Quoted 8 lines

kernel/trace/trace.c static struct vfsmount *trace_automount(void *ingore) · line 6740

Quoted 5 lines

kernel/trace/trace.c +5 -0

+		 */
+		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.

Quoted 2 lines

Do you mean return ERR_PTR(-ENOMEM); when debugfs_create_automount() return NULL?

Right.

Quoted 4 lines

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.

Quoted 11 lines

Quoted 9 lines

kernel/trace/trace.c static struct vfsmount *trace_automount(void *ingore) · line 6744

Quoted 5 lines

kernel/trace/trace.c +2 -0

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

Quoted 7 lines

Quoted 7 lines

-- Steve

Quoted 3 lines

return NULL;
}

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

kernel/trace/trace.c +12 -0

+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)

kernel/trace/trace.c +6 -6

+	}
+
+	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();