2006-10-17 04:52:45

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 0 of 2] fsstack: generic stackable filesystem helper functions

From: Josef "Jeff" Sipek <[email protected]>

The following patches introduce fsstack_copy_* functions. These functions
copy inode attributes (such as {a,c,m}time, mode, etc.) from one inode to
another.

While, intended for stackable filesystems any portion of the kernel wishing
to copy inode attributes can use them.

This series consists of two patches, the first introduces the fsstack
functions, and the second makes eCryptfs a user.

Changes since previous submission:

- rename to fsstack (akpm)
- removed fstack_copy_attr_timesizes (akpm)
- move non-inlined functions to fs/stack.c (Jan Engelhardt)

Signed-off-by: Josef "Jeff" Sipek <[email protected]>



2006-10-17 04:52:46

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 1 of 2] fsstack: Introduce fsstack_copy_{attr,inode}_*

From: Josef "Jeff" Sipek <[email protected]>

The following patch introduces several fsstack_copy_* functions which allow
stackable filesystems (such as eCryptfs and Unionfs) to easily copy over
(currently only) inode attributes. This prevents code duplication and allows
for code reuse.

Signed-off-by: Josef "Jeff" Sipek <[email protected]>

---

3 files changed, 80 insertions(+), 1 deletion(-)
fs/Makefile | 3 ++-
fs/stack.c | 39 +++++++++++++++++++++++++++++++++++++++
include/linux/fs_stack.h | 39 +++++++++++++++++++++++++++++++++++++++

diff --git a/fs/Makefile b/fs/Makefile
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -10,7 +10,8 @@ obj-y := open.o read_write.o file_table.
ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
attr.o bad_inode.o file.o filesystems.o namespace.o aio.o \
seq_file.o xattr.o libfs.o fs-writeback.o \
- pnode.o drop_caches.o splice.o sync.o utimes.o
+ pnode.o drop_caches.o splice.o sync.o utimes.o \
+ stack.o

ifeq ($(CONFIG_BLOCK),y)
obj-y += buffer.o bio.o block_dev.o direct-io.o mpage.o ioprio.o
diff --git a/fs/stack.c b/fs/stack.c
new file mode 100644
--- /dev/null
+++ b/fs/stack.c
@@ -0,0 +1,39 @@
+#include <linux/module.h>
+#include <linux/fs.h>
+
+/* does _NOT_ require i_mutex to be held.
+ *
+ * This function cannot be inlined since i_size_{read,write} is rather
+ * heavy-weight on 32-bit systems
+ */
+void fsstack_copy_inode_size(struct inode *dst, const struct inode *src)
+{
+ i_size_write(dst, i_size_read((struct inode *)src));
+ dst->i_blocks = src->i_blocks;
+}
+
+/* copy all attributes; get_nlinks is optional way to override the i_nlink
+ * copying
+ */
+void __fsstack_copy_attr_all(struct inode *dest,
+ const struct inode *src,
+ int (*get_nlinks)(struct inode *))
+{
+ if (!get_nlinks)
+ dest->i_nlink = src->i_nlink;
+ else
+ dest->i_nlink = (*get_nlinks)(dest);
+
+ dest->i_mode = src->i_mode;
+ dest->i_uid = src->i_uid;
+ dest->i_gid = src->i_gid;
+ dest->i_rdev = src->i_rdev;
+ dest->i_atime = src->i_atime;
+ dest->i_mtime = src->i_mtime;
+ dest->i_ctime = src->i_ctime;
+ dest->i_blkbits = src->i_blkbits;
+ dest->i_flags = src->i_flags;
+}
+
+EXPORT_SYMBOL_GPL(fsstack_copy_inode_size);
+EXPORT_SYMBOL_GPL(__fsstack_copy_attr_all);
diff --git a/include/linux/fs_stack.h b/include/linux/fs_stack.h
new file mode 100644
--- /dev/null
+++ b/include/linux/fs_stack.h
@@ -0,0 +1,39 @@
+#ifndef _LINUX_FS_STACK_H
+#define _LINUX_FS_STACK_H
+
+/* This file defines generic functions used primarily by stackable
+ * filesystems; none of these functions require i_mutex to be held.
+ */
+
+#include <linux/fs.h>
+
+/* externs for fs/stack.c */
+extern void __fsstack_copy_attr_all(struct inode *dest,
+ const struct inode *src,
+ int (*get_nlinks)(struct inode *));
+
+extern void fsstack_copy_inode_size(struct inode *dst, const struct inode *src);
+
+/* inlines */
+static inline void fsstack_copy_attr_atime(struct inode *dest,
+ const struct inode *src)
+{
+ dest->i_atime = src->i_atime;
+}
+
+static inline void fsstack_copy_attr_times(struct inode *dest,
+ const struct inode *src)
+{
+ dest->i_atime = src->i_atime;
+ dest->i_mtime = src->i_mtime;
+ dest->i_ctime = src->i_ctime;
+}
+
+static inline void fsstack_copy_attr_all(struct inode *dest,
+ const struct inode *src)
+{
+ __fsstack_copy_attr_all(dest, src, NULL);
+}
+
+#endif /* _LINUX_FS_STACK_H */
+


