[PATCHSET driver-core-next] kernfs: implement kernfs_dir_ops

[PATCHSET driver-core-next] kernfs: implement kernfs_dir_ops

[Tejun Heo]

Hello,

kernfs is now properly separated out from sysfs and can be used for
other pseudo filesystems too; however, there are still some missing
features - kernfs filesystems can't process any directory manipulation
systemcalls such as mkdir(2), rmdir(2) and rename(2), which cgroup,
one of the prospect kernfs users, requires for cgroup management.

This patchset implements those directory operation hooks so that
kernfs users can be hooked into those system calls and implement
appropriate actions, which may or may not match the actual semantics
of those syscalls.

The implementation is mostly straight forward except for one aspect.
sysfs, and thus kernfs, never allowed negative dentries.  They are
actively shot down and lookup callback assumes that there's no
negative dentries.  This doesn't really have a lot of benefits and
changing isn't difficult.  The lookup path simply needs to tell the
vfs layer that a full fresh lookup is necessary for negative dentries.

The patchset is lightly tested with sample module which implements
"test" filesystem with weird behaviors using kernfs (will be posted as
a reply to this message).  Note that the module needs kernfs APIs to
be exported.  It might be necessary that we might need further changes
to actually convert cgroup to kernfs but I think this should provide
pretty good basis.

This patchset contains the following six patches.

 0001-kernfs-add-mode-to-kernfs_create_dir-_ns.patch
 0002-kernfs-add-REMOVED-check-to-create-and-rename-paths.patch
 0003-kernfs-mark-static-names-with-KERNFS_STATIC_NAME.patch
 0004-kernfs-update-kernfs_rename_ns-to-consider-KERNFS_ST.patch
 0005-kernfs-allow-negative-dentries.patch
 0006-kernfs-add-kernfs_dir_ops.patch

0001-0004 are prepatory patches.

0005 updates kernfs to allow negative dentries.

0006 implements kernfs_dir_ops.

This patchset is based on

  driver-core-next a8b1c0193602 ("Documentation: start documenting driver design patterns")
+ [1] [PATCHSET] kernfs: s/sysfs/kernfs/

and also available in the following git branch.

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git review-kernfs-dir_ops

diffstat follows.

 fs/kernfs/dir.c             |  111 ++++++++++++++++++++++++++++++--------------
 fs/kernfs/file.c            |   23 +++++----
 fs/kernfs/kernfs-internal.h |    2
 fs/sysfs/dir.c              |    3 -
 fs/sysfs/file.c             |    4 -
 fs/sysfs/group.c            |    3 -
 fs/sysfs/mount.c            |    2
 include/linux/kernfs.h      |   61 +++++++++++++++---------
 8 files changed, 140 insertions(+), 69 deletions(-)

Thanks.

--
tejun

[1] http://lkml.kernel.org/g/1386789118-24733-1-git-send-email-tj@kernel.org

[PATCH 1/6] kernfs: add @mode to kernfs_create_dir[_ns]()

[Tejun Heo]

sysfs assumed 0755 for all newly created directories and kernfs
inherited it.  This assumption is unnecessarily restrictive and
inconsistent with kernfs_create_file[_ns]().  This patch adds @mode
parameter to kernfs_create_dir[_ns]() and update uses in sysfs
accordingly.  Among others, this will be useful for implementations of
the planned ->mkdir() method.

This patch doesn't introduce any behavior differences.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 fs/kernfs/dir.c        |  9 +++++----
 fs/sysfs/dir.c         |  3 ++-
 fs/sysfs/group.c       |  3 ++-
 include/linux/kernfs.h | 13 +++++++------
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 6520066..e55bb02 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -639,22 +639,23 @@ void kernfs_destroy_root(struct kernfs_root *root)
  * kernfs_create_dir_ns - create a directory
  * @parent: parent in which to create a new directory
  * @name: name of the new directory
+ * @mode: mode of the new directory
  * @priv: opaque data associated with the new directory
  * @ns: optional namespace tag of the directory
  *
  * Returns the created node on success, ERR_PTR() value on failure.
  */
 struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
