[RFC/PATCH 1/2] stackfs: generic functions for obtaining hidden object

[RFC/PATCH 1/2] stackfs: generic functions for obtaining hidden object

[Pekka J Enberg]

From: Pekka Enberg <penberg@cs.helsinki.fi>

Add generic functions for obtaining the hidden object (superblock, inode,
dentry, and dentry mount-point) in a stacked filesystem.  As fan-out 
stacked filesystems have multiple hidden objects, we store them in a 
statically allocated array of pointers.  The current hard-coded limit 
STACKFS_MAX_BRANCHES is not enough for unionfs (for which users have more 
than 100 branches).  That, however, can be fixed later for unionfs.

Cc: Josef "Jeff" Sipek <jsipek@cs.sunysb.edu>
Cc: Erez Zadok <ezk@cs.sunysb.edu>
Cc: Mike Halcrow <mhalcrow@us.ibm.com>
Cc: Phillip Hellewell <phillip@hellewell.homeip.net>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 include/linux/stack_fs.h |   93 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

Index: 2.6/include/linux/stack_fs.h
===================================================================
--- /dev/null
+++ 2.6/include/linux/stack_fs.h
@@ -0,0 +1,93 @@
+#ifndef __LINUX_STACK_FS_H
+#define __LINUX_STACK_FS_H
+
+#include <linux/dcache.h>
+#include <linux/fs.h>
+#include <linux/mount.h>
+
+#define STACKFS_MAX_BRANCHES (8)
+
+struct stackfs_sb_info {
+	struct super_block *hidden_sbs[STACKFS_MAX_BRANCHES];
+};
+
+struct stackfs_inode_info {
+	struct inode *hidden_inodes[STACKFS_MAX_BRANCHES];
+};
+
+struct stackfs_dentry_info {
+	struct dentry *hidden_dentrys[STACKFS_MAX_BRANCHES];
+	struct vfsmount *hidden_mnts[STACKFS_MAX_BRANCHES];
+};
+
+struct stackfs_file_info {
+	struct file *hidden_files[STACKFS_MAX_BRANCHES];
+};
+
+/*
+ * The functions expect struct stackfs_*_info to be the first member in the
+ * filesystem-specific info structures and that inode->i_private points at
+ * the beginning of the inode container.
+ */
+
+static inline struct super_block *
+__stackfs_hidden_sb(struct super_block *sb, unsigned long branch_idx)
+{
+	struct stackfs_sb_info *info = sb->s_fs_info;
+	return info->hidden_sbs[branch_idx];
+}
+
+static inline struct super_block *stackfs_hidden_sb(struct super_block *sb)
+{
+	return __stackfs_hidden_sb(sb, 0);
+}
+
+static inline struct inode *
+__stackfs_hidden_inode(struct inode *inode, unsigned long branch_idx)
+{
+	struct stackfs_inode_info *info = inode->i_private;
+	return info->hidden_inodes[branch_idx];
+}
+
+static inline struct inode *stackfs_hidden_inode(struct inode *inode)
+{
+	return __stackfs_hidden_inode(inode, 0);
+}
+
+static inline struct dentry *
+__stackfs_hidden_dentry(struct dentry *dentry, unsigned long branch_idx)
+{
+	struct stackfs_dentry_info *info = dentry->d_fsdata;
+	return info->hidden_dentrys[branch_idx];
+}
+
+static inline struct dentry *stackfs_hidden_dentry(struct dentry *dentry)
+{
+	return __stackfs_hidden_dentry(dentry, 0);
+}
+
+static inline struct vfsmount *
+__stackfs_hidden_dentry_mnt(struct dentry *dentry, unsigned long branch_idx)
+{
+	struct stackfs_dentry_info *info = dentry->d_fsdata;
+	return info->hidden_mnts[branch_idx];
+}
+
+static inline struct vfsmount *stackfs_hidden_dentry_mnt(struct dentry *dentry)
+{
+	return __stackfs_hidden_dentry_mnt(dentry, 0);
+}
+
+static inline struct file *
+__stackfs_hidden_file(struct file *file, unsigned long branch_idx)
+{
+	struct stackfs_file_info *info = file->private_data;
+	return info->hidden_files[branch_idx];
+}
+
+static inline struct file *stackfs_hidden_file(struct file *file)
+{
+	return __stackfs_hidden_file(file, 0);
+}
+
+#endif

Re: [RFC/PATCH 1/2] stackfs: generic functions for obtaining hidden object

[Josef Sipek]

On Fri, Oct 13, 2006 at 04:22:09PM +0300, Pekka J Enberg wrote:
> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> Add generic functions for obtaining the hidden object (superblock, inode,
> dentry, and dentry mount-point) in a stacked filesystem.  As fan-out 
> stacked filesystems have multiple hidden objects, we store them in a 
> statically allocated array of pointers.  The current hard-coded limit 
> STACKFS_MAX_BRANCHES is not enough for unionfs (for which users have more 
> than 100 branches).  That, however, can be fixed later for unionfs.
 
Most users have 2-3 branches, however there are few that do indeed have 100
or more branches in one Union.
 
The functions you have allow you to get the values out of the arrays, but
there are no set functions. Otherwise it looks good.

Acked-by: Josef "Jeff" Sipek <jsipek@cs.sunysb.edu>

-- 
I think there is a world market for maybe five computers.
		- Thomas Watson, chairman of IBM, 1943.

