LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 00/16 v3] tracing: Add new file system tracefs
Date: Mon, 26 Jan 2015 15:42:59 -0500	[thread overview]
Message-ID: <20150126154259.5e650d32@gandalf.local.home> (raw)
In-Reply-To: <20150126193049.GN29656@ZenIV.linux.org.uk>

On Mon, 26 Jan 2015 19:30:49 +0000
Al Viro <viro@ZenIV.linux.org.uk> wrote:

> You are still fighting an inconvenient API, but now it's not debugfs one -
> it's your copy thereof.  Why not give your instances/ an inode_operations
> of its own?  One with ->mkdir() and ->rmdir(), leaving all other directories
> as-is.  That way you don't need the secondary methods at all.  And sure,
> debugfs_create_dir() grabs ->i_mutex on parent, making you drop that in
> your ->mkdir() if you want to call it.  But now you are not talking to it -
> just to your own code, where you are free to change the calling conventions,
> making it caller's responsibility to get that ->i_mutex.  The same goes for
> the rmdir side...

The vfs layer grabs the i_mutex, which needs to be dropped. I was
hoping to keep the dropping of the i_mutex within the tracefs code and
not have the tracing code worry about it. I still will need to add
i_private to hold this info.

But that said, I could clean it up a little be having a
tracefs_create_instance_dir(), which does all the work within the
tracefs code. The caller would pass in the functions to be called by
that.

Like this patch applied on top of my current series:

-- Steve

diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index c7eb4c58579e..13e17bf677f4 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -50,6 +50,11 @@ static const struct file_operations tracefs_file_operations = {
 	.llseek =	noop_llseek,
 };
 