-					 const char *name, void *priv,
-					 const void *ns)
+					 const char *name, umode_t mode,
+					 void *priv, const void *ns)
 {
-	umode_t mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
 	struct kernfs_addrm_cxt acxt;
 	struct kernfs_node *kn;
 	int rc;
 
 	/* allocate */
-	kn = kernfs_new_node(kernfs_root(parent), name, mode, KERNFS_DIR);
+	kn = kernfs_new_node(kernfs_root(parent), name, mode | S_IFDIR,
+			     KERNFS_DIR);
 	if (!kn)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index aa00740..ee0d761 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -73,7 +73,8 @@ int sysfs_create_dir_ns(struct kobject *kobj, const void *ns)
 	if (!parent)
 		return -ENOENT;
 
-	kn = kernfs_create_dir_ns(parent, kobject_name(kobj), kobj, ns);
+	kn = kernfs_create_dir_ns(parent, kobject_name(kobj),
+				  S_IRWXU | S_IRUGO | S_IXUGO, kobj, ns);
 	if (IS_ERR(kn)) {
 		if (PTR_ERR(kn) == -EEXIST)
 			sysfs_warn_dup(parent, kobject_name(kobj));
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 4d00d399..6b57938 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -100,7 +100,8 @@ static int internal_create_group(struct kobject *kobj, int update,
 		return -EINVAL;
 	}
 	if (grp->name) {
-		kn = kernfs_create_dir(kobj->sd, grp->name, kobj);
+		kn = kernfs_create_dir(kobj->sd, grp->name,
+				       S_IRWXU | S_IRUGO | S_IXUGO, kobj);
 		if (IS_ERR(kn)) {
 			if (PTR_ERR(kn) == -EEXIST)
 				sysfs_warn_dup(kobj->sd, grp->name);
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index e9c4e3a..0ca2aed 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -210,8 +210,8 @@ struct kernfs_root *kernfs_create_root(void *priv);
 void kernfs_destroy_root(struct kernfs_root *root);
 
 struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
-					 const char *name, void *priv,
-					 const void *ns);
+					 const char *name, umode_t mode,
+					 void *priv, const void *ns);
 struct kernfs_node *kernfs_create_file_ns_key(struct kernfs_node *parent,
 					      const char *name,
 					      umode_t mode, loff_t size,
@@ -260,8 +260,8 @@ static inline struct kernfs_root *kernfs_create_root(void *priv)
 static inline void kernfs_destroy_root(struct kernfs_root *root) { }
 
 static inline struct kernfs_node *
-kernfs_create_dir_ns(struct kernfs_node *parent, const char *name, void *priv,
-		     const void *ns)
+kernfs_create_dir_ns(struct kernfs_node *parent, const char *name,
+		     umode_t mode, void *priv, const void *ns)
 { return ERR_PTR(-ENOSYS); }
 
 static inline struct kernfs_node *
@@ -314,9 +314,10 @@ kernfs_find_and_get(struct kernfs_node *kn, const char *name)
 }
 
 static inline struct kernfs_node *
-kernfs_create_dir(struct kernfs_node *parent, const char *name, void *priv)
+kernfs_create_dir(struct kernfs_node *parent, const char *name, umode_t mode,
+		  void *priv)
 {
-	return kernfs_create_dir_ns(parent, name, priv, NULL);
+	return kernfs_create_dir_ns(parent, name, mode, priv, NULL);
 }
 
 static inline struct kernfs_node *
-- 
1.8.4.2

[PATCH 2/6] kernfs: add REMOVED check to create and rename paths

[Tejun Heo]

kernfs currently assumes that the caller doesn't try to create a new
node under a removed parent, rename a removed node, or move a node
under a removed node.  While this works fine for sysfs, it'd be nice
to have protection against such cases especially given that kernfs is
planned to add support for mkdir, rmdir and rename requsts from
userland which may make race conditions more likely.

This patch updates create and rename paths to check REMOVED and fail
the operation with -ENOENT if performed on or towards removed nodes.
Note that remove path already has such check.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 fs/kernfs/dir.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index e55bb02..ba5f372 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -430,6 +430,9 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
 	if (kernfs_type(parent) != KERNFS_DIR)
 		return -EINVAL;
 
+	if (parent->flags & KERNFS_REMOVED)
+		return -ENOENT;
+
 	kn->hash = kernfs_name_hash(kn->name, kn->ns);
 	kn->parent = parent;
 	kernfs_get(parent);
@@ -863,6 +866,10 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent,
 
 	mutex_lock(&kernfs_mutex);
 
+	error = -ENOENT;
+	if ((kn->flags | new_parent->flags) & KERNFS_REMOVED)
+		goto out;
+
 	error = 0;
 	if ((kn->parent == new_parent) && (kn->ns == new_ns) &&
 	    (strcmp(kn->name, new_name) == 0))
-- 
1.8.4.2

[PATCH 3/6] kernfs: mark static names with KERNFS_STATIC_NAME

[Tejun Heo]

Because sysfs used struct attribute which are supposed to stay
constant, sysfs didn't copy names when creating regular files.  The
specified string for name was supposed to stay constant.  Such
distinction isn't inherent for kernfs.  kernfs_create_file[_ns]()
should be able to take the same @name as kernfs_create_dir[_ns]()

As there can be huge number of sysfs attributes, we still want to be
able to use static names for sysfs attributes.  This patch renames
kernfs_create_file_ns_key() to __kernfs_create_file() and adds
@name_is_static parameter so that the caller can explicitly indicate
that @name can be used without copying.  kernfs is updated to use
KERNFS_STATIC_NAME to distinguish static and copied names.

This patch doesn't introduce any behavior changes.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 fs/kernfs/dir.c             |  8 ++++----
 fs/kernfs/file.c            | 23 +++++++++++++++--------
 fs/kernfs/kernfs-internal.h |  2 +-
 fs/sysfs/file.c             |  4 ++--
 include/linux/kernfs.h      | 27 ++++++++++++++-------------
 5 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index ba5f372..e168177 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -240,7 +240,7 @@ void kernfs_put(struct kernfs_node *kn)
 
 	if (kernfs_type(kn) == KERNFS_LINK)
 		kernfs_put(kn->symlink.target_kn);
-	if (kernfs_type(kn) & KERNFS_COPY_NAME)
+	if (!(kn->flags & KERNFS_STATIC_NAME))
 		kfree(kn->name);
 	if (kn->iattr) {
 		if (kn->iattr->ia_secdata)
@@ -336,13 +336,13 @@ const struct dentry_operations kernfs_dops = {
 };
 
 struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
-				    umode_t mode, int type)
+				    umode_t mode, unsigned flags)
 {
 	char *dup_name = NULL;
 	struct kernfs_node *kn;
 	int ret;
 
-	if (type & KERNFS_COPY_NAME) {
+	if (!(flags & KERNFS_STATIC_NAME)) {
 		name = dup_name = kstrdup(name, GFP_KERNEL);
 		if (!name)
 			return NULL;
@@ -362,7 +362,7 @@ struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
 
 	kn->name = name;
 	kn->mode = mode;
-	kn->flags = type | KERNFS_REMOVED;
+	kn->flags = flags | KERNFS_REMOVED;
 
 	return kn;
 
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 053cfd9..316604c 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -762,7 +762,7 @@ const struct file_operations kernfs_file_fops = {
 };
 
 /**
- * kernfs_create_file_ns_key - create a file
+ * __kernfs_create_file - kernfs internal function to create a file
  * @parent: directory to create the file in
  * @name: name of the file
  * @mode: mode of the file
@@ -770,23 +770,30 @@ const struct file_operations kernfs_file_fops = {
  * @ops: kernfs operations for the file
  * @priv: private data for the file
  * @ns: optional namespace tag of the file
+ * @static_name: don't copy file name
  * @key: lockdep key for the file's active_ref, %NULL to disable lockdep
  *
  * Returns the created node on success, ERR_PTR() value on error.
  */
-struct kernfs_node *kernfs_create_file_ns_key(struct kernfs_node *parent,
-					      const char *name,
-					      umode_t mode, loff_t size,
-					      const struct kernfs_ops *ops,
-					      void *priv, const void *ns,
-					      struct lock_class_key *key)
+struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
+					 const char *name,
+					 umode_t mode, loff_t size,
+					 const struct kernfs_ops *ops,
+					 void *priv, const void *ns,
+					 bool name_is_static,
+					 struct lock_class_key *key)
 {
 	struct kernfs_addrm_cxt acxt;
 	struct kernfs_node *kn;
+	unsigned flags;
 	int rc;
 
+	flags = KERNFS_FILE;
+	if (name_is_static)
+		flags |= KERNFS_STATIC_NAME;
+
 	kn = kernfs_new_node(kernfs_root(parent), name,
-			     (mode & S_IALLUGO) | S_IFREG, KERNFS_FILE);
+			     (mode & S_IALLUGO) | S_IFREG, flags);
 	if (!kn)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index a4ff491..c6ba5bc 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -105,7 +105,7 @@ int kernfs_add_one(struct kernfs_addrm_cxt *acxt, struct kernfs_node *kn,
 		   struct kernfs_node *parent);
 void kernfs_addrm_finish(struct kernfs_addrm_cxt *acxt);
 struct kernfs_node *kernfs_new_node(struct kernfs_root *root, const char *name,
-				    umode_t mode, int type);
+				    umode_t mode, unsigned flags);
 
 /*
  * file.c
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index fe6388f..810cf6e 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -252,8 +252,8 @@ int sysfs_add_file_mode_ns(struct kernfs_node *parent,
 	if (!attr->ignore_lockdep)
 		key = attr->key ?: (struct lock_class_key *)&attr->skey;
 #endif
-	kn = kernfs_create_file_ns_key(parent, attr->name, mode, size,
-				       ops, (void *)attr, ns, key);
+	kn = __kernfs_create_file(parent, attr->name, mode, size, ops,
+				  (void *)attr, ns, true, key);
 	if (IS_ERR(kn)) {
 		if (PTR_ERR(kn) == -EEXIST)
 			sysfs_warn_dup(parent, attr->name);
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 0ca2aed..321ed84 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -34,7 +34,6 @@ enum kernfs_node_type {
 };
 
 #define KERNFS_TYPE_MASK	0x000f
-#define KERNFS_COPY_NAME	(KERNFS_DIR | KERNFS_LINK)
 #define KERNFS_ACTIVE_REF	KERNFS_FILE
 #define KERNFS_FLAG_MASK	~KERNFS_TYPE_MASK
 
@@ -44,6 +43,7 @@ enum kernfs_node_flag {
 	KERNFS_HAS_SEQ_SHOW	= 0x0040,
 	KERNFS_HAS_MMAP		= 0x0080,
 	KERNFS_LOCKDEP		= 0x0100,
+	KERNFS_STATIC_NAME	= 0x0200,
 };
 
 /* type-specific structures for kernfs_node union members */
@@ -212,12 +212,13 @@ void kernfs_destroy_root(struct kernfs_root *root);
 struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
 					 const char *name, umode_t mode,
 					 void *priv, const void *ns);
-struct kernfs_node *kernfs_create_file_ns_key(struct kernfs_node *parent,
-					      const char *name,
-					      umode_t mode, loff_t size,
-					      const struct kernfs_ops *ops,
-					      void *priv, const void *ns,
-					      struct lock_class_key *key);
+struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
+					 const char *name,
+					 umode_t mode, loff_t size,
+					 const struct kernfs_ops *ops,
+					 void *priv, const void *ns,
+					 bool name_is_static,
+					 struct lock_class_key *key);
 struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
 				       const char *name,
 				       struct kernfs_node *target);
