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

Groups > linux.kernel > #1647469 > unrolled thread

[PATCH 0/5] kernfs: add exportfs operations

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] kernfs: add exportfs operations Shaohua Li <shli@fb.com> - 2017-05-23 01:00 +0200
    [PATCH 3/5] kernfs: add an API to get kernfs node from inode number Shaohua Li <shli@fb.com> - 2017-05-23 01:00 +0200
    [PATCH 2/5] kernfs: use idr instead of ida to manage inode number Shaohua Li <shli@fb.com> - 2017-05-23 01:00 +0200
    Re: [PATCH 0/5] kernfs: add exportfs operations Christoph Hellwig <hch@infradead.org> - 2017-05-23 09:50 +0200
    Re: [PATCH 0/5] kernfs: add exportfs operations Tejun Heo <tj@kernel.org> - 2017-05-23 21:10 +0200

#1647469 — [PATCH 0/5] kernfs: add exportfs operations

Hi,

The goal isn't to export kernfs to NFS. The intention is to make tracing cgroup
aware. To do this, tracing will record an id for cgroup and use the id to find
cgroup name later. The best id is the cgroup directory inode number. Further to
filter out stale cgroup directory, fhandle is the best to identify a cgroup. So
this is what this series try to do.

Thanks,
Shaohua

Shaohua Li (5):
  kernfs: implement i_generation
  kernfs: use idr instead of ida to manage inode number
  kernfs: add an API to get kernfs node from inode number
  kernfs: don't set dentry->d_fsdata
  kernfs: add exportfs operations

 fs/kernfs/dir.c             | 76 +++++++++++++++++++++++++++++++++------------
 fs/kernfs/file.c            |  6 ++--
 fs/kernfs/inode.c           |  7 +++--
 fs/kernfs/kernfs-internal.h |  4 +++
 fs/kernfs/mount.c           | 67 ++++++++++++++++++++++++++++++++++-----
 fs/kernfs/symlink.c         |  2 +-
 include/linux/kernfs.h      |  4 ++-
 7 files changed, 131 insertions(+), 35 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1647470 — [PATCH 3/5] kernfs: add an API to get kernfs node from inode number

Add an API to get kernfs node from inode number. We will need this to
implement exportfs operations.

To make the API lock free, kernfs node is freed in RCU context. And we
depend on kernfs_node count/ino number to filter stale kernfs nodes.

Signed-off-by: Shaohua Li <shli@fb.com>
---
 fs/kernfs/dir.c             | 35 +++++++++++++++++++++++++++++++++++
 fs/kernfs/kernfs-internal.h |  2 ++
 fs/kernfs/mount.c           |  4 +++-
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 8e8545a..4c86e4c 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -643,6 +643,7 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
 	kn->ino = ret;
 	kn->generation = atomic_inc_return(&root->next_generation);
 
+	/* set ino first. Above atomic_inc_return has a barrier */
 	atomic_set(&kn->count, 1);
 	atomic_set(&kn->active, KN_DEACTIVATED_BIAS);
 	RB_CLEAR_NODE(&kn->rb);
@@ -674,6 +675,40 @@ struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
 	return kn;
 }
 
+/*
+ * kernfs_get_node_by_ino - get kernfs_node from inode number
+ * @root: the kernfs root
+ * @ino: inode number
+ *
+ * RETURNS:
+ * NULL on failure. Return a kernfs node with reference counter incremented
+ */
+struct kernfs_node *kernfs_get_node_by_ino(struct kernfs_root *root,
+					   unsigned int ino)
+{
+	struct kernfs_node *kn;
+
+	rcu_read_lock();
+	kn = idr_find(&root->ino_idr, ino);
+	if (!kn)
+		goto out;
+	/* kernfs_put removes the ino after count is 0 */
+	if (!atomic_inc_not_zero(&kn->count)) {
+		kn = NULL;
+		goto out;
+	}
+	/* If this node is reused, __kernfs_new_node sets ino before count */
+	if (kn->ino != ino)
+		goto out;
+	rcu_read_unlock();
+
+	return kn;
+out:
+	rcu_read_unlock();
+	kernfs_put(kn);
+	return NULL;
+}
+
 /**
  *	kernfs_add_one - add kernfs_node to parent without warning
  *	@kn: kernfs_node to be added
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index 2d5144a..3534cfe 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -98,6 +98,8 @@ int kernfs_add_one(struct kernfs_node *kn);
 struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
 				    const char *name, umode_t mode,
 				    unsigned flags);
+struct kernfs_node *kernfs_get_node_by_ino(struct kernfs_root *root,
+					   unsigned int ino);
 
 /*
  * file.c
diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index d5b149a..343dfeb 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -332,5 +332,7 @@ void __init kernfs_init(void)
 {
 	kernfs_node_cache = kmem_cache_create("kernfs_node_cache",
 					      sizeof(struct kernfs_node),
-					      0, SLAB_PANIC, NULL);
+					      0,
+					      SLAB_PANIC | SLAB_TYPESAFE_BY_RCU,
+					      NULL);
 }
-- 
2.9.3

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

#1647472 — [PATCH 2/5] kernfs: use idr instead of ida to manage inode number

kernfs uses ida to manage inode number. The problem is we can't get
kernfs_node from inode number with ida. Switching to use idr, next patch
will add an API to get kernfs_node from inode number.

Signed-off-by: Shaohua Li <shli@fb.com>
---
 fs/kernfs/dir.c        | 17 ++++++++++++-----
 include/linux/kernfs.h |  2 +-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 09d093e..8e8545a 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -21,6 +21,7 @@
 DEFINE_MUTEX(kernfs_mutex);
 static DEFINE_SPINLOCK(kernfs_rename_lock);	/* kn->parent and ->name */
 static char kernfs_pr_cont_buf[PATH_MAX];	/* protected by rename_lock */