2006-10-17 04:53:09

by Josef 'Jeff' Sipek

[permalink] [raw]
Subject: [PATCH 2 of 2] eCryptfs: Use fsstack's generic copy inode attr functions

From: Josef "Jeff" Sipek <[email protected]>

Replace eCryptfs specific code & calls with the more generic fsstack
equivalents and remove the eCryptfs specific functions.

Signed-off-by: Josef "Jeff" Sipek <[email protected]>

---

3 files changed, 24 insertions(+), 59 deletions(-)
fs/ecryptfs/file.c | 3 +-
fs/ecryptfs/inode.c | 75 ++++++++++++---------------------------------------
fs/ecryptfs/main.c | 5 ++-

diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -30,6 +30,7 @@
#include <linux/security.h>
#include <linux/smp_lock.h>
#include <linux/compat.h>
+#include <linux/fs_stack.h>
#include "ecryptfs_kernel.h"

/**
@@ -192,7 +193,7 @@ retry:
goto retry;
file->f_pos = lower_file->f_pos;
if (rc >= 0)
- ecryptfs_copy_attr_atime(inode, lower_file->f_dentry->d_inode);
+ fsstack_copy_attr_atime(inode, lower_file->f_dentry->d_inode);
return rc;
}

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -30,6 +30,7 @@
#include <linux/namei.h>
#include <linux/mount.h>
#include <linux/crypto.h>
+#include <linux/fs_stack.h>
#include "ecryptfs_kernel.h"

static struct dentry *lock_parent(struct dentry *dentry)
@@ -51,48 +52,6 @@ static void unlock_dir(struct dentry *di
{
mutex_unlock(&dir->d_inode->i_mutex);
dput(dir);
-}
-
-void ecryptfs_copy_inode_size(struct inode *dst, const struct inode *src)
-{
- i_size_write(dst, i_size_read((struct inode *)src));
- dst->i_blocks = src->i_blocks;
-}
-
-void ecryptfs_copy_attr_atime(struct inode *dest, const struct inode *src)
-{
- dest->i_atime = src->i_atime;
-}
-
-static void ecryptfs_copy_attr_times(struct inode *dest,
- const struct inode *src)
-{
- dest->i_atime = src->i_atime;
- dest->i_mtime = src->i_mtime;
- dest->i_ctime = src->i_ctime;
-}
-
-static void ecryptfs_copy_attr_timesizes(struct inode *dest,
- const struct inode *src)
-{
- dest->i_atime = src->i_atime;
- dest->i_mtime = src->i_mtime;
- dest->i_ctime = src->i_ctime;
- ecryptfs_copy_inode_size(dest, src);
-}
-
-void ecryptfs_copy_attr_all(struct inode *dest, const struct inode *src)
-{
- dest->i_mode = src->i_mode;
- dest->i_nlink = src->i_nlink;
- dest->i_uid = src->i_uid;
- dest->i_gid = src->i_gid;
- dest->i_rdev = src->i_rdev;
- dest->i_atime = src->i_atime;
- dest->i_mtime = src->i_mtime;
- dest->i_ctime = src->i_ctime;
- dest->i_blkbits = src->i_blkbits;
- dest->i_flags = src->i_flags;
}

/**
@@ -171,8 +130,8 @@ ecryptfs_do_create(struct inode *directo
ecryptfs_printk(KERN_ERR, "Failure in ecryptfs_interpose\n");
goto out_lock;
}
- ecryptfs_copy_attr_timesizes(directory_inode,
- lower_dir_dentry->d_inode);
+ fsstack_copy_attr_times(directory_inode, lower_dir_dentry->d_inode);
+ fsstack_copy_inode_size(directory_inode, lower_dir_dentry->d_inode);
out_lock:
unlock_dir(lower_dir_dentry);
out:
@@ -372,7 +331,7 @@ static struct dentry *ecryptfs_lookup(st
"d_name.name = [%s]\n", lower_dentry,
lower_dentry->d_name.name);
lower_inode = lower_dentry->d_inode;
- ecryptfs_copy_attr_atime(dir, lower_dir_dentry->d_inode);
+ fsstack_copy_attr_atime(dir, lower_dir_dentry->d_inode);
BUG_ON(!atomic_read(&lower_dentry->d_count));
ecryptfs_set_dentry_private(dentry,
kmem_cache_alloc(ecryptfs_dentry_info_cache,
@@ -478,7 +437,8 @@ static int ecryptfs_link(struct dentry *
rc = ecryptfs_interpose(lower_new_dentry, new_dentry, dir->i_sb, 0);
if (rc)
goto out_lock;
- ecryptfs_copy_attr_timesizes(dir, lower_new_dentry->d_inode);
+ fsstack_copy_attr_times(dir, lower_new_dentry->d_inode);
+ fsstack_copy_inode_size(dir, lower_new_dentry->d_inode);
old_dentry->d_inode->i_nlink =
ecryptfs_inode_to_lower(old_dentry->d_inode)->i_nlink;
i_size_write(new_dentry->d_inode, file_size_save);
@@ -503,7 +463,7 @@ static int ecryptfs_unlink(struct inode
ecryptfs_printk(KERN_ERR, "Error in vfs_unlink\n");
goto out_unlock;
}
- ecryptfs_copy_attr_times(dir, lower_dir_inode);
+ fsstack_copy_attr_times(dir, lower_dir_inode);
dentry->d_inode->i_nlink =
ecryptfs_inode_to_lower(dentry->d_inode)->i_nlink;
dentry->d_inode->i_ctime = dir->i_ctime;
@@ -542,7 +502,8 @@ static int ecryptfs_symlink(struct inode
rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
if (rc)
goto out_lock;
- ecryptfs_copy_attr_timesizes(dir, lower_dir_dentry->d_inode);
+ fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
+ fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
out_lock:
unlock_dir(lower_dir_dentry);
dput(lower_dentry);
@@ -565,7 +526,8 @@ static int ecryptfs_mkdir(struct inode *
rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
if (rc)
goto out;
- ecryptfs_copy_attr_timesizes(dir, lower_dir_dentry->d_inode);
+ fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
+ fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
dir->i_nlink = lower_dir_dentry->d_inode->i_nlink;
out:
unlock_dir(lower_dir_dentry);
@@ -601,7 +563,7 @@ static int ecryptfs_rmdir(struct inode *
d_delete(tlower_dentry);
tlower_dentry = NULL;
}
- ecryptfs_copy_attr_times(dir, lower_dir_dentry->d_inode);
+ fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
dir->i_nlink = lower_dir_dentry->d_inode->i_nlink;
unlock_dir(lower_dir_dentry);
if (!rc)
@@ -629,7 +591,8 @@ ecryptfs_mknod(struct inode *dir, struct
rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
if (rc)
goto out;
- ecryptfs_copy_attr_timesizes(dir, lower_dir_dentry->d_inode);
+ fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
+ fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
out:
unlock_dir(lower_dir_dentry);
if (!dentry->d_inode)
@@ -658,9 +621,9 @@ ecryptfs_rename(struct inode *old_dir, s
lower_new_dir_dentry->d_inode, lower_new_dentry);
if (rc)
goto out_lock;
- ecryptfs_copy_attr_all(new_dir, lower_new_dir_dentry->d_inode);
+ fsstack_copy_attr_all(new_dir, lower_new_dir_dentry->d_inode);
if (new_dir != old_dir)
- ecryptfs_copy_attr_all(old_dir, lower_old_dir_dentry->d_inode);
+ fsstack_copy_attr_all(old_dir, lower_old_dir_dentry->d_inode);
out_lock:
unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
dput(lower_new_dentry);
@@ -714,8 +677,8 @@ ecryptfs_readlink(struct dentry *dentry,
rc = -EFAULT;
}
kfree(decoded_name);
- ecryptfs_copy_attr_atime(dentry->d_inode,
- lower_dentry->d_inode);
+ fsstack_copy_attr_atime(dentry->d_inode,
+ lower_dentry->d_inode);
}
out_free_lower_buf:
kfree(lower_buf);
@@ -945,7 +908,7 @@ static int ecryptfs_setattr(struct dentr
}
rc = notify_change(lower_dentry, ia);
out:
- ecryptfs_copy_attr_all(inode, lower_inode);
+ fsstack_copy_attr_all(inode, lower_inode);
return rc;
}

diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -35,6 +35,7 @@
#include <linux/pagemap.h>
#include <linux/key.h>
#include <linux/parser.h>
+#include <linux/fs_stack.h>
#include "ecryptfs_kernel.h"

/**
@@ -115,10 +116,10 @@ int ecryptfs_interpose(struct dentry *lo
d_add(dentry, inode);
else
d_instantiate(dentry, inode);
- ecryptfs_copy_attr_all(inode, lower_inode);
+ fsstack_copy_attr_all(inode, lower_inode);
/* This size will be overwritten for real files w/ headers and
* other metadata */
- ecryptfs_copy_inode_size(inode, lower_inode);
+ fsstack_copy_inode_size(inode, lower_inode);
out:
return rc;
}