@@ -265,10 +266,10 @@ kernfs_create_dir_ns(struct kernfs_node *parent, const char *name,
 { return ERR_PTR(-ENOSYS); }
 
 static inline struct kernfs_node *
-kernfs_create_file_ns_key(struct kernfs_node *parent, const char *name,
-			  umode_t mode, loff_t size,
-			  const struct kernfs_ops *ops, void *priv,
-			  const void *ns, struct lock_class_key *key)
+__kernfs_create_file(struct kernfs_node *parent, const char *name,
+		     umode_t mode, loff_t size, const struct kernfs_ops *ops,
+		     void *priv, const void *ns, bool name_is_static,
+		     struct lock_class_key *key)
 { return ERR_PTR(-ENOSYS); }
 
 static inline struct kernfs_node *
@@ -330,8 +331,8 @@ kernfs_create_file_ns(struct kernfs_node *parent, const char *name,
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	key = (struct lock_class_key *)&ops->lockdep_key;
 #endif
-	return kernfs_create_file_ns_key(parent, name, mode, size, ops, priv,
-					 ns, key);
+	return __kernfs_create_file(parent, name, mode, size, ops, priv, ns,
+				    false, key);
 }
 
 static inline struct kernfs_node *
-- 
1.8.4.2

[PATCH 4/6] kernfs: update kernfs_rename_ns() to consider KERNFS_STATIC_NAME

[Tejun Heo]

kernfs_rename_ns() currently assumes that the target sysfs_dirent has
a copied name.  This has been okay because sysfs supports rename only
for directories which always have copied names; however, there's
nothing in kernfs interface which calls for such restriction and
currently invoking kernfs_rename_ns() on a regular file leads to oops
because it ends up trying to kfree() a static name.

This patch updates kernfs_rename_ns() so that it skips kfree() of the
old name if it's static.  This allows it to be used for all node
types.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 fs/kernfs/dir.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index e168177..d33af95 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -886,7 +886,11 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct kernfs_node *new_parent,
 		if (!new_name)
 			goto out;
 
-		kfree(kn->name);
+		if (kn->flags & KERNFS_STATIC_NAME)
+			kn->flags &= ~KERNFS_STATIC_NAME;
+		else
+			kfree(kn->name);
+
 		kn->name = new_name;
 	}
 