+struct tracefs_dir_ops {
+	int (*mkdir)(const char *name);
+	int (*rmdir)(const char *name);
+};
+
 static char *get_dname(struct dentry *dentry)
 {
 	const char *dname;
@@ -67,13 +72,10 @@ static char *get_dname(struct dentry *dentry)
 
 static int tracefs_syscall_mkdir(struct inode *inode, struct dentry *dentry, umode_t mode)
 {
-	const struct tracefs_dir_ops *ops = inode ? inode->i_private : NULL;
+	const struct tracefs_dir_ops *ops = inode->i_private;
 	char *name;
 	int ret;
 
-	if (!ops)
-		return -EPERM;
-
 	name = get_dname(dentry);
 	if (!name)
 		return -ENOMEM;
@@ -98,9 +100,6 @@ static int tracefs_syscall_rmdir(struct inode *inode, struct dentry *dentry)
 	char *name;
 	int ret;
 
-	if (!ops)
-		return -EPERM;
-
 	name = get_dname(dentry);
 	if (!name)
 		return -ENOMEM;
@@ -317,16 +316,6 @@ static struct file_system_type trace_fs_type = {
 };
 MODULE_ALIAS_FS("tracefs");
 
-void tracefs_add_dir_ops(struct dentry *dentry, const struct tracefs_dir_ops *ops)
-{
-	struct inode *inode = dentry->d_inode;
-
-	if (!inode)
-		return;
-
-	inode->i_private = (void *)ops;
-}
-
 static struct dentry *start_creating(const char *name, struct dentry *parent)
 {
 	struct dentry *dentry;
@@ -425,6 +414,31 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode,
 	return end_creating(dentry);
 }
 
+static struct dentry *__create_dir(const char *name, struct dentry *parent,
+				   const struct inode_operations *ops)
+{
+	struct dentry *dentry = start_creating(name, parent);
+	struct inode *inode;
+
+	if (IS_ERR(dentry))
+		return NULL;
+
+	inode = tracefs_get_inode(dentry->d_sb);
+	if (unlikely(!inode))
+		return failed_creating(dentry);
+
+	inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
+	inode->i_op = ops;
+	inode->i_fop = &simple_dir_operations;
+
+	/* directory inodes start off with i_nlink == 2 (for "." entry) */
+	inc_nlink(inode);
+	d_instantiate(dentry, inode);
+	inc_nlink(dentry->d_parent->d_inode);
+	fsnotify_mkdir(dentry->d_parent->d_inode, dentry);
+	return end_creating(dentry);
+}
+
 /**
  * tracefs_create_dir - create a directory in the tracefs filesystem
  * @name: a pointer to a string containing the name of the directory to
@@ -444,26 +458,30 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode,
  */
 struct dentry *tracefs_create_dir(const char *name, struct dentry *parent)
 {
-	struct dentry *dentry = start_creating(name, parent);
-	struct inode *inode;
+	return __create_dir(name, parent, &simple_dir_inode_operations);
+}
 
-	if (IS_ERR(dentry))
+struct dentry *tracefs_create_instance_dir(const char *name, struct dentry *parent,
+					  int (*mkdir)(const char *name),
+					  int (*rmdir)(const char *name))
+{
+	struct tracefs_dir_ops *dir_ops;
+	struct dentry *dentry;
+
+	dir_ops = kmalloc(sizeof(*dir_ops), GFP_KERNEL);
+	if (!dir_ops)
 		return NULL;
 
-	inode = tracefs_get_inode(dentry->d_sb);
-	if (unlikely(!inode))
-		return failed_creating(dentry);
+	dir_ops->mkdir = mkdir;
+	dir_ops->rmdir = mkdir;
 
-	inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
-	inode->i_op = &tracefs_dir_inode_operations;
-	inode->i_fop = &simple_dir_operations;
+	dentry = __create_dir(name, parent, &tracefs_dir_inode_operations);
+	if (!dentry)
+		return NULL;
 
-	/* directory inodes start off with i_nlink == 2 (for "." entry) */
-	inc_nlink(inode);
-	d_instantiate(dentry, inode);
-	inc_nlink(dentry->d_parent->d_inode);
-	fsnotify_mkdir(dentry->d_parent->d_inode, dentry);
-	return end_creating(dentry);
+	dentry->d_inode->i_private = (void *)dir_ops;
+
+	return dentry;
 }
 
 static inline int tracefs_positive(struct dentry *dentry)
diff --git a/include/linux/tracefs.h b/include/linux/tracefs.h
index d142b1f9d453..5b727a17beee 100644
--- a/include/linux/tracefs.h
+++ b/include/linux/tracefs.h
@@ -34,12 +34,9 @@ struct dentry *tracefs_create_dir(const char *name, struct dentry *parent);
 void tracefs_remove(struct dentry *dentry);
 void tracefs_remove_recursive(struct dentry *dentry);
 
-struct tracefs_dir_ops {
-	int (*mkdir)(const char *name);
-	int (*rmdir)(const char *name);
-};
-
-void tracefs_add_dir_ops(struct dentry *dentry, const struct tracefs_dir_ops *ops);
+struct dentry *tracefs_create_instance_dir(const char *name, struct dentry *parent,
+					   int (*mkdir)(const char *name),
+					   int (*rmdir)(const char *name));
 
 bool tracefs_initialized(void);
 
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c555027c8aaf..2047501dcd16 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -6445,18 +6445,13 @@ static int instance_rmdir(const char *name)
 	return ret;
 }
 
-static const struct tracefs_dir_ops instance_dir_ops = {
-	.mkdir		= instance_mkdir,
-	.rmdir		= instance_rmdir,
-};
-
 static __init void create_trace_instances(struct dentry *d_tracer)
 {
-	trace_instance_dir = tracefs_create_dir("instances", d_tracer);
+	trace_instance_dir = tracefs_create_instance_dir("instances", d_tracer,
+							 instance_mkdir,
+							 instance_rmdir);
 	if (WARN_ON(!trace_instance_dir))
 		return;
-
-	tracefs_add_dir_ops(trace_instance_dir, &instance_dir_ops);
 }
 
 static void

  reply	other threads:[~2015-01-26 20:43 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-26 15:09 Steven Rostedt
2015-01-26 15:09 ` [PATCH 01/16 v3] debugfs_{mkdir,create,link}(): get rid of redundant argument Steven Rostedt
2015-01-26 15:09 ` [PATCH 02/16 v3] debugfs: split the beginning and the end of __create_file() off Steven Rostedt
2015-01-26 15:09 ` [PATCH 03/16 v3] debugfs: kill __create_file() Steven Rostedt
2015-01-26 15:09 ` [PATCH 04/16 v3] fold debugfs_link() into caller Steven Rostedt
2015-01-26 15:09 ` [PATCH 05/16 v3] debugfs_mknod(): get rid useless arguments Steven Rostedt
2015-01-26 15:09 ` [PATCH 06/16 v3] fold debugfs_mkdir() into caller Steven Rostedt
2015-01-26 15:09 ` [PATCH 07/16 v3] fold debugfs_create() " Steven Rostedt
2015-01-26 15:09 ` [PATCH 08/16 v3] fold debugfs_mknod() into callers Steven Rostedt
2015-01-26 15:09 ` [PATCH 09/16 v3] debugfs: take mode-dependent parts of debugfs_get_inode() " Steven Rostedt
2015-01-26 15:09 ` [PATCH 10/16 v3] debugfs: split end_creating() into success and failure cases Steven Rostedt
2015-01-26 15:09 ` [PATCH 11/16 v3] new primitive: debugfs_create_automount() Steven Rostedt
2015-01-26 15:09 ` [PATCH 12/16 v3] tracefs: Add new tracefs file system Steven Rostedt
2015-01-26 15:09 ` [PATCH 13/16 v3] tracing: Convert the tracing facility over to use tracefs Steven Rostedt
2015-01-26 15:09 ` [PATCH 14/16 v3] tracing: Automatically mount tracefs on debugfs/tracing Steven Rostedt
2015-01-26 15:09 ` [PATCH 15/16 v3] tracefs: Add directory /sys/kernel/tracing Steven Rostedt
2015-01-26 15:09 ` [PATCH 16/16 v3] tracing: Have mkdir and rmdir be part of tracefs Steven Rostedt
2015-01-26 19:30 ` [PATCH 00/16 v3] tracing: Add new file system tracefs Al Viro
2015-01-26 20:42   ` Steven Rostedt [this message]
2015-01-26 20:44     ` Steven Rostedt
2015-01-26 20:58       ` Steven Rostedt
2015-01-26 21:44         ` Al Viro
2015-01-26 21:46     ` Al Viro
2015-01-26 23:02       ` Al Viro
2015-01-26 23:39         ` Steven Rostedt
2015-01-26 23:43       ` Steven Rostedt
2015-01-26 23:49         ` Steven Rostedt
2015-01-27  0:39           ` Steven Rostedt
2015-01-27  4:34             ` Al Viro
2015-01-27 15:15               ` Steven Rostedt
2015-01-27  0:37         ` Al Viro
2015-01-27  1:02           ` Steven Rostedt
2015-01-27  1:03             ` Steven Rostedt
2015-01-27  4:38               ` Al Viro
2015-01-27 15:19                 ` Steven Rostedt
2015-01-27 15:44                   ` Steven Rostedt
2015-01-26 21:40   ` Al Viro
2015-01-26 23:37     ` Steven Rostedt
2015-01-27  2:34       ` Steven Rostedt
2015-01-27  4:46         ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150126154259.5e650d32@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    --subject='Re: [PATCH 00/16 v3] tracing: Add new file system tracefs' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).