2006-10-17 05:07:20

by Josef Sipek

[permalink] [raw]
Subject: Re: [PATCH 0 of 2] fsstack: generic... - ignore this copy

On Tue, Oct 17, 2006 at 12:42:26AM -0400, Josef Jeff Sipek wrote:
> From: Josef "Jeff" Sipek <[email protected]>

This was sent accidentally (without all the Cc's for example)...please ignore..

Josef "Jeff" Sipek.

--
Penguin : Linux version 2.4.20-46.9.legacysmp on an i686 machine (5564.00 BogoMips).

2006-10-17 10:55:17

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH 1 of 2] fsstack: Introduce fsstack_copy_{attr,inode}_*


>To: [email protected]

(Superb idea to prekill any Cc, re-adding them)

>+void __fsstack_copy_attr_all(struct inode *dest,
>+ const struct inode *src,
>+ int (*get_nlinks)(struct inode *))
>+{
>[big]
>+}
>+
>+/* externs for fs/stack.c */
>+extern void __fsstack_copy_attr_all(struct inode *dest,
>+ const struct inode *src,
>+ int (*get_nlinks)(struct inode *));
>+
>+static inline void fsstack_copy_attr_all(struct inode *dest,
>+ const struct inode *src)
>+{
>+ __fsstack_copy_attr_all(dest, src, NULL);
>+}