-- 
1.8.4.2

[PATCH 5/6] kernfs: allow negative dentries

[Tejun Heo]

kernfs doesn't allow negative dentries - kernfs_iop_lookup() returns
ERR_PTR(-ENOENT) instead of NULL which short-circuits negative dentry
creation and kernfs's d_delete() callback, kernfs_dop_delete(),
returns 1 for all removed nodes.  This in turn allows
kernfs_dop_revalidate() to assume that there's no negative dentry for
kernfs.

This worked fine for sysfs but kernfs is scheduled to grow mkdir(2)
support which depend on negative dentries.  This patch updates so that
kernfs allows negative dentries.  The required changes are almost
trivial - kernfs_iop_lookup() now returns NULL instead of
ERR_PTR(-ENOENT) when the target kernfs_node doesn't exist,
kernfs_dop_delete() is removed and kernfs_dop_revalidate() is updated
to check whether the target dentry is negative and request fresh
lookup if so.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 fs/kernfs/dir.c | 37 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index d33af95..42c5b9f 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -264,12 +264,6 @@ void kernfs_put(struct kernfs_node *kn)
 }
 EXPORT_SYMBOL_GPL(kernfs_put);
 
-static int kernfs_dop_delete(const struct dentry *dentry)
-{
-	struct kernfs_node *kn = dentry->d_fsdata;
-	return !(kn && !(kn->flags & KERNFS_REMOVED));
-}
-
 static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
 {
 	struct kernfs_node *kn;
@@ -277,6 +271,10 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
+	/* Always perform fresh lookup for negatives */
+	if (!dentry->d_inode)
+		goto out_bad_unlocked;
+
 	kn = dentry->d_fsdata;
 	mutex_lock(&kernfs_mutex);
 
@@ -301,22 +299,14 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
 out_valid:
 	return 1;
 out_bad:
-	/*
-	 * Remove the dentry from the dcache hashes.
-	 * If this is a deleted dentry we use d_drop instead of d_delete
-	 * so kernfs doesn't need to cope with negative dentries.
-	 *
-	 * If this is a dentry that has simply been renamed we
-	 * use d_drop to remove it from the dcache lookup on its
-	 * old parent.  If this dentry persists later when a lookup
-	 * is performed at its new name the dentry will be readded
-	 * to the dcache hashes.
-	 */
 	mutex_unlock(&kernfs_mutex);
-
-	/* If we have submounts we must allow the vfs caches
-	 * to lie about the state of the filesystem to prevent
-	 * leaks and other nasty things.
+out_bad_unlocked:
+	/*
+	 * @dentry doesn't match the underlying kernfs node, drop the
+	 * dentry and force lookup.  If we have submounts we must allow the
+	 * vfs caches to lie about the state of the filesystem to prevent
+	 * leaks and other nasty things, so use check_submounts_and_drop()
+	 * instead of d_drop().
 	 */
 	if (check_submounts_and_drop(dentry) != 0)
 		goto out_valid;
@@ -331,7 +321,6 @@ static void kernfs_dop_release(struct dentry *dentry)
 
 const struct dentry_operations kernfs_dops = {
 	.d_revalidate	= kernfs_dop_revalidate,
-	.d_delete	= kernfs_dop_delete,
 	.d_release	= kernfs_dop_release,
 };
 
@@ -682,7 +671,7 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir,
 					struct dentry *dentry,
 					unsigned int flags)
 {
-	struct dentry *ret = NULL;
+	struct dentry *ret;
 	struct kernfs_node *parent = dentry->d_parent->d_fsdata;
 	struct kernfs_node *kn;
 	struct inode *inode;
@@ -697,7 +686,7 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir,
 
 	/* no such entry */
 	if (!kn) {
-		ret = ERR_PTR(-ENOENT);
+		ret = NULL;
 		goto out_unlock;
 	}
 	kernfs_get(kn);
-- 
1.8.4.2

[PATCH 6/6] kernfs: add kernfs_dir_ops

[Tejun Heo]

Add support for mkdir(2), rmdir(2) and rename(2) syscalls.  This is
implemented through optional kernfs_dir_ops callback table which can
be specified on kernfs_create_root().  An implemented callback is
invoked when the matching syscall is invoked.

As kernfs keep dcache syncs with internal representation and
revalidates dentries on each access, the implementation of these
methods is extremely simple.  Each just discovers the relevant
kernfs_node(s) and invokes the requested callback which is allowed to
do any kernfs operations and the end result doesn't necessarily have
to match the expected semantics of the syscall.

This will be used to convert cgroup to use kernfs instead of its own
filesystem implementation.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 fs/kernfs/dir.c        | 44 +++++++++++++++++++++++++++++++++++++++++++-
 fs/sysfs/mount.c       |  2 +-
 include/linux/kernfs.h | 21 +++++++++++++++++++--
 3 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 42c5b9f..510b506 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -583,12 +583,13 @@ EXPORT_SYMBOL_GPL(kernfs_find_and_get_ns);
 
 /**
  * kernfs_create_root - create a new kernfs hierarchy
+ * @kdops: optional directory syscall operations for the hierarchy
  * @priv: opaque data associated with the new directory
  *
  * Returns the root of the new hierarchy on success, ERR_PTR() value on
  * failure.
  */
-struct kernfs_root *kernfs_create_root(void *priv)
+struct kernfs_root *kernfs_create_root(struct kernfs_dir_ops *kdops, void *priv)
 {
 	struct kernfs_root *root;
 	struct kernfs_node *kn;
@@ -610,6 +611,7 @@ struct kernfs_root *kernfs_create_root(void *priv)
 	kn->priv = priv;
 	kn->dir.root = root;
 
+	root->dir_ops = kdops;
 	root->kn = kn;
 
 	return root;
@@ -706,6 +708,42 @@ static struct dentry *kernfs_iop_lookup(struct inode *dir,
 	return ret;
 }
 
+static int kernfs_iop_mkdir(struct inode *dir, struct dentry *dentry,
+			    umode_t mode)
+{
+	struct kernfs_node *parent = dir->i_private;
+	struct kernfs_dir_ops *kdops = kernfs_root(parent)->dir_ops;
+
+	if (!kdops || !kdops->mkdir)
+		return -EPERM;
+
+	return kdops->mkdir(parent, dentry->d_name.name, mode);
+}
+
+static int kernfs_iop_rmdir(struct inode *dir, struct dentry *dentry)
+{
+	struct kernfs_node *kn  = dentry->d_fsdata;
+	struct kernfs_dir_ops *kdops = kernfs_root(kn)->dir_ops;
+
+	if (!kdops || !kdops->rmdir)
+		return -EPERM;
+
+	return kdops->rmdir(kn);
+}
+
+static int kernfs_iop_rename(struct inode *old_dir, struct dentry *old_dentry,
+			     struct inode *new_dir, struct dentry *new_dentry)
+{
+	struct kernfs_node *kn  = old_dentry->d_fsdata;
+	struct kernfs_node *new_parent = new_dir->i_private;
+	struct kernfs_dir_ops *kdops = kernfs_root(kn)->dir_ops;
+
+	if (!kdops || !kdops->rename)
+		return -EPERM;
+
+	return kdops->rename(kn, new_parent, new_dentry->d_name.name);
+}
+
 const struct inode_operations kernfs_dir_iops = {
 	.lookup		= kernfs_iop_lookup,
 	.permission	= kernfs_iop_permission,
@@ -715,6 +753,10 @@ const struct inode_operations kernfs_dir_iops = {
 	.removexattr	= kernfs_iop_removexattr,
 	.getxattr	= kernfs_iop_getxattr,
 	.listxattr	= kernfs_iop_listxattr,
+
+	.mkdir		= kernfs_iop_mkdir,
+	.rmdir		= kernfs_iop_rmdir,
+	.rename		= kernfs_iop_rename,
 };
 
 static struct kernfs_node *kernfs_leftmost_descendant(struct kernfs_node *pos)
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index 701a56f..6211230 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -62,7 +62,7 @@ int __init sysfs_init(void)
 {
 	int err;
 
-	sysfs_root = kernfs_create_root(NULL);
+	sysfs_root = kernfs_create_root(NULL, NULL);
 	if (IS_ERR(sysfs_root))
 		return PTR_ERR(sysfs_root);
 
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 321ed84..d2c439d 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -111,12 +111,27 @@ struct kernfs_node {
 	struct kernfs_iattrs	*iattr;
 };
 
+/*
+ * kernfs_dir_ops may be specified on kernfs_create_root() to support
+ * directory manipulation syscalls.  These optional callbacks are invoked
+ * on the matching syscalls and can perform any kernfs operations which
+ * don't necessarily have to be the exact operation requested.
+ */
+struct kernfs_dir_ops {
+	int (*mkdir)(struct kernfs_node *parent, const char *name,
+		     umode_t mode);
+	int (*rmdir)(struct kernfs_node *kn);
+	int (*rename)(struct kernfs_node *kn, struct kernfs_node *new_parent,
+		      const char *new_name);
+};
+
 struct kernfs_root {
 	/* published fields */
 	struct kernfs_node	*kn;
 
 	/* private fields, do not use outside kernfs proper */
 	struct ida		ino_ida;
+	struct kernfs_dir_ops	*dir_ops;
 };
 
 struct kernfs_open_file {
@@ -206,7 +221,8 @@ struct kernfs_node *kernfs_find_and_get_ns(struct kernfs_node *parent,
 void kernfs_get(struct kernfs_node *kn);
 void kernfs_put(struct kernfs_node *kn);
 
-struct kernfs_root *kernfs_create_root(void *priv);
+struct kernfs_root *kernfs_create_root(struct kernfs_dir_ops *kdops,
+				       void *priv);
 void kernfs_destroy_root(struct kernfs_root *root);
 
 struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
@@ -255,7 +271,8 @@ kernfs_find_and_get_ns(struct kernfs_node *parent, const char *name,
 static inline void kernfs_get(struct kernfs_node *kn) { }
 static inline void kernfs_put(struct kernfs_node *kn) { }
 
-static inline struct kernfs_root *kernfs_create_root(void *priv)
+static inline struct kernfs_root *
+kernfs_create_root(struct kernfs_dir_ops *kdops, void *priv)
 { return ERR_PTR(-ENOSYS); }
 
 static inline void kernfs_destroy_root(struct kernfs_root *root) { }
-- 
1.8.4.2

test-kernfs module

[Tejun Heo]

diff --git a/test-kernfs/Makefile b/test-kernfs/Makefile
new file mode 100644
index 0000000..e64553b
--- /dev/null
+++ b/test-kernfs/Makefile
@@ -0,0 +1 @@
+obj-m := test-kernfs.o
diff --git a/test-kernfs/test-kernfs.c b/test-kernfs/test-kernfs.c
new file mode 100644
index 0000000..4f91b1b
--- /dev/null
+++ b/test-kernfs/test-kernfs.c
@@ -0,0 +1,153 @@
+#include <linux/module.h>
+#include <linux/kernfs.h>
+#include <linux/fs.h>
+#include <linux/seq_file.h>
+
+static struct kernfs_root *test_root;
+
+char test_buf[4096] = "whatever";
+
+static int test_seq_show(struct seq_file *sf, void *v)
+{
+	struct kernfs_open_file *of = sf->private;
+
+	seq_printf(sf, "%s: %s\n", of->kn->name, test_buf);
+	return 0;
+}
+
+static ssize_t test_write(struct kernfs_open_file *sf, char *buf, size_t bytes,
+			  loff_t off)
+{
+	bytes = min_t(size_t, bytes, PAGE_SIZE - 1);
+	memcpy(test_buf, buf, bytes);
+	test_buf[bytes] = '\0';
+	return bytes;
+}
+
+static struct kernfs_ops test_kernfs_ops = {
+	.seq_show	= test_seq_show,
+	.write		= test_write,
+};
+
+static int test_mkdir(struct kernfs_node *parent, const char *name, umode_t mode)
+{
+	struct kernfs_node *kn;
+
+	if (IS_ERR(kn = kernfs_create_dir(parent, name, mode, NULL))) {
+		printk("XXX test_mkdir %zd\n", PTR_ERR(kn));
+		return PTR_ERR(kn);
+	}
+
+	kernfs_create_file(kn, "file", 0755, 4096, &test_kernfs_ops, NULL);
+
+	printk("XXX test_mkdir success\n");
+	return 0;
+}
+
+static int test_rmdir(struct kernfs_node *kn)
+{
+	printk("XXX test_rmdir\n");
+	kernfs_remove(kn);
+	return 0;
+}
+
+static int test_rename(struct kernfs_node *kn, struct kernfs_node *new_parent,
+		       const char *new_name)
+{
+	static char namebuf[4096];
+
+	snprintf(namebuf, sizeof(namebuf), "%s-XXX", new_name);
+	printk("XXX test_rename\n");
+	return kernfs_rename_ns(kn, new_parent, namebuf, NULL);
+}
+
+static struct kernfs_dir_ops test_dops = {
+	.mkdir		= test_mkdir,
+	.rmdir		= test_rmdir,
+	.rename		= test_rename,
+};
+
+static void test_populate(void)
+{
+	struct kernfs_node *root = test_root->kn;
+	struct kernfs_node *d0, *d00, *d01, *d1, *d10;
+	struct kernfs_node *f0_0, *f0_1, *f00_0, *f01_1;
+	struct kernfs_node *l1_0, *l1_1, *l10_0;
+
+	if (IS_ERR(d0 = kernfs_create_dir(root, "d0", 0755, NULL)))
+		goto err;
+	if (IS_ERR(d00 = kernfs_create_dir(d0, "d00", 0755, NULL)))
+		goto err;
+	if (IS_ERR(d01 = kernfs_create_dir(d0, "d01", 0755, NULL)))
+		goto err;
+	if (IS_ERR(d1 = kernfs_create_dir(root, "d1", 0755, NULL)))
+		goto err;
+	if (IS_ERR(d10 = kernfs_create_dir(d1, "d10", 0755, NULL)))
+		goto err;
+
+	if (IS_ERR(f0_0 = kernfs_create_file(d0, "f0_0", 0755, 4096,
+					     &test_kernfs_ops, NULL)))
+		goto err;
+	if (IS_ERR(f0_1 = kernfs_create_file(d0, "f0_1", 0755, 4096,
+					     &test_kernfs_ops, NULL)))
+		goto err;
+	if (IS_ERR(f00_0 = kernfs_create_file(d00, "f00_0", 0755, 4096,
+					      &test_kernfs_ops, NULL)))
+		goto err;
+	if (IS_ERR(f01_1 = kernfs_create_file(d01, "f01_1", 0755, 4096,
+					      &test_kernfs_ops, NULL)))
+		goto err;
+
+	if (IS_ERR(l1_0 = kernfs_create_link(d1, "l1_0", f0_0)))
+		goto err;
+	if (IS_ERR(l1_1 = kernfs_create_link(d1, "l1_1", f0_1)))
+		goto err;
+	if (IS_ERR(l10_0 = kernfs_create_link(d10, "l10_0", f0_0)))
+		goto err;
+
+	return;
+err:
+	printk("XXX failed to populate\n");
+}
+
+static struct dentry *test_mount(struct file_system_type *fs_type, int flags,
+				 const char *dev_name, void *data)
+{
+	return kernfs_mount(fs_type, flags, test_root);
+}
+
+static struct file_system_type test_fs_type = {
+	.name		= "test",
+	.mount		= test_mount,
+	.kill_sb	= kernfs_kill_sb,
+	.owner		= THIS_MODULE,
+};
+
+static int __init test_init(void)
+{
+	int err;
+
+	test_root = kernfs_create_root(&test_dops, NULL);
+	if (IS_ERR(test_root))
+		return PTR_ERR(test_root);
+
+	err = register_filesystem(&test_fs_type);
+	if (err) {
+		kernfs_destroy_root(test_root);
+		return err;
+	}
+
+	test_populate();
+
+	return 0;
+}
+
+static void __exit test_exit(void)
+{
+	unregister_filesystem(&test_fs_type);
+	kernfs_destroy_root(test_root);
+}
+
+module_init(test_init);
+module_exit(test_exit);
+MODULE_LICENSE("GPL");

Re: [PATCHSET driver-core-next] kernfs: implement kernfs_dir_ops

[Greg KH]

On Wed, Dec 11, 2013 at 04:02:54PM -0500, Tejun Heo wrote:
> Hello,
> 
> kernfs is now properly separated out from sysfs and can be used for
> other pseudo filesystems too; however, there are still some missing
> features - kernfs filesystems can't process any directory manipulation
> systemcalls such as mkdir(2), rmdir(2) and rename(2), which cgroup,
> one of the prospect kernfs users, requires for cgroup management.
> 
> This patchset implements those directory operation hooks so that
> kernfs users can be hooked into those system calls and implement
> appropriate actions, which may or may not match the actual semantics
> of those syscalls.
> 
> The implementation is mostly straight forward except for one aspect.
> sysfs, and thus kernfs, never allowed negative dentries.  They are
> actively shot down and lookup callback assumes that there's no
> negative dentries.  This doesn't really have a lot of benefits and
> changing isn't difficult.  The lookup path simply needs to tell the
> vfs layer that a full fresh lookup is necessary for negative dentries.

Very cool, will this let us move configfs to use kernfs as well?  That's
one of the reasons that the two filesystems couldn't be merged in the
past from what I remember.

thanks,

greg k-h

Re: [PATCHSET driver-core-next] kernfs: implement kernfs_dir_ops

[Tejun Heo]

Hey, Greg.

On Wed, Dec 11, 2013 at 01:19:07PM -0800, Greg KH wrote:
> Very cool, will this let us move configfs to use kernfs as well?  That's
> one of the reasons that the two filesystems couldn't be merged in the
> past from what I remember.

I haven't looked into the details but configfs would need hooks for
more operations - file creation and all; however, those should be
trivial.  kernfs doesn't care what those operations do at all.  All it
needs to do is just routing the invocations through the callbacks.

Thanks.

-- 
tejun