+static DEFINE_SPINLOCK(kernfs_idr_lock);	/* root->ino_idr */
 
 #define rb_to_kn(X) rb_entry((X), struct kernfs_node, rb)
 
@@ -533,7 +534,9 @@ void kernfs_put(struct kernfs_node *kn)
 		simple_xattrs_free(&kn->iattr->xattrs);
 	}
 	kfree(kn->iattr);
-	ida_simple_remove(&root->ino_ida, kn->ino);
+	spin_lock(&kernfs_idr_lock);
+	idr_remove(&root->ino_idr, kn->ino);
+	spin_unlock(&kernfs_idr_lock);
 	kmem_cache_free(kernfs_node_cache, kn);
 
 	kn = parent;
@@ -542,7 +545,7 @@ void kernfs_put(struct kernfs_node *kn)
 			goto repeat;
 	} else {
 		/* just released the root kn, free @root too */
-		ida_destroy(&root->ino_ida);
+		idr_destroy(&root->ino_idr);
 		kfree(root);
 	}
 }
@@ -630,7 +633,11 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
 	if (!kn)
 		goto err_out1;
 
-	ret = ida_simple_get(&root->ino_ida, 1, 0, GFP_KERNEL);
+	idr_preload(GFP_KERNEL);
+	spin_lock(&kernfs_idr_lock);
+	ret = idr_alloc(&root->ino_idr, kn, 1, 0, GFP_ATOMIC);
+	spin_unlock(&kernfs_idr_lock);
+	idr_preload_end();
 	if (ret < 0)
 		goto err_out2;
 	kn->ino = ret;
@@ -876,14 +883,14 @@ struct kernfs_root *kernfs_create_root(struct kernfs_syscall_ops *scops,
 	if (!root)
 		return ERR_PTR(-ENOMEM);
 
-	ida_init(&root->ino_ida);
+	idr_init(&root->ino_idr);
 	INIT_LIST_HEAD(&root->supers);
 	atomic_set(&root->next_generation, 0);
 
 	kn = __kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO,
 			       KERNFS_DIR);
 	if (!kn) {
-		ida_destroy(&root->ino_ida);
+		idr_destroy(&root->ino_idr);
 		kfree(root);
 		return ERR_PTR(-ENOMEM);
 	}
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index c5f0fa7..61668d1 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -164,7 +164,7 @@ struct kernfs_root {
 	unsigned int		flags;	/* KERNFS_ROOT_* flags */
 
 	/* private fields, do not use outside kernfs proper */
-	struct ida		ino_ida;
+	struct idr		ino_idr;
 	struct kernfs_syscall_ops *syscall_ops;
 
 	/* list of kernfs_super_info of this root, protected by kernfs_mutex */
-- 
2.9.3

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

#1647740

On Mon, May 22, 2017 at 03:53:04PM -0700, Shaohua Li wrote:
> Hi,
> 
> The goal isn't to export kernfs to NFS. The intention is to make tracing cgroup
> aware. To do this, tracing will record an id for cgroup and use the id to find
> cgroup name later. The best id is the cgroup directory inode number. Further to
> filter out stale cgroup directory, fhandle is the best to identify a cgroup. So
> this is what this series try to do.

Eww.  Even if you need to maintain i_generation for that please don't
add the full export_operations.  People will just get stupid ideas based
on that.

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

#1648351

Hello,

On Mon, May 22, 2017 at 03:53:04PM -0700, Shaohua Li wrote:
> The goal isn't to export kernfs to NFS. The intention is to make tracing cgroup
> aware. To do this, tracing will record an id for cgroup and use the id to find
> cgroup name later. The best id is the cgroup directory inode number. Further to
> filter out stale cgroup directory, fhandle is the best to identify a cgroup. So
> this is what this series try to do.

Generally looks good to me.  Had some review points in other replies.
There is a related issue which can be a nice follow-up - replacing
cgrp->id with kernfs's fid.  cgrp->id is allocated on its own idr and
its only use is to accelerate testing for ancestry and we can easily
replace that with the unique file id.

Thanks a lot!

-- 
tejun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web