Do we really need this indirection? Can't __fsstack_copy_attr_all be
named fsstack_copy_attr_all instead?


-`J'
--

2006-10-17 20:35:06

by Josef Sipek

[permalink] [raw]
Subject: Re: [PATCH 1 of 2] fsstack: Introduce fsstack_copy_{attr,inode}_*

On Tue, Oct 17, 2006 at 12:40:31PM +0200, Jan Engelhardt wrote:
>
> >To: [email protected]
>
> (Superb idea to prekill any Cc, re-adding them)

Yeah, I like to test the emails but I accidentally sent them. From now on,
no testing :)

> >+void __fsstack_copy_attr_all(struct inode *dest,
> >+ const struct inode *src,
> >+ int (*get_nlinks)(struct inode *))
> >+{
> >[big]
> >+}
> >+
> >+/* externs for fs/stack.c */
> >+extern void __fsstack_copy_attr_all(struct inode *dest,
> >+ const struct inode *src,
> >+ int (*get_nlinks)(struct inode *));
> >+
> >+static inline void fsstack_copy_attr_all(struct inode *dest,
> >+ const struct inode *src)
> >+{
> >+ __fsstack_copy_attr_all(dest, src, NULL);
> >+}
>
> Do we really need this indirection? Can't __fsstack_copy_attr_all be
> named fsstack_copy_attr_all instead?

I suppose it could. There is no API-breakage to avoid.

Josef "Jeff" Sipek.

--
Computer Science is no more about computers than astronomy is about
telescopes.
- Edsger Dijkstra