From: David Howells <[email protected]>
Stop the UnionFS filesystem from using iget() and read_inode(). Replace
unionfs_read_inode() with unionfs_iget(), and call that instead of iget().
unionfs_iget() then uses iget_locked() directly and returns a proper error code
instead of an inode in the event of an error.
unionfs_fill_super() returns any error incurred when getting the root inode
instead of EINVAL.
Signed-off-by: David Howells <[email protected]>
---
fs/unionfs/main.c | 11 +++++------
fs/unionfs/super.c | 23 +++++++++++++++++------
fs/unionfs/union.h | 1 +
3 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 4faae44..8d28520 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -104,9 +104,8 @@ struct dentry *unionfs_interpose(struct dentry *dentry, struct super_block *sb,
BUG_ON(is_negative_dentry);
/*
- * We allocate our new inode below, by calling iget.
- * iget will call our read_inode which will initialize some
- * of the new inode's fields
+ * We allocate our new inode below by calling unionfs_iget,
+ * which will initialize some of the new inode's fields
*/
/*
@@ -128,9 +127,9 @@ struct dentry *unionfs_interpose(struct dentry *dentry, struct super_block *sb,
}
} else {
/* get unique inode number for unionfs */
- inode = iget(sb, iunique(sb, UNIONFS_ROOT_INO));
- if (!inode) {
- err = -EACCES;
+ inode = unionfs_iget(sb, iunique(sb, UNIONFS_ROOT_INO));
+ if (IS_ERR(inode)) {
+ err = PTR_ERR(inode);
goto out;
}
if (atomic_read(&inode->i_count) > 1)
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 4e0fe7c..c9d4562 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -24,14 +24,22 @@
*/
static struct kmem_cache *unionfs_inode_cachep;
-static void unionfs_read_inode(struct inode *inode)
+struct inode *unionfs_iget(struct super_block *sb, unsigned long ino)
{
extern struct address_space_operations unionfs_aops;
int size;
- struct unionfs_inode_info *info = UNIONFS_I(inode);
+ struct unionfs_inode_info *info;
+ struct inode *inode;
- unionfs_read_lock(inode->i_sb);
+ inode = iget_locked(sb, ino);
+ if (!inode)
+ return ERR_PTR(-ENOMEM);
+ if (!(inode->i_state & I_NEW))
+ return inode;
+ unionfs_read_lock(sb);
+
+ info = UNIONFS_I(inode);
memset(info, 0, offsetof(struct unionfs_inode_info, vfs_inode));
info->bstart = -1;
info->bend = -1;
@@ -47,7 +55,9 @@ static void unionfs_read_inode(struct inode *inode)
if (!info->lower_inodes) {
printk(KERN_ERR "unionfs: no kernel memory when allocating "
"lower-pointer array!\n");
- BUG();
+ iget_failed(inode);
+ unionfs_read_unlock(sb);
+ return ERR_PTR(-ENOMEM);
}
inode->i_version++;
@@ -56,7 +66,9 @@ static void unionfs_read_inode(struct inode *inode)
inode->i_mapping->a_ops = &unionfs_aops;
- unionfs_read_unlock(inode->i_sb);
+ unlock_new_inode(inode);
+ unionfs_read_unlock(sb);
+ return inode;
}
/*
@@ -996,7 +1008,6 @@ out:
}
struct super_operations unionfs_sops = {
- .read_inode = unionfs_read_inode,
.delete_inode = unionfs_delete_inode,
.put_super = unionfs_put_super,
.statfs = unionfs_statfs,
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 97da8f9..2653802 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -322,6 +322,7 @@ extern int unionfs_fsync(struct file *file, struct dentry *dentry,
extern int unionfs_fasync(int fd, struct file *file, int flag);
/* Inode operations */
+extern struct inode *unionfs_iget(struct super_block *sb, unsigned long ino);
extern int unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct inode *new_dir, struct dentry *new_dentry);
extern int unionfs_unlink(struct inode *dir, struct dentry *dentry);
In message <[email protected]>, David Howells writes:
> From: David Howells <[email protected]>
>
> Stop the UnionFS filesystem from using iget() and read_inode(). Replace
> unionfs_read_inode() with unionfs_iget(), and call that instead of iget().
> unionfs_iget() then uses iget_locked() directly and returns a proper error code
> instead of an inode in the event of an error.
>
> unionfs_fill_super() returns any error incurred when getting the root inode
> instead of EINVAL.
>
> Signed-off-by: David Howells <[email protected]>
[...]
Thanks. I tested this code and it passed all my tests. I'll shortly submit
a slightly revised patch which applies cleanly against the unionfs code in
-mm.
Cheers,
Erez.
On Wed, 2007-11-07 at 01:44 -0500, Erez Zadok wrote:
> In message <[email protected]>, David Howells writes:
> > From: David Howells <[email protected]>
> >
> > Stop the UnionFS filesystem from using iget() and read_inode(). Replace
> > unionfs_read_inode() with unionfs_iget(), and call that instead of iget().
> > unionfs_iget() then uses iget_locked() directly and returns a proper error code
> > instead of an inode in the event of an error.
> >
> > unionfs_fill_super() returns any error incurred when getting the root inode
> > instead of EINVAL.
> >
> > Signed-off-by: David Howells <[email protected]>
> [...]
>
> Thanks. I tested this code and it passed all my tests. I'll shortly submit
> a slightly revised patch which applies cleanly against the unionfs code in
> -mm.
Does your test set include readahead-list? I can't get it to work with a
union mounted filesystem without segfault'ing and kernel oops'ing.
Paul Albrecht
Paul Albrecht <[email protected]> wrote:
> Does your test set include readahead-list? I can't get it to work with a
> union mounted filesystem without segfault'ing and kernel oops'ing.
Is that with or without this patch?
David
On Wed, 2007-11-07 at 15:04 +0000, David Howells wrote:
> Paul Albrecht <[email protected]> wrote:
>
> > Does your test set include readahead-list? I can't get it to work with a
> > union mounted filesystem without segfault'ing and kernel oops'ing.
>
> Is that with or without this patch?
Without ... what's the difference? I asked a specific question about the
test set that was used to validate unionfs and your fix.
Paul Albrecht
Paul Albrecht <[email protected]> wrote:
> Without ... what's the difference? I asked a specific question about the
> test set that was used to validate unionfs and your fix.
I was just checking it wasn't something my patch introduced.
David
In message <[email protected]>, Paul Albrecht writes:
> On Wed, 2007-11-07 at 15:04 +0000, David Howells wrote:
> > Paul Albrecht <[email protected]> wrote:
> >
> > > Does your test set include readahead-list? I can't get it to work with a
> > > union mounted filesystem without segfault'ing and kernel oops'ing.
> >
> > Is that with or without this patch?
>
> Without ... what's the difference? I asked a specific question about the
> test set that was used to validate unionfs and your fix.
>
> Paul Albrecht
Paul,
I'll check out the readahead stuff shortly. I doubt the iget patch has
anything to do with it.
Cheers,
Erez.