Re: [RFC/PATCH 1/2] stackfs: generic functions for obtaining hidden object

[Erez Zadok]

In message <Pine.LNX.4.64.0610131615370.563@sbz-30.cs.Helsinki.FI>, Pekka J Enberg writes:
> From: Pekka Enberg <penberg@cs.helsinki.fi>
> 
> Add generic functions for obtaining the hidden object (superblock, inode,
> dentry, and dentry mount-point) in a stacked filesystem.  As fan-out 
> stacked filesystems have multiple hidden objects, we store them in a 
> statically allocated array of pointers.  The current hard-coded limit 
> STACKFS_MAX_BRANCHES is not enough for unionfs (for which users have more 
> than 100 branches).  That, however, can be fixed later for unionfs.

I think we should do it right the first time (i.e., now :-)

> +#define STACKFS_MAX_BRANCHES (8)

> +struct stackfs_sb_info {
> +	struct super_block *hidden_sbs[STACKFS_MAX_BRANCHES];
> +};

Why not make it something more dynamic, such as a mount-time option per sb?
Even at 8, you waste most of that space for non-fan-out stackable file
systems such as ecryptfs; and those unionfs users who want more, will have
to _recompile_ the code.

And given that this code is shared, if just one f/s needs 100 branches, why
should ecryptfs waste 99*sizeof(pointer) bytes for every *_info structure?

> +static inline struct super_block *
> +__stackfs_hidden_sb(struct super_block *sb, unsigned long branch_idx)
> +{
> +	struct stackfs_sb_info *info = sb->s_fs_info;
> +	return info->hidden_sbs[branch_idx];
> +}

Also, the functions don't check array bounds.  Where, if at all, this gets
checked against the STACKFS_MAX_BRANCHES value?  Shouldn't we at least put
some ASSERT's in there to catch bugs?

Of course, I realize that the above code is rather simple now and changing
it as I propose will require kmalloc/kfree (or containers) carefully used
throughout.

Thanks,
Erez.

Re: Re: [RFC/PATCH 1/2] stackfs: generic functions for obtaining hidden object

[Pekka Enberg]

On 10/13/06, Erez Zadok <ezk@cs.sunysb.edu> wrote:
> I think we should do it right the first time (i.e., now :-)

I would much rather merge it now (assuming I didn't break ecryptfs)
and have you unionfs developers fix it later :-).

On 10/13/06, Erez Zadok <ezk@cs.sunysb.edu> wrote:
> Why not make it something more dynamic, such as a mount-time option per sb?
> Even at 8, you waste most of that space for non-fan-out stackable file
> systems such as ecryptfs; and those unionfs users who want more, will have
> to _recompile_ the code.

Yes, we discussed this with Jeff already. For unionfs, we must make it
more dynamic. However, using slab unconditionally makes it totally
unacceptable for ecryptfs. Therefore, we need a small static array
that should satisfy most user (I think we can drop the static array
size to three):

struct stackfs_inode_info {
    struct inode **hidden_inodes;
    struct inode *static_inodes[3];
};

Initially, hidden_inodes can point to static_inodes which we can the
replace with a dynamic array if required. Btw, we probably want to do
krealloc() for that in the slab allocator.

                           Pekka

Re: Re: [RFC/PATCH 1/2] stackfs: generic functions for obtaining hidden object

[Josef Sipek]

On Fri, Oct 13, 2006 at 07:23:36PM +0300, Pekka Enberg wrote:
> On 10/13/06, Erez Zadok <ezk@cs.sunysb.edu> wrote:
> >I think we should do it right the first time (i.e., now :-)
> 
> I would much rather merge it now (assuming I didn't break ecryptfs)
> and have you unionfs developers fix it later :-).

Thanks :) As they say, it's the thought that counts, isn't it? ;)

> On 10/13/06, Erez Zadok <ezk@cs.sunysb.edu> wrote:
> >Why not make it something more dynamic, such as a mount-time option per sb?
> >Even at 8, you waste most of that space for non-fan-out stackable file
> >systems such as ecryptfs; and those unionfs users who want more, will have
> >to _recompile_ the code.
> 
> Yes, we discussed this with Jeff already. For unionfs, we must make it
> more dynamic. However, using slab unconditionally makes it totally
> unacceptable for ecryptfs. Therefore, we need a small static array
> that should satisfy most user (I think we can drop the static array
> size to three):

Nice, 3 pointers to inodes, and one to inode* = 4 pointers total, 128/256
bit struct on i386/x86_64.

> struct stackfs_inode_info {
>    struct inode **hidden_inodes;
>    struct inode *static_inodes[3];
> };
> 
> Initially, hidden_inodes can point to static_inodes which we can the
> replace with a dynamic array if required.

Hrm. You can have static store inodes {0,1,2} and the dynamic {3,4,5,...}
(this is what unionfs used to do - inline objects for performance). The
other way can be static array is ignored if dynamic array exists. In which
case, you effectively have {} in static, and {0,1,2,3,4,5,...} in dynamic. I
guess you could justify the wasting of the static array by arguing that if
the number of branches is << than number of static array elements, but I'm
afraid that that won't be the case most of the time.

> Btw, we probably want to do krealloc() for that in the slab allocator.

krealloc should be trivial to do (if the new size <= size of current slab,
do nothing, else alloc from a larger one).

Josef "Jeff" Sipek.

-- 
We have joy, we have fun, we have Linux on a Sun...