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

Groups > linux.kernel > #1647466 > unrolled thread

[PATCH 5/5] kernfs: add exportfs operations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 5/5] kernfs: add exportfs operations Shaohua Li <shli@fb.com> - 2017-05-23 01:00 +0200
    Re: [PATCH 5/5] kernfs: add exportfs operations Christoph Hellwig <hch@infradead.org> - 2017-05-23 09:50 +0200
      Re: [PATCH 5/5] kernfs: add exportfs operations Tejun Heo <tj@kernel.org> - 2017-05-23 21:00 +0200
        Re: [PATCH 5/5] kernfs: add exportfs operations Christoph Hellwig <hch@infradead.org> - 2017-05-24 19:40 +0200
          Re: [PATCH 5/5] kernfs: add exportfs operations Tejun Heo <tj@kernel.org> - 2017-05-24 19:50 +0200
            Re: [PATCH 5/5] kernfs: add exportfs operations Christoph Hellwig <hch@infradead.org> - 2017-05-24 19:50 +0200
              Re: [PATCH 5/5] kernfs: add exportfs operations Tejun Heo <tj@kernel.org> - 2017-05-24 19:50 +0200
    Re: [PATCH 5/5] kernfs: add exportfs operations Tejun Heo <tj@kernel.org> - 2017-05-23 21:00 +0200

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

Now we have the facilities to implement exportfs operations.

Signed-off-by: Shaohua Li <shli@fb.com>
---
 fs/kernfs/mount.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/fs/kernfs/mount.c b/fs/kernfs/mount.c
index 462a40c..5af88cc 100644
--- a/fs/kernfs/mount.c
+++ b/fs/kernfs/mount.c
@@ -16,6 +16,7 @@
 #include <linux/pagemap.h>
 #include <linux/namei.h>
 #include <linux/seq_file.h>
+#include <linux/exportfs.h>
 
 #include "kernfs-internal.h"
 
@@ -64,6 +65,59 @@ const struct super_operations kernfs_sops = {
 	.show_path	= kernfs_sop_show_path,
 };
 
+static struct inode *kernfs_nfs_get_inode(struct super_block *sb,
+		u64 ino, u32 generation)
+{
+	struct kernfs_super_info *info = kernfs_info(sb);
+	struct inode *inode;
+	struct kernfs_node *kn;
+
+	if (ino == 0)
+		return ERR_PTR(-ESTALE);
+
+	kn = kernfs_get_node_by_ino(info->root, ino);
+	if (!kn)
+		return ERR_PTR(-ESTALE);
+	inode = kernfs_get_inode(sb, kn);
+	kernfs_put(kn);
+	if (IS_ERR(inode))
+		return ERR_CAST(inode);
+
+	if (generation && inode->i_generation != generation) {
+		/* we didn't find the right inode.. */
+		iput(inode);
+		return ERR_PTR(-ESTALE);
+	}
+	return inode;
+}
+
+static struct dentry *kernfs_fh_to_dentry(struct super_block *sb, struct fid *fid,
+		int fh_len, int fh_type)
+{
+	return generic_fh_to_dentry(sb, fid, fh_len, fh_type,
+				    kernfs_nfs_get_inode);
+}
+
+static struct dentry *kernfs_fh_to_parent(struct super_block *sb, struct fid *fid,
+		int fh_len, int fh_type)
+{
+	return generic_fh_to_parent(sb, fid, fh_len, fh_type,
+				    kernfs_nfs_get_inode);
+}
+
+static struct dentry *kernfs_get_parent_dentry(struct dentry *child)
+{
+	struct kernfs_node *kn = kernfs_dentry_node(child);
+
+	return d_obtain_alias(kernfs_get_inode(child->d_sb, kn->parent));
+}
+
+static const struct export_operations kernfs_export_ops = {
+	.fh_to_dentry = kernfs_fh_to_dentry,
+	.fh_to_parent = kernfs_fh_to_parent,
+	.get_parent = kernfs_get_parent_dentry,
+};
+
 /**
  * kernfs_root_from_sb - determine kernfs_root associated with a super_block
  * @sb: the super_block in question
@@ -159,6 +213,7 @@ static int kernfs_fill_super(struct super_block *sb, unsigned long magic)
 	sb->s_magic = magic;
 	sb->s_op = &kernfs_sops;
 	sb->s_xattr = kernfs_xattr_handlers;
+	sb->s_export_op = &kernfs_export_ops;
 	sb->s_time_gran = 1;
 
 	/* get root inode, initialize and unlock it */
-- 
2.9.3

[toc] | [next] | [standalone]

#1647746

On Mon, May 22, 2017 at 03:53:09PM -0700, Shaohua Li wrote:
> Now we have the facilities to implement exportfs operations.

But do we have a use case?  I'd rather avoid this..

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

#1648344

Hello, Christoph.

On Tue, May 23, 2017 at 12:40:19AM -0700, Christoph Hellwig wrote:
> On Mon, May 22, 2017 at 03:53:09PM -0700, Shaohua Li wrote:
> > Now we have the facilities to implement exportfs operations.
> 
> But do we have a use case?  I'd rather avoid this..

Yeah, this is one of the repeatedly requested features - a cgroup id
which can be looked up in a scalable way.  We probably should have
added this earlier too as we already have places where we're passing
in full cgroup path into the kernel (ipt_cgroup match).

IIUC, Shaohua is adding it so that block tracing can be made aware of
cgroups but something like this is necessary whenever we try to refer
to a cgroup in a race-free / scalable way.

If you have any other ideas, I'm all ears.

Thanks.

-- 
tejun

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

#1649814

On Tue, May 23, 2017 at 02:57:11PM -0400, Tejun Heo wrote:
> 
> Yeah, this is one of the repeatedly requested features - a cgroup id
> which can be looked up in a scalable way.  We probably should have
> added this earlier too as we already have places where we're passing
> in full cgroup path into the kernel (ipt_cgroup match).

I still have no idea how this ties into export ops.  Currently I don't
see any tracing code using export operations, so maybe a big part of
the series is missing/

> IIUC, Shaohua is adding it so that block tracing can be made aware of
> cgroups but something like this is necessary whenever we try to refer
> to a cgroup in a race-free / scalable way.

You can already get i_ino using statx, and i_generation using
FS_IOC_GETVERSION, so I'm a little lost on why you need the full
exportfs infrastructure.

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

#1649818

Hello, Christoph.

On Wed, May 24, 2017 at 10:38:22AM -0700, Christoph Hellwig wrote:
> On Tue, May 23, 2017 at 02:57:11PM -0400, Tejun Heo wrote:
> > 
> > Yeah, this is one of the repeatedly requested features - a cgroup id
> > which can be looked up in a scalable way.  We probably should have
> > added this earlier too as we already have places where we're passing
> > in full cgroup path into the kernel (ipt_cgroup match).
> 
> I still have no idea how this ties into export ops.  Currently I don't
> see any tracing code using export operations, so maybe a big part of
> the series is missing/

Oh yeah, it's not adding any users yet.

> > IIUC, Shaohua is adding it so that block tracing can be made aware of
> > cgroups but something like this is necessary whenever we try to refer
> > to a cgroup in a race-free / scalable way.
> 
> You can already get i_ino using statx, and i_generation using
> FS_IOC_GETVERSION, so I'm a little lost on why you need the full
> exportfs infrastructure.

But how do you map that back to the cgroup without scanning the cgroup
hierarchy?

Thanks.

-- 
tejun

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

#1649825

On Wed, May 24, 2017 at 01:39:39PM -0400, Tejun Heo wrote:
> > You can already get i_ino using statx, and i_generation using
> > FS_IOC_GETVERSION, so I'm a little lost on why you need the full
> > exportfs infrastructure.
> 
> But how do you map that back to the cgroup without scanning the cgroup
> hierarchy?

I'm totally lost on why you would do that.  So maybe you just need
to send the full patch so that reviewers get the full picture.

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

#1649830

Hello, Christoph.

On Wed, May 24, 2017 at 10:41:38AM -0700, Christoph Hellwig wrote:
> > But how do you map that back to the cgroup without scanning the cgroup
> > hierarchy?
> 
> I'm totally lost on why you would do that.  So maybe you just need
> to send the full patch so that reviewers get the full picture.

Here's a simple scenario.  Let's say blktrace now exposes the cgroup
inode and generation numbers per trace.  Userland tool now wants to
show that in a human readable format but it can only map back the
inode and generation numbers to the path by scanning the cgroup tree.
So, the goal is having a token which is not path which uniquely
identifies a cgroup and the ability to map that back to cgroup path.
We can add a dedicated interface to cgroup root, for example, and
allow querying by echoing inode and generation numbers into it but
that's kinda clumsy.

Thanks.

-- 
tejun

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

#1648345

On Mon, May 22, 2017 at 03:53:09PM -0700, Shaohua Li wrote:
> +static struct inode *kernfs_nfs_get_inode(struct super_block *sb,
> +		u64 ino, u32 generation)

Heh, do we have to name this kernfs_nfs_get_inode()?  I think it'd be
nice to give it a different name and add a comment explaining the goal
of the interface.

Also, I think it probably would be better to make this an optional
feature of kernfs.  sysfs has no need to provide something like this
yet.

Thanks.

-- 
tejun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web