Re: [RFC PATCH] overlayfs: support more than one read-only layer

[Erez Zadok]

Indeed, Miklos, changing layers dynamically is VERY tricky.  Unionfs supports 
that, but a good amount of its code all over is dedicated to handling dynamic 
layer changes.  I advise you to avoid supporting this feature for a while, if 
ever: get a small, functional, working overlayfs first.

I personally never liked the idea of changing layers dynamically: it’s like 
asking a file system to remain consistent and detect all lower changes when 
someone goes and hand-edits disk blocks.  Too messy.  The pain of developing 
and supporting such a feature is much greater than the annoyance to users who 
have to unmount and remount back their layers to form a new structure.

Cheers,
Erez.

> On Nov 10, 2014, at 4:09 AM, Miklos Szeredi  wrote:
> 
> As for changing the stacking while the overlayfs is mounted: currently this is
> not supported, the layers specified at the mount time remain there until the
> overlay is unmounted.  Currently there's no possibility to add or remove 
> layers
> in a dynamic way, and it is definitely more tricky to implement than the 
> static
> configuration.
> 
> Thanks,
> Miklos

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] overlayfs: support more than one read-only layer

[Erez Zadok]

Indeed, Miklos, changing layers dynamically is VERY tricky.  Unionfs supports 
that, but a good amount of its code all over is dedicated to handling dynamic 
layer changes.  I advise you to avoid supporting this feature for a while, if 
ever: get a small, functional, working overlayfs first.

I personally never liked the idea of changing layers dynamically: it’s like 
asking a file system to remain consistent and detect all lower changes when 
someone goes and hand-edits disk blocks.  Too messy.  The pain of developing 
and supporting such a feature is much greater than the annoyance to users who 
have to unmount and remount back their layers to form a new structure.

Cheers,
Erez.

 On Nov 10, 2014, at 4:09 AM, Miklos Szeredi mik...@szeredi.hu wrote:
 
 As for changing the stacking while the overlayfs is mounted: currently this is
 not supported, the layers specified at the mount time remain there until the
 overlay is unmounted.  Currently there's no possibility to add or remove 
 layers
 in a dynamic way, and it is definitely more tricky to implement than the 
 static
 configuration.
 
 Thanks,
 Miklos

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/9] overlay filesystem: request for inclusion (v17)

[Erez Zadok]

I tend to agree with Al's and Linus's POV regarding whiteouts. There are three 
general techniques to implementing whiteouts:

1. namespace: special file names, hard/symlinks, or special "hidden" dot files.
2. extended attributes.
3. DT_WHT dirent flags.
(there's actually a 4th method I've tried before that I won't discuss below: 
implementing your own data structures on a raw partition — way too cumbersome.)

The namespace techniques require lower file systems to support hard/symlinks 
and sometimes need long names. A plus: they work on most file systems (but not 
all).  But they cause all sorts of namespace ugliness, where you have to hide 
the special file names, avoid them, ensure atomic updates for ops that involve 
whiteouts.  It's all doable, as had been demonstrated by several 
implementations.  But it's still icky in terms of namespace pollution.

The extended attributes technique, I think, is better than the namespace one in 
that you don't pollute the namespace; plus, I think the EA technique minimizes 
atomicity issues that show up with the namespace method.  Yet, it still 
requires EA support in lower file systems, so it won't work unless lower file 
systems support xattr ops.  Plus it could fail for file systems that have 
limited xattr support (e.g., number of EAs per inode).

The DT_WHT technique is the cleanest in the long run, and the best of the three 
IMHO.  It's well understood and has been done in BSD a long time ago.  It 
doesn't have the namespace pollution as seen in technique #1 above.  And I 
believe it also minimize atomicity issues.  Plus you won't have issues running 
out of EAs.

A while back I've looked at the unionmount code for DT_WHT support for 
ext2/tmpfs, and it was small, clean, and mostly additive.  I even had a 
prototype port of unionfs using unionmount's DT_WHT support: it was relatively 
easy to port unionfs to use DT_WHT instead of namespace techniques. Plus, I was 
able to reduce the amount of code devoted to whiteout support by quite a bit.

So I think it'll be easy to port overlayfs to use DT_WHT.  Given that most 
people use unioning with ext* and tmpfs, minimal DT_WHT support in those would 
get most users happy initially. And we can then let other file systems support 
DT_WHT on their own, in whatever way they deem suitable (as Al suggested, this 
is really best deferred to the F/S to implement).

Lastly, what I'm not sure is what API to use for whiteouts: should every f/s 
implement some new methods to add/remove/query a whiteout, or should the upper 
f/s and VFS directly check DT_WHT flags with S_ISWHT.  The generic f/s methods 
may allow file systems to implement whiteouts in arbitrary ways, not 
necessarily as a dirent flag.

Cheers,
Erez.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 0/9] overlay filesystem: request for inclusion (v17)

[Erez Zadok]

I tend to agree with Al's and Linus's POV regarding whiteouts. There are three 
general techniques to implementing whiteouts:

1. namespace: special file names, hard/symlinks, or special hidden dot files.
2. extended attributes.
3. DT_WHT dirent flags.
(there's actually a 4th method I've tried before that I won't discuss below: 
implementing your own data structures on a raw partition — way too cumbersome.)

The namespace techniques require lower file systems to support hard/symlinks 
and sometimes need long names. A plus: they work on most file systems (but not 
all).  But they cause all sorts of namespace ugliness, where you have to hide 
the special file names, avoid them, ensure atomic updates for ops that involve 
whiteouts.  It's all doable, as had been demonstrated by several 
implementations.  But it's still icky in terms of namespace pollution.

The extended attributes technique, I think, is better than the namespace one in 
that you don't pollute the namespace; plus, I think the EA technique minimizes 
atomicity issues that show up with the namespace method.  Yet, it still 
requires EA support in lower file systems, so it won't work unless lower file 
systems support xattr ops.  Plus it could fail for file systems that have 
limited xattr support (e.g., number of EAs per inode).

The DT_WHT technique is the cleanest in the long run, and the best of the three 
IMHO.  It's well understood and has been done in BSD a long time ago.  It 
doesn't have the namespace pollution as seen in technique #1 above.  And I 
believe it also minimize atomicity issues.  Plus you won't have issues running 
out of EAs.

A while back I've looked at the unionmount code for DT_WHT support for 
ext2/tmpfs, and it was small, clean, and mostly additive.  I even had a 
prototype port of unionfs using unionmount's DT_WHT support: it was relatively 
easy to port unionfs to use DT_WHT instead of namespace techniques. Plus, I was 
able to reduce the amount of code devoted to whiteout support by quite a bit.

So I think it'll be easy to port overlayfs to use DT_WHT.  Given that most 
people use unioning with ext* and tmpfs, minimal DT_WHT support in those would 
get most users happy initially. And we can then let other file systems support 
DT_WHT on their own, in whatever way they deem suitable (as Al suggested, this 
is really best deferred to the F/S to implement).

Lastly, what I'm not sure is what API to use for whiteouts: should every f/s 
implement some new methods to add/remove/query a whiteout, or should the upper 
f/s and VFS directly check DT_WHT flags with S_ISWHT.  The generic f/s methods 
may allow file systems to implement whiteouts in arbitrary ways, not 
necessarily as a dirent flag.

Cheers,
Erez.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [2.6 patch] make vfs_ioctl() static

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Christoph Hellwig writes:
> On Sun, Feb 17, 2008 at 10:18:42AM +0200, Adrian Bunk wrote:
> > This patch makes the needlessly global vfs_ioctl() static.
> 
> I think the point was toa eventually export it for stackable filesystem
> use.  But until they start using it marking it static seems fine with
> me.

Right.  I'm not using it yet in unionfs, although I could; for now I'm just
calling very similar code myself.  This is only used in unionfs after I
process my own ioctls; IOW, I pass all unknown ioctls to the lower level and
let it handle it.

eCryptfs, however, doesn't pass unknown ioctls to the lower layer: it only
processes its own.

Honestly I'm not sure which is more appropriate: should a stackable f/s pass
unknown ioctls to the lower f/s or not?  If it doesn't, would any important
functionality be lost?

Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [2.6 patch] make vfs_ioctl() static

[Erez Zadok]

In message [EMAIL PROTECTED], Christoph Hellwig writes:
 On Sun, Feb 17, 2008 at 10:18:42AM +0200, Adrian Bunk wrote:
  This patch makes the needlessly global vfs_ioctl() static.
 
 I think the point was toa eventually export it for stackable filesystem
 use.  But until they start using it marking it static seems fine with
 me.

Right.  I'm not using it yet in unionfs, although I could; for now I'm just
calling very similar code myself.  This is only used in unionfs after I
process my own ioctls; IOW, I pass all unknown ioctls to the lower level and
let it handle it.

eCryptfs, however, doesn't pass unknown ioctls to the lower layer: it only
processes its own.

Honestly I'm not sure which is more appropriate: should a stackable f/s pass
unknown ioctls to the lower f/s or not?  If it doesn't, would any important
functionality be lost?

Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: unionfs_copy_attr_times oopses

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Hugh Dickins writes:

> Hi Erez,
> 
> Aside from the occasional "unionfs: new lower inode mtime" messages
> on directories (which I've got into the habit of ignoring now), the
> only problem I'm still suffering with unionfs over tmpfs (not tested
> any other fs's below it recently) is oops in unionfs_copy_attr_times.
> 
> I believe I'm working with your latest: 2.6.24-rc8-mm1 plus the four
> patches you posted to lkml on 26 Jan.  But this problem has been around
> for a while before that: I'd been hoping to debug it myself, but taken
> too long to make too little progress, so now handing over to you.
> 
> The oops occurs while doing repeated "make -j20" kernel builds in a
> unionfs mount of a tmpfs (though I doubt tmpfs is relevant): most of
> my testing was while swapping, but today I find that's irrelevant,
> and it should happen much quicker without.  SMP kernels (4 cpus),
> I haven't tried UP; happens with or without PREEMPT, may just be
> coincidence that it happens quicker on the machines with PREEMPT.
> 
> Most commonly it's unionfs_copy_attr_times called from unionfs_create,
> but that's probably just the most common route in this workload:
> I've seen it also when called from unionfs_rename or unionfs_open or
> unionfs_unlink.  It looks like there's a locking or refcounting bug,
> hence a race: the unionfs_inode_info which unionfs_copy_attr_times
> is working on gets changed underneath it, so it oopses on NULL
> lower_inodes.
[...]

Hugh,

Check out my latest set of patches (which correspond to release 2.2.4 of
Unionfs).  Thanks to your info and the patch, I was able to trigger several
races more frequently, and fix them.  I've tested my code with make -j N
(for N=4 and N=20), on a 4 cpu machine a well as a 2 cpu machine (w/
different amounts of memory and CPU speeds, also 32-bit vs 64-bit); I ran a
kernel compile for ~10-12 hours.  With the patches I just posted, I wasn't
able to trigger any of the WARN_ON's in unionfs_copy_attr_times.  I also
tried it while flushing caches via /proc, and/or performing branch-mgmt
commands in unionfs.

Give it a good shake and let me know what you find.

Thanks,
Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 11/17] Unionfs: lock parents' branch configuration fixes

[Erez Zadok]

Ensure that we lock the branch configuration of parent and child dentries in
operations which need it, and in the right order.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/commonfops.c |   31 +---
 fs/unionfs/dentry.c |   26 +---
 fs/unionfs/inode.c  |   58 +++---
 fs/unionfs/union.h  |5 ++-
 fs/unionfs/unlink.c |   11 -
 5 files changed, 91 insertions(+), 40 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 96473c4..491e2ff 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -300,8 +300,9 @@ out:
  * Revalidate the struct file
  * @file: file to revalidate
  * @willwrite: true if caller may cause changes to the file; false otherwise.
+ * Caller must lock/unlock dentry's branch configuration.
  */
-int unionfs_file_revalidate(struct file *file, bool willwrite)
+int unionfs_file_revalidate_locked(struct file *file, bool willwrite)
 {
struct super_block *sb;
struct dentry *dentry;
@@ -311,7 +312,6 @@ int unionfs_file_revalidate(struct file *file, bool 
willwrite)
int err = 0;
 
dentry = file->f_path.dentry;
-   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
sb = dentry->d_sb;
 
/*
@@ -416,7 +416,17 @@ out:
 out_nofree:
if (!err)
unionfs_check_file(file);
-   unionfs_unlock_dentry(dentry);
+   return err;
+}
+
+int unionfs_file_revalidate(struct file *file, bool willwrite)
+{
+   int err;
+
+   unionfs_lock_dentry(file->f_path.dentry, UNIONFS_DMUTEX_CHILD);
+   err = unionfs_file_revalidate_locked(file, willwrite);
+   unionfs_unlock_dentry(file->f_path.dentry);
+
return err;
 }
 
@@ -524,9 +534,18 @@ int unionfs_open(struct inode *inode, struct file *file)
struct dentry *dentry = file->f_path.dentry;
int bindex = 0, bstart = 0, bend = 0;
int size;
+   int valid = 0;
 
unionfs_read_lock(inode->i_sb, UNIONFS_SMUTEX_PARENT);
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+   if (dentry != dentry->d_parent)
+   unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT);
+
+   valid = __unionfs_d_revalidate_chain(dentry->d_parent, NULL, false);
+   if (unlikely(!valid)) {
+   err = -ESTALE;
+   goto out_nofree;
+   }
 
file->private_data =
kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
@@ -589,6 +608,8 @@ out_nofree:
unionfs_check_file(file);
unionfs_check_inode(inode);
}
+   if (dentry != dentry->d_parent)
+   unionfs_unlock_dentry(dentry->d_parent);
unionfs_unlock_dentry(dentry);
unionfs_read_unlock(inode->i_sb);
return err;
@@ -797,8 +818,9 @@ int unionfs_flush(struct file *file, fl_owner_t id)
int bindex, bstart, bend;
 
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
-   err = unionfs_file_revalidate(file, true);
+   err = unionfs_file_revalidate_locked(file, true);
if (unlikely(err))
goto out;
unionfs_check_file(file);
@@ -824,6 +846,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
 
 out:
unionfs_check_file(file);
+   unionfs_unlock_dentry(file->f_path.dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
 }
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 3bd2dfb..17b297d 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -363,6 +363,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, 
struct nameidata *nd,
chain_len = 0;
sbgen = atomic_read(_SB(dentry->d_sb)->generation);
dtmp = dentry->d_parent;
+   verify_locked(dentry);
if (dentry != dtmp)
unionfs_lock_dentry(dtmp, UNIONFS_DMUTEX_REVAL_PARENT);
dgen = atomic_read(_D(dtmp)->generation);
@@ -453,7 +454,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, 
struct nameidata *nd,
 
 out_this:
/* finally, lock this dentry and revalidate it */
-   verify_locked(dentry);
+   verify_locked(dentry);  /* verify child is locked */
if (dentry != dentry->d_parent)
unionfs_lock_dentry(dentry->d_parent,
UNIONFS_DMUTEX_REVAL_PARENT);
@@ -491,24 +492,20 @@ static int unionfs_d_revalidate(struct dentry *dentry, 
struct nameidata *nd)
return err;
 }
 
-/*
- * At this point no one can reference this dentry, so we don't have to be
- * careful about concurrent access.
- */
 static void unionfs_d_release(struct dentry *dentry)
 {
int bindex, bstart, bend;
 
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+   /* must lock our branch con

[PATCH 14/17] Unionfs: stop using iget() and read_inode()

[Erez Zadok]

From: David Howells <[EMAIL PROTECTED]>

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]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/main.c  |   11 +--
 fs/unionfs/super.c |   19 ++-
 fs/unionfs/union.h |1 +
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index ba3471d..4bc2c66 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(>i_count) > 1)
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 175840f..b71fc2a 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -24,11 +24,19 @@
  */
 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)
 {
int size;
-   struct unionfs_inode_info *info = UNIONFS_I(inode);
+   struct unionfs_inode_info *info;
+   struct inode *inode;
 
+   inode = iget_locked(sb, ino);
+   if (!inode)
+   return ERR_PTR(-ENOMEM);
+   if (!(inode->i_state & I_NEW))
+   return inode;
+
+   info = UNIONFS_I(inode);
memset(info, 0, offsetof(struct unionfs_inode_info, vfs_inode));
info->bstart = -1;
info->bend = -1;
@@ -44,7 +52,8 @@ static void unionfs_read_inode(struct inode *inode)
if (unlikely(!info->lower_inodes)) {
printk(KERN_CRIT "unionfs: no kernel memory when allocating "
   "lower-pointer array!\n");
-   BUG();
+   iget_failed(inode);
+   return ERR_PTR(-ENOMEM);
}
 
inode->i_version++;
@@ -60,7 +69,8 @@ static void unionfs_read_inode(struct inode *inode)
inode->i_atime.tv_sec = inode->i_atime.tv_nsec = 0;
inode->i_mtime.tv_sec = inode->i_mtime.tv_nsec = 0;
inode->i_ctime.tv_sec = inode->i_ctime.tv_nsec = 0;
-
+   unlock_new_inode(inode);
+   return inode;
 }
 
 /*
@@ -1025,7 +1035,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 1bf0c09..533806c 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -364,6 +364,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);
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 06/17] Unionfs: extend dentry branch configuration lock in open

[Erez Zadok]

Dentry branch configuration "info node" lock should extend to calls to
copy_attr_times.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/commonfops.c |   14 --
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index f37192f..96473c4 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -521,11 +521,12 @@ int unionfs_open(struct inode *inode, struct file *file)
 {
int err = 0;
struct file *lower_file = NULL;
-   struct dentry *dentry = NULL;
+   struct dentry *dentry = file->f_path.dentry;
int bindex = 0, bstart = 0, bend = 0;
int size;
 
unionfs_read_lock(inode->i_sb, UNIONFS_SMUTEX_PARENT);
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
file->private_data =
kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
@@ -551,9 +552,6 @@ int unionfs_open(struct inode *inode, struct file *file)
goto out;
}
 
-   dentry = file->f_path.dentry;
-   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
-
bstart = fbstart(file) = dbstart(dentry);
bend = fbend(file) = dbend(dentry);
 
@@ -573,15 +571,12 @@ int unionfs_open(struct inode *inode, struct file *file)
if (!lower_file)
continue;
 
-   branchput(file->f_path.dentry->d_sb, bindex);
+   branchput(dentry->d_sb, bindex);
/* fput calls dput for lower_dentry */
fput(lower_file);
}
}
 
-   /* XXX: should this unlock be moved to the function bottom? */
-   unionfs_unlock_dentry(dentry);
-
 out:
if (err) {
kfree(UNIONFS_F(file)->lower_files);
@@ -590,12 +585,11 @@ out:
}
 out_nofree:
if (!err) {
-   dentry = file->f_path.dentry;
-   unionfs_copy_attr_times(dentry->d_parent->d_inode);
unionfs_copy_attr_times(inode);
unionfs_check_file(file);
unionfs_check_inode(inode);
}
+   unionfs_unlock_dentry(dentry);
unionfs_read_unlock(inode->i_sb);
return err;
 }
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 15/17] Unionfs: embed a struct path into struct nameidata instead of nd dentrymnt

[Erez Zadok]

From: Andrew Morton <[EMAIL PROTECTED]>

Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/inode.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 2e791fd..640969d 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -254,8 +254,8 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 
/* save the dentry & vfsmnt from namei */
if (nd) {
-   path_save.dentry = nd->dentry;
-   path_save.mnt = nd->mnt;
+   path_save.dentry = nd->path.dentry;
+   path_save.mnt = nd->path.mnt;
}
 
/*
@@ -266,8 +266,8 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 
/* restore the dentry & vfsmnt in namei */
if (nd) {
-   nd->dentry = path_save.dentry;
-   nd->mnt = path_save.mnt;
+   nd->path.dentry = path_save.dentry;
+   nd->path.mnt = path_save.mnt;
}
if (!IS_ERR(ret)) {
if (ret)
@@ -885,7 +885,7 @@ static int unionfs_permission(struct inode *inode, int mask,
const int write_mask = (mask & MAY_WRITE) && !(mask & MAY_READ);
 
if (nd)
-   unionfs_lock_dentry(nd->dentry, UNIONFS_DMUTEX_CHILD);
+   unionfs_lock_dentry(nd->path.dentry, UNIONFS_DMUTEX_CHILD);
 
if (!UNIONFS_I(inode)->lower_inodes) {
if (is_file)/* dirs can be unlinked but chdir'ed to */
@@ -960,7 +960,7 @@ out:
unionfs_check_inode(inode);
unionfs_check_nd(nd);
if (nd)
-   unionfs_unlock_dentry(nd->dentry);
+   unionfs_unlock_dentry(nd->path.dentry);
return err;
 }
 
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 13/17] Unionfs: use dget_parent in revalidation code

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/dentry.c |   13 -
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index a956b94..f8f65e1 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -410,15 +410,10 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, 
struct nameidata *nd,
goto out;
}
 
-   /*
-* lock all dentries in chain, in child to parent order.
-* if failed, then sleep for a little, then retry.
-*/
-   dtmp = dentry->d_parent;
-   for (i = chain_len-1; i >= 0; i--) {
-   chain[i] = dget(dtmp);
-   dtmp = dtmp->d_parent;
-   }
+   /* grab all dentries in chain, in child to parent order */
+   dtmp = dentry;
+   for (i = chain_len-1; i >= 0; i--)
+   dtmp = chain[i] = dget_parent(dtmp);
 
/*
 * call __unionfs_d_revalidate_one() on each dentry, but in parent
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 05/17] Unionfs: initialize path_save variable

[Erez Zadok]

This is not strictly necessary, but it helps quiet a gcc-4.2 warning (a good
optimizer may optimize this initialization away).

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
Signed-off-by: Josef 'Jeff' Sipek <[EMAIL PROTECTED]>
---
 fs/unionfs/inode.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 0b92da2..8d939dc 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -246,7 +246,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 struct dentry *dentry,
 struct nameidata *nd)
 {
-   struct path path_save;
+   struct path path_save = {NULL, NULL};
struct dentry *ret;
 
unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 10/17] Unionfs: factor out revalidation routine

[Erez Zadok]

To be used by rest of revalidation code, as well a callers who already
locked the child and parent dentry branch-configurations.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/dentry.c |   87 +++---
 fs/unionfs/union.h  |3 ++
 2 files changed, 57 insertions(+), 33 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index afa2120..3bd2dfb 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -285,6 +285,59 @@ void purge_sb_data(struct super_block *sb)
 }
 
 /*
+ * Revalidate a single file/symlink/special dentry.  Assume that info nodes
+ * of the dentry and its parent are locked.  Assume that parent(s) are all
+ * valid already, but the child may not yet be valid.  Returns true if
+ * valid, false otherwise.
+ */
+bool __unionfs_d_revalidate_one_locked(struct dentry *dentry,
+  struct nameidata *nd,
+  bool willwrite)
+{
+   bool valid = false; /* default is invalid */
+   int sbgen, dgen, bindex;
+
+   verify_locked(dentry);
+   verify_locked(dentry->d_parent);
+
+   sbgen = atomic_read(_SB(dentry->d_sb)->generation);
+   dgen = atomic_read(_D(dentry)->generation);
+
+   if (unlikely(is_newer_lower(dentry))) {
+   /* root dentry special case as aforementioned */
+   if (IS_ROOT(dentry)) {
+   unionfs_copy_attr_times(dentry->d_inode);
+   } else {
+   /*
+* reset generation number to zero, guaranteed to be
+* "old"
+*/
+   dgen = 0;
+   atomic_set(_D(dentry)->generation, dgen);
+   }
+   if (!willwrite)
+   purge_inode_data(dentry->d_inode);
+   }
+   valid = __unionfs_d_revalidate_one(dentry, nd);
+
+   /*
+* If __unionfs_d_revalidate_one() succeeded above, then it will
+* have incremented the refcnt of the mnt's, but also the branch
+* indices of the dentry will have been updated (to take into
+* account any branch insertions/deletion.  So the current
+* dbstart/dbend match the current, and new, indices of the mnts
+* which __unionfs_d_revalidate_one has incremented.  Note: the "if"
+* test below does not depend on whether chain_len was 0 or greater.
+*/
+   if (!valid || sbgen == dgen)
+   goto out;
+   for (bindex = dbstart(dentry); bindex <= dbend(dentry); bindex++)
+   unionfs_mntput(dentry, bindex);
+out:
+   return valid;
+}
+
+/*
  * Revalidate a parent chain of dentries, then the actual node.
  * Assumes that dentry is locked, but will lock all parents if/when needed.
  *
@@ -404,42 +457,10 @@ out_this:
if (dentry != dentry->d_parent)
unionfs_lock_dentry(dentry->d_parent,
UNIONFS_DMUTEX_REVAL_PARENT);
-   dgen = atomic_read(_D(dentry)->generation);
-
-   if (unlikely(is_newer_lower(dentry))) {
-   /* root dentry special case as aforementioned */
-   if (IS_ROOT(dentry)) {
-   unionfs_copy_attr_times(dentry->d_inode);
-   } else {
-   /*
-* reset generation number to zero, guaranteed to be
-* "old"
-*/
-   dgen = 0;
-   atomic_set(_D(dentry)->generation, dgen);
-   }
-   if (!willwrite)
-   purge_inode_data(dentry->d_inode);
-   }
-   valid = __unionfs_d_revalidate_one(dentry, nd);
+   valid = __unionfs_d_revalidate_one_locked(dentry, nd, willwrite);
if (dentry != dentry->d_parent)
unionfs_unlock_dentry(dentry->d_parent);
 
-   /*
-* If __unionfs_d_revalidate_one() succeeded above, then it will
-* have incremented the refcnt of the mnt's, but also the branch
-* indices of the dentry will have been updated (to take into
-* account any branch insertions/deletion.  So the current
-* dbstart/dbend match the current, and new, indices of the mnts
-* which __unionfs_d_revalidate_one has incremented.  Note: the "if"
-* test below does not depend on whether chain_len was 0 or greater.
-*/
-   if (valid && sbgen != dgen)
-   for (bindex = dbstart(dentry);
-bindex <= dbend(dentry);
-bindex++)
-   unionfs_mntput(dentry, bindex);
-
 out_free:
/* unlock/dput all dentries in chain and return status */
if (chain_len > 0) {
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 0e0ccde..50

[PATCH 08/17] Unionfs: improve debugging in copy_attr_times

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/subr.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
index 68a6280..1a40f63 100644
--- a/fs/unionfs/subr.c
+++ b/fs/unionfs/subr.c
@@ -247,8 +247,14 @@ void unionfs_copy_attr_times(struct inode *upper)
int bindex;
struct inode *lower;
 
-   if (!upper || ibstart(upper) < 0)
+   if (!upper)
return;
+   if (ibstart(upper) < 0) {
+#ifdef CONFIG_UNION_FS_DEBUG
+   WARN_ON(ibstart(upper) < 0);
+#endif /* CONFIG_UNION_FS_DEBUG */
+   return;
+   }
for (bindex = ibstart(upper); bindex <= ibend(upper); bindex++) {
lower = unionfs_lower_inode_idx(upper, bindex);
if (!lower)
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 16/17] Unionfs: use the new path_put

[Erez Zadok]

From: Jan Blunck <[EMAIL PROTECTED]>

* Add path_put() functions for releasing a reference to the dentry and
  vfsmount of a struct path in the right order

* Switch from path_release(nd) to path_put(>path)

* Rename dput_path() to path_put_conditional()

Signed-off-by: Jan Blunck <[EMAIL PROTECTED]>
Signed-off-by: Andreas Gruenbacher <[EMAIL PROTECTED]>
Acked-by: Christoph Hellwig <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/main.c  |2 +-
 fs/unionfs/super.c |   12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 4bc2c66..3585b29 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -371,7 +371,7 @@ static int parse_dirs_option(struct super_block *sb, struct 
unionfs_dentry_info
if (err) {
printk(KERN_ERR "unionfs: lower directory "
   "'%s' is not a valid branch\n", name);
-   path_release();
+   path_put();
goto out;
}
 
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index b71fc2a..773623e 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -234,7 +234,7 @@ static noinline int do_remount_mode_option(char *optarg, 
int cur_branches,
if (nd.mnt == new_lower_paths[idx].mnt &&
nd.dentry == new_lower_paths[idx].dentry)
break;
-   path_release();  /* no longer needed */
+   path_put(); /* no longer needed */
if (idx == cur_branches) {
err = -ENOENT;  /* err may have been reset above */
printk(KERN_ERR "unionfs: branch \"%s\" "
@@ -277,7 +277,7 @@ static noinline int do_remount_del_option(char *optarg, int 
cur_branches,
if (nd.mnt == new_lower_paths[idx].mnt &&
nd.dentry == new_lower_paths[idx].dentry)
break;
-   path_release();  /* no longer needed */
+   path_put(); /* no longer needed */
if (idx == cur_branches) {
printk(KERN_ERR "unionfs: branch \"%s\" "
   "not found\n", optarg);
@@ -296,7 +296,7 @@ static noinline int do_remount_del_option(char *optarg, int 
cur_branches,
 * new_data and new_lower_paths one to the left.  Finally, adjust
 * cur_branches.
 */
-   pathput(_lower_paths[idx]);
+   path_put(_lower_paths[idx]);
 
if (idx < cur_branches - 1) {
/* if idx==cur_branches-1, we delete last branch: easy */
@@ -361,7 +361,7 @@ static noinline int do_remount_add_option(char *optarg, int 
cur_branches,
if (nd.mnt == new_lower_paths[idx].mnt &&
nd.dentry == new_lower_paths[idx].dentry)
break;
-   path_release();  /* no longer needed */
+   path_put(); /* no longer needed */
if (idx == cur_branches) {
printk(KERN_ERR "unionfs: branch \"%s\" "
   "not found\n", optarg);
@@ -408,7 +408,7 @@ found_insertion_point:
if (err) {
printk(KERN_ERR "unionfs: lower directory "
   "\"%s\" is not a valid branch\n", optarg);
-   path_release();
+   path_put();
goto out;
}
 
@@ -818,7 +818,7 @@ out_release:
/* no need to cleanup/release anything in tmp_data */
if (tmp_lower_paths)
for (i = 0; i < new_branches; i++)
-   pathput(_lower_paths[i]);
+   path_put(_lower_paths[i]);
 out_free:
kfree(tmp_lower_paths);
kfree(tmp_data);
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 12/17] Unionfs: branch management/configuration fixes

[Erez Zadok]

Remove unnecessary calls to update branch m/ctimes, and use them only when
needed.  Update branch vfsmounts after operations that could cause a copyup.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/commonfops.c |9 +++--
 fs/unionfs/copyup.c |3 ++-
 fs/unionfs/dentry.c |1 +
 fs/unionfs/mmap.c   |   17 -
 fs/unionfs/rename.c |7 +--
 5 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 491e2ff..2add167 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -604,6 +604,7 @@ out:
}
 out_nofree:
if (!err) {
+   unionfs_postcopyup_setmnt(dentry);
unionfs_copy_attr_times(inode);
unionfs_check_file(file);
unionfs_check_inode(inode);
@@ -839,13 +840,9 @@ int unionfs_flush(struct file *file, fl_owner_t id)
 
}
 
-   /* on success, update our times */
-   unionfs_copy_attr_times(dentry->d_inode);
-   /* parent time could have changed too (async) */
-   unionfs_copy_attr_times(dentry->d_parent->d_inode);
-
 out:
-   unionfs_check_file(file);
+   if (!err)
+   unionfs_check_file(file);
unionfs_unlock_dentry(file->f_path.dentry);
unionfs_read_unlock(dentry->d_sb);
return err;
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 9beac01..f71bddf 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -819,7 +819,8 @@ begin:
 * update times of this dentry, but also the parent, because if
 * we changed, the parent may have changed too.
 */
-   unionfs_copy_attr_times(parent_dentry->d_inode);
+   fsstack_copy_attr_times(parent_dentry->d_inode,
+   lower_parent_dentry->d_inode);
unionfs_copy_attr_times(child_dentry->d_inode);
 
parent_dentry = child_dentry;
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 17b297d..a956b94 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -482,6 +482,7 @@ static int unionfs_d_revalidate(struct dentry *dentry, 
struct nameidata *nd)
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
err = __unionfs_d_revalidate_chain(dentry, nd, false);
if (likely(err > 0)) { /* true==1: dentry is valid */
+   unionfs_postcopyup_setmnt(dentry);
unionfs_check_dentry(dentry);
unionfs_check_nd(nd);
}
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index ad770ac..d6ac61e 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -227,20 +227,11 @@ static int unionfs_prepare_write(struct file *file, 
struct page *page,
int err;
 
unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
-   /*
-* This is the only place where we unconditionally copy the lower
-* attribute times before calling unionfs_file_revalidate.  The
-* reason is that our ->write calls do_sync_write which in turn will
-* call our ->prepare_write and then ->commit_write.  Before our
-* ->write is called, the lower mtimes are in sync, but by the time
-* the VFS calls our ->commit_write, the lower mtimes have changed.
-* Therefore, the only reasonable time for us to sync up from the
-* changed lower mtimes, and avoid an invariant violation warning,
-* is here, in ->prepare_write.
-*/
-   unionfs_copy_attr_times(file->f_path.dentry->d_inode);
err = unionfs_file_revalidate(file, true);
-   unionfs_check_file(file);
+   if (!err) {
+   unionfs_copy_attr_times(file->f_path.dentry->d_inode);
+   unionfs_check_file(file);
+   }
unionfs_read_unlock(file->f_path.dentry->d_sb);
 
return err;
diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index 5ab13f9..cc16eb2 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -138,6 +138,11 @@ static int __unionfs_rename(struct inode *old_dir, struct 
dentry *old_dentry,
err = vfs_rename(lower_old_dir_dentry->d_inode, lower_old_dentry,
 lower_new_dir_dentry->d_inode, lower_new_dentry);
 out_err_unlock:
+   if (!err) {
+   /* update parent dir times */
+   fsstack_copy_attr_times(old_dir, lower_old_dir_dentry->d_inode);
+   fsstack_copy_attr_times(new_dir, lower_new_dir_dentry->d_inode);
+   }
unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
lockdep_on();
 
@@ -526,8 +531,6 @@ int unionfs_rename(struct inode *old_dir, struct dentry 
*old_dentry,
}
}
/* if all of this renaming succeeded, update our times */
-   unionfs_copy_attr_times(old_dir);
-   unionfs_copy_attr_times(new_dir);
unionfs

[PATCH 04/17] Unionfs: uninline unionfs_copy_attr_times and unionfs_copy_attr_all

[Erez Zadok]

This reduces text size by about 6k.

Cc: Hugh Dickins <[EMAIL PROTECTED]>

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/fanout.h |   50 --
 fs/unionfs/subr.c   |   50 ++
 fs/unionfs/union.h  |2 ++
 3 files changed, 52 insertions(+), 50 deletions(-)

diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index 4d9a45f..29d42fb 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -313,54 +313,4 @@ static inline void verify_locked(struct dentry *d)
BUG_ON(!mutex_is_locked(_D(d)->lock));
 }
 
-/* copy a/m/ctime from the lower branch with the newest times */
-static inline void unionfs_copy_attr_times(struct inode *upper)
-{
-   int bindex;
-   struct inode *lower;
-
-   if (!upper || ibstart(upper) < 0)
-   return;
-   for (bindex = ibstart(upper); bindex <= ibend(upper); bindex++) {
-   lower = unionfs_lower_inode_idx(upper, bindex);
-   if (!lower)
-   continue; /* not all lower dir objects may exist */
-   if (unlikely(timespec_compare(>i_mtime,
- >i_mtime) < 0))
-   upper->i_mtime = lower->i_mtime;
-   if (unlikely(timespec_compare(>i_ctime,
- >i_ctime) < 0))
-   upper->i_ctime = lower->i_ctime;
-   if (unlikely(timespec_compare(>i_atime,
- >i_atime) < 0))
-   upper->i_atime = lower->i_atime;
-   }
-}
-
-/*
- * A unionfs/fanout version of fsstack_copy_attr_all.  Uses a
- * unionfs_get_nlinks to properly calcluate the number of links to a file.
- * Also, copies the max() of all a/m/ctimes for all lower inodes (which is
- * important if the lower inode is a directory type)
- */
-static inline void unionfs_copy_attr_all(struct inode *dest,
-const struct inode *src)
-{
-   dest->i_mode = src->i_mode;
-   dest->i_uid = src->i_uid;
-   dest->i_gid = src->i_gid;
-   dest->i_rdev = src->i_rdev;
-
-   unionfs_copy_attr_times(dest);
-
-   dest->i_blkbits = src->i_blkbits;
-   dest->i_flags = src->i_flags;
-
-   /*
-* Update the nlinks AFTER updating the above fields, because the
-* get_links callback may depend on them.
-*/
-   dest->i_nlink = unionfs_get_nlinks(dest);
-}
-
 #endif /* not _FANOUT_H */
diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
index 0a0fce9..68a6280 100644
--- a/fs/unionfs/subr.c
+++ b/fs/unionfs/subr.c
@@ -240,3 +240,53 @@ char *alloc_whname(const char *name, int len)
 
return buf;
 }
+
+/* copy a/m/ctime from the lower branch with the newest times */
+void unionfs_copy_attr_times(struct inode *upper)
+{
+   int bindex;
+   struct inode *lower;
+
+   if (!upper || ibstart(upper) < 0)
+   return;
+   for (bindex = ibstart(upper); bindex <= ibend(upper); bindex++) {
+   lower = unionfs_lower_inode_idx(upper, bindex);
+   if (!lower)
+   continue; /* not all lower dir objects may exist */
+   if (unlikely(timespec_compare(>i_mtime,
+ >i_mtime) < 0))
+   upper->i_mtime = lower->i_mtime;
+   if (unlikely(timespec_compare(>i_ctime,
+ >i_ctime) < 0))
+   upper->i_ctime = lower->i_ctime;
+   if (unlikely(timespec_compare(>i_atime,
+ >i_atime) < 0))
+   upper->i_atime = lower->i_atime;
+   }
+}
+
+/*
+ * A unionfs/fanout version of fsstack_copy_attr_all.  Uses a
+ * unionfs_get_nlinks to properly calcluate the number of links to a file.
+ * Also, copies the max() of all a/m/ctimes for all lower inodes (which is
+ * important if the lower inode is a directory type)
+ */
+void unionfs_copy_attr_all(struct inode *dest,
+  const struct inode *src)
+{
+   dest->i_mode = src->i_mode;
+   dest->i_uid = src->i_uid;
+   dest->i_gid = src->i_gid;
+   dest->i_rdev = src->i_rdev;
+
+   unionfs_copy_attr_times(dest);
+
+   dest->i_blkbits = src->i_blkbits;
+   dest->i_flags = src->i_flags;
+
+   /*
+* Update the nlinks AFTER updating the above fields, because the
+* get_links callback may depend on them.
+*/
+   dest->i_nlink = unionfs_get_nlinks(dest);
+}
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 786c4be..0e0ccde 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -199,6 +199

[PATCH 17/17] VFS/Unionfs: use generic path_get/path_put functions

[Erez Zadok]

Remove unionfs's versions thereof.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/main.c |   10 +-
 fs/unionfs/super.c|   27 ++-
 include/linux/namei.h |   12 
 3 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 3585b29..8f59fb5 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -230,11 +230,11 @@ void unionfs_reinterpose(struct dentry *dentry)
 int check_branch(struct nameidata *nd)
 {
/* XXX: remove in ODF code -- stacking unions allowed there */
-   if (!strcmp(nd->dentry->d_sb->s_type->name, UNIONFS_NAME))
+   if (!strcmp(nd->path.dentry->d_sb->s_type->name, UNIONFS_NAME))
return -EINVAL;
-   if (!nd->dentry->d_inode)
+   if (!nd->path.dentry->d_inode)
return -ENOENT;
-   if (!S_ISDIR(nd->dentry->d_inode->i_mode))
+   if (!S_ISDIR(nd->path.dentry->d_inode->i_mode))
return -ENOTDIR;
return 0;
 }
@@ -375,8 +375,8 @@ static int parse_dirs_option(struct super_block *sb, struct 
unionfs_dentry_info
goto out;
}
 
-   lower_root_info->lower_paths[bindex].dentry = nd.dentry;
-   lower_root_info->lower_paths[bindex].mnt = nd.mnt;
+   lower_root_info->lower_paths[bindex].dentry = nd.path.dentry;
+   lower_root_info->lower_paths[bindex].mnt = nd.path.mnt;
 
set_branchperms(sb, bindex, perms);
set_branch_count(sb, bindex, 0);
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 773623e..fba1598 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -231,8 +231,8 @@ static noinline int do_remount_mode_option(char *optarg, 
int cur_branches,
goto out;
}
for (idx = 0; idx < cur_branches; idx++)
-   if (nd.mnt == new_lower_paths[idx].mnt &&
-   nd.dentry == new_lower_paths[idx].dentry)
+   if (nd.path.mnt == new_lower_paths[idx].mnt &&
+   nd.path.dentry == new_lower_paths[idx].dentry)
break;
path_put(); /* no longer needed */
if (idx == cur_branches) {
@@ -274,8 +274,8 @@ static noinline int do_remount_del_option(char *optarg, int 
cur_branches,
goto out;
}
for (idx = 0; idx < cur_branches; idx++)
-   if (nd.mnt == new_lower_paths[idx].mnt &&
-   nd.dentry == new_lower_paths[idx].dentry)
+   if (nd.path.mnt == new_lower_paths[idx].mnt &&
+   nd.path.dentry == new_lower_paths[idx].dentry)
break;
path_put(); /* no longer needed */
if (idx == cur_branches) {
@@ -358,8 +358,8 @@ static noinline int do_remount_add_option(char *optarg, int 
cur_branches,
goto out;
}
for (idx = 0; idx < cur_branches; idx++)
-   if (nd.mnt == new_lower_paths[idx].mnt &&
-   nd.dentry == new_lower_paths[idx].dentry)
+   if (nd.path.mnt == new_lower_paths[idx].mnt &&
+   nd.path.dentry == new_lower_paths[idx].dentry)
break;
path_put(); /* no longer needed */
if (idx == cur_branches) {
@@ -425,10 +425,10 @@ found_insertion_point:
memmove(_lower_paths[idx+1], _lower_paths[idx],
(cur_branches - idx) * sizeof(struct path));
}
-   new_lower_paths[idx].dentry = nd.dentry;
-   new_lower_paths[idx].mnt = nd.mnt;
+   new_lower_paths[idx].dentry = nd.path.dentry;
+   new_lower_paths[idx].mnt = nd.path.mnt;
 
-   new_data[idx].sb = nd.dentry->d_sb;
+   new_data[idx].sb = nd.path.dentry->d_sb;
atomic_set(_data[idx].open_files, 0);
new_data[idx].branchperms = perms;
new_data[idx].branch_id = ++*high_branch_id; /* assign new branch ID */
@@ -577,7 +577,7 @@ static int unionfs_remount_fs(struct super_block *sb, int 
*flags,
memcpy(tmp_lower_paths, UNIONFS_D(sb->s_root)->lower_paths,
   cur_branches * sizeof(struct path));
for (i = 0; i < cur_branches; i++)
-   pathget(_lower_paths[i]); /* drop refs at end of fxn */
+   path_get(_lower_paths[i]); /* drop refs at end of fxn */
 
/***
 * For each branch command, do path_lookup on the requested branch,
@@ -1008,9 +1008,10 @@ static int unionfs_show_options(struct seq_file *m, 
struct vfsmount *mnt)
 
seq_printf(m, ",dirs=");
for (bindex = bstart; bindex <= bend; bindex++) {
-   path = d_path(unionfs_lower_dentry_idx(sb->s_root, bindex),
-   

[PATCH 09/17] Unionfs: revalidation code cleanup and refactoring

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/dentry.c |   55 --
 1 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index cd15243..afa2120 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -18,6 +18,39 @@
 
 #include "union.h"
 
+
+static inline void __dput_lowers(struct dentry *dentry, int start, int end)
+{
+   struct dentry *lower_dentry;
+   int bindex;
+
+   if (start < 0)
+   return;
+   for (bindex = start; bindex <= end; bindex++) {
+   lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+   if (!lower_dentry)
+   continue;
+   unionfs_set_lower_dentry_idx(dentry, bindex, NULL);
+   dput(lower_dentry);
+   }
+}
+
+static inline void __iput_lowers(struct inode *inode, int start, int end)
+{
+   struct inode *lower_inode;
+   int bindex;
+
+   if (start < 0)
+   return;
+   for (bindex = start; bindex <= end; bindex++) {
+   lower_inode = unionfs_lower_inode_idx(inode, bindex);
+   if (!lower_inode)
+   continue;
+   unionfs_set_lower_inode_idx(inode, bindex, NULL);
+   iput(lower_inode);
+   }
+}
+
 /*
  * Revalidate a single dentry.
  * Assume that dentry's info node is locked.
@@ -72,15 +105,7 @@ static bool __unionfs_d_revalidate_one(struct dentry 
*dentry,
/* Free the pointers for our inodes and this dentry. */
bstart = dbstart(dentry);
bend = dbend(dentry);
-   if (bstart >= 0) {
-   struct dentry *lower_dentry;
-   for (bindex = bstart; bindex <= bend; bindex++) {
-   lower_dentry =
-   unionfs_lower_dentry_idx(dentry,
-bindex);
-   dput(lower_dentry);
-   }
-   }
+   __dput_lowers(dentry, bstart, bend);
set_dbstart(dentry, -1);
set_dbend(dentry, -1);
 
@@ -90,17 +115,7 @@ static bool __unionfs_d_revalidate_one(struct dentry 
*dentry,
 
bstart = ibstart(dentry->d_inode);
bend = ibend(dentry->d_inode);
-   if (bstart >= 0) {
-   struct inode *lower_inode;
-   for (bindex = bstart; bindex <= bend;
-bindex++) {
-   lower_inode =
-   unionfs_lower_inode_idx(
-   dentry->d_inode,
-   bindex);
-   iput(lower_inode);
-   }
-   }
+   __iput_lowers(dentry->d_inode, bstart, bend);
kfree(UNIONFS_I(dentry->d_inode)->lower_inodes);
UNIONFS_I(dentry->d_inode)->lower_inodes = NULL;
ibstart(dentry->d_inode) = -1;
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 02/17] Unionfs: ensure consistent lower inodes types

[Erez Zadok]

When looking up a lower object in multiple branches, especially for
directories, ignore any existing entries whose type is different than the
type of the first found object (otherwise we'll be trying to, say, call
readdir on a non-dir inode).

Signed-off-by: Himanshu Kanda <[EMAIL PROTECTED]>
Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/lookup.c |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index b9ee072..755158e 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -256,6 +256,19 @@ struct dentry *unionfs_lookup_backend(struct dentry 
*dentry,
continue;
}
 
+   /*
+* If we already found at least one positive dentry
+* (dentry_count is non-zero), then we skip all remaining
+* positive dentries if their type is a non-dir.  This is
+* because only directories are allowed to stack on multiple
+* branches, but we have to skip non-dirs (to avoid, say,
+* calling readdir on a regular file).
+*/
+   if (!S_ISDIR(lower_dentry->d_inode->i_mode) && dentry_count) {
+   dput(lower_dentry);
+   continue;
+   }
+
/* number of positive dentries */
dentry_count++;
 
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 07/17] Unionfs: follow_link locking fixes

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/inode.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 8d939dc..6377533 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -820,7 +820,11 @@ static void *unionfs_follow_link(struct dentry *dentry, 
struct nameidata *nd)
err = 0;
 
 out:
-   unionfs_check_dentry(dentry);
+   if (!err) {
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+   unionfs_check_dentry(dentry);
+   unionfs_unlock_dentry(dentry);
+   }
unionfs_check_nd(nd);
return ERR_PTR(err);
 }
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 03/17] Unionfs: document behavior when the lower topology changes

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 Documentation/filesystems/unionfs/concepts.txt |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/Documentation/filesystems/unionfs/concepts.txt 
b/Documentation/filesystems/unionfs/concepts.txt
index bed69bd..8d9a1c5 100644
--- a/Documentation/filesystems/unionfs/concepts.txt
+++ b/Documentation/filesystems/unionfs/concepts.txt
@@ -210,4 +210,17 @@ there's a lot of concurrent activity on both the upper and 
lower objects,
 for the same file(s).  Lastly, this delayed time attribute detection is
 similar to how NFS clients operate (e.g., acregmin).
 
+Finally, there is no way currently in Linux to prevent lower directories
+from being moved around (i.e., topology changes); there's no way to prevent
+modifications to directory sub-trees of whole file systems which are mounted
+read-write.  It is therefore possible for in-flight operations in unionfs to
+take place, while a lower directory is being moved around.  Therefore, if
+you try to, say, create a new file in a directory through unionfs, while the
+directory is being moved around directly, then the new file may get created
+in the new location where that directory was moved to.  This is a somewhat
+similar behaviour in NFS: an NFS client could be creating a new file while
+th NFS server is moving th directory around; the file will get successfully
+created in the new location.  (The one exception in unionfs is that if the
+branch is marked read-only by unionfs, then a copyup will take place.)
+
 For more information, see <http://unionfs.filesystems.org/>.
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 01/17] Unionfs: grab lower super_block references

[Erez Zadok]

This prevents the lower super_block from being destroyed too early, when a
lower file system is being unmounted with MNT_FORCE or MNT_DETACH.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/main.c  |3 +++
 fs/unionfs/super.c |   14 ++
 fs/unionfs/union.h |2 +-
 3 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 23c18f7..ba3471d 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -636,6 +636,7 @@ static int unionfs_read_super(struct super_block *sb, void 
*raw_data,
sbend(sb) = bend = lower_root_info->bend;
for (bindex = bstart; bindex <= bend; bindex++) {
struct dentry *d = lower_root_info->lower_paths[bindex].dentry;
+   atomic_inc(>d_sb->s_active);
unionfs_set_lower_super_idx(sb, bindex, d->d_sb);
}
 
@@ -711,6 +712,8 @@ out_dput:
dput(d);
/* initializing: can't use unionfs_mntput here */
mntput(m);
+   /* drop refs we took earlier */
+   atomic_dec(>d_sb->s_active);
}
kfree(lower_root_info->lower_paths);
kfree(lower_root_info);
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 986c980..175840f 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -116,6 +116,14 @@ static void unionfs_put_super(struct super_block *sb)
}
BUG_ON(leaks != 0);
 
+   /* decrement lower super references */
+   for (bindex = bstart; bindex <= bend; bindex++) {
+   struct super_block *s;
+   s = unionfs_lower_super_idx(sb, bindex);
+   unionfs_set_lower_super_idx(sb, bindex, NULL);
+   atomic_dec(>s_active);
+   }
+
kfree(spd->data);
kfree(spd);
sb->s_fs_info = NULL;
@@ -729,6 +737,12 @@ out_no_change:
 */
purge_sb_data(sb);
 
+   /* grab new lower super references; release old ones */
+   for (i = 0; i < new_branches; i++)
+   atomic_inc(_data[i].sb->s_active);
+   for (i = 0; i < new_branches; i++)
+   atomic_dec(_SB(sb)->data[i].sb->s_active);
+
/* copy new vectors into their correct place */
tmp_data = UNIONFS_SB(sb)->data;
UNIONFS_SB(sb)->data = new_data;
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 4b4d6c9..786c4be 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -127,7 +127,7 @@ struct unionfs_dentry_info {
 
 /* These are the pointers to our various objects. */
 struct unionfs_data {
-   struct super_block *sb;
+   struct super_block *sb; /* lower super_block */
atomic_t open_files;/* number of open files on branch */
int branchperms;
int branch_id;  /* unique branch ID at re/mount time */
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL -mm] 00/17 Unionfs updates/fixes/cleanups

[Erez Zadok]

The following is a series of patchsets related to Unionfs.  The most
significant changes are several fixes to races/locking.  This release also
supports newer APIs in 2.6.25-rc: not using iget/read_inode, using the
changed d_path, using the revised nameidata which embeds a struct path, and
using path_get/put.

These patches were tested (where appropriate) on 2.6.25, MM, as well as the
backports to 2.6.{24,23,22,21,20,19,18,9} on ext2/3/4, xfs, reiserfs,
nfs2/3/4, jffs2, ramfs, tmpfs, cramfs, and squashfs (where available).  Also
tested with LTP-full and with a continuous parallel kernel compile (while
forcing cache flushing, manipulating lower branches, etc.).  See
http://unionfs.filesystems.org/ to download back-ported unionfs code.

Please pull from the 'master' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/ezk/unionfs.git

to receive the following:

Andrew Morton (1):
  Unionfs: embed a struct path into struct nameidata instead of nd dentrymnt

David Howells (1):
  Unionfs: stop using iget() and read_inode()

Erez Zadok (14):
  Unionfs: grab lower super_block references
  Unionfs: ensure consistent lower inodes types
  Unionfs: document behavior when the lower topology changes
  Unionfs: uninline unionfs_copy_attr_times and unionfs_copy_attr_all
  Unionfs: initialize path_save variable
  Unionfs: extend dentry branch configuration lock in open
  Unionfs: follow_link locking fixes
  Unionfs: improve debugging in copy_attr_times
  Unionfs: revalidation code cleanup and refactoring
  Unionfs: factor out revalidation routine
  Unionfs: lock parents' branch configuration fixes
  Unionfs: branch management/configuration fixes
  Unionfs: use dget_parent in revalidation code
  VFS/Unionfs: use generic path_get/path_put functions

Jan Blunck (1):
  Unionfs: use the new path_put

 Documentation/filesystems/unionfs/concepts.txt |   13 +
 fs/unionfs/commonfops.c|   54 ---
 fs/unionfs/copyup.c|3 
 fs/unionfs/dentry.c|  182 ++---
 fs/unionfs/fanout.h|   50 --
 fs/unionfs/inode.c |   78 +++---
 fs/unionfs/lookup.c|   13 +
 fs/unionfs/main.c  |   26 +--
 fs/unionfs/mmap.c  |   17 --
 fs/unionfs/rename.c|7 
 fs/unionfs/subr.c  |   56 +++
 fs/unionfs/super.c |   72 ++---
 fs/unionfs/union.h |   13 +
 fs/unionfs/unlink.c|   11 +
 include/linux/namei.h  |   12 -
 15 files changed, 366 insertions(+), 241 deletions(-)

Thanks.
---
Erez Zadok
[EMAIL PROTECTED]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH -mmotm] fs/sysfs/file.c d_path fix

[Erez Zadok]

Using mmotm-2008-02-15-11-03, I get

  CC  fs/sysfs/file.o
fs/sysfs/file.c: In function 'sysfs_open_file':
fs/sysfs/file.c:334: warning: passing argument 1 of 'd_path' from incompatible 
pointer type
fs/sysfs/file.c:334: warning: passing argument 2 of 'd_path' from incompatible 
pointer type
fs/sysfs/file.c:334: warning: passing argument 3 of 'd_path' makes integer from 
pointer without a cast
fs/sysfs/file.c:334: error: too many arguments to function 'd_path'
make[2]: *** [fs/sysfs/file.o] Error 1
make[1]: *** [fs/sysfs] Error 2
make: *** [fs] Error 2

The following small patch fixes it for me.

Cheers,
Erez.


Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 02223e2..a57b024 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -329,9 +329,11 @@ static int sysfs_open_file(struct inode *inode, struct 
file *file)
struct sysfs_ops *ops;
int error = -EACCES;
char *p;
+   struct path sysfs_path;
 
-   p = d_path(file->f_dentry, sysfs_mount, last_sysfs_file,
-  sizeof(last_sysfs_file));
+   sysfs_path.dentry = file->f_dentry;
+   sysfs_path.mnt = sysfs_mount;
+   p = d_path(_path, last_sysfs_file, sizeof(last_sysfs_file));
if (p)
memmove(last_sysfs_file, p, strlen(p) + 1);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH -mmotm] one more reject fix in arch/x86/Kconfig

[Erez Zadok]

Using mmotm-2008-02-15-11-03, I get

$ make ARCH=i386
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/kxgettext.o
  HOSTCC  scripts/kconfig/zconf.tab.o
  HOSTLD  scripts/kconfig/conf
scripts/kconfig/conf -s arch/x86/Kconfig
arch/x86/Kconfig:24: unknown option "HEAD"
arch/x86/Kconfig:30: unknown option "FETCH_HEAD"
make[2]: *** [silentoldconfig] Error 1
make[1]: *** [silentoldconfig] Error 2
make: *** No rule to make target `include/config/auto.conf', needed by 
`include/config/kernel.release'.  Stop.

The following small patch fixes it for me, but may not be the right fix (I
chose to resolve the conflict by including both parts).

Cheers,
Erez.


diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6d6150b..dd4d64d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -21,13 +21,9 @@ config X86
select HAVE_IDE
select HAVE_OPROFILE
select HAVE_KPROBES
-<<< HEAD:arch/x86/Kconfig
select HAVE_KVM
select HAVE_ARCH_KGDB
-
-===
select HAVE_FTRACE
->>> FETCH_HEAD:arch/x86/Kconfig
 
 config GENERIC_LOCKBREAK
def_bool n
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH -mmotm] one more reject fix in arch/x86/Kconfig

[Erez Zadok]

Using mmotm-2008-02-15-11-03, I get

$ make ARCH=i386
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/kxgettext.o
  HOSTCC  scripts/kconfig/zconf.tab.o
  HOSTLD  scripts/kconfig/conf
scripts/kconfig/conf -s arch/x86/Kconfig
arch/x86/Kconfig:24: unknown option HEAD
arch/x86/Kconfig:30: unknown option FETCH_HEAD
make[2]: *** [silentoldconfig] Error 1
make[1]: *** [silentoldconfig] Error 2
make: *** No rule to make target `include/config/auto.conf', needed by 
`include/config/kernel.release'.  Stop.

The following small patch fixes it for me, but may not be the right fix (I
chose to resolve the conflict by including both parts).

Cheers,
Erez.


diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6d6150b..dd4d64d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -21,13 +21,9 @@ config X86
select HAVE_IDE
select HAVE_OPROFILE
select HAVE_KPROBES
- HEAD:arch/x86/Kconfig
select HAVE_KVM
select HAVE_ARCH_KGDB
-
-===
select HAVE_FTRACE
- FETCH_HEAD:arch/x86/Kconfig
 
 config GENERIC_LOCKBREAK
def_bool n
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH -mmotm] fs/sysfs/file.c d_path fix

[Erez Zadok]

Using mmotm-2008-02-15-11-03, I get

  CC  fs/sysfs/file.o
fs/sysfs/file.c: In function 'sysfs_open_file':
fs/sysfs/file.c:334: warning: passing argument 1 of 'd_path' from incompatible 
pointer type
fs/sysfs/file.c:334: warning: passing argument 2 of 'd_path' from incompatible 
pointer type
fs/sysfs/file.c:334: warning: passing argument 3 of 'd_path' makes integer from 
pointer without a cast
fs/sysfs/file.c:334: error: too many arguments to function 'd_path'
make[2]: *** [fs/sysfs/file.o] Error 1
make[1]: *** [fs/sysfs] Error 2
make: *** [fs] Error 2

The following small patch fixes it for me.

Cheers,
Erez.


Signed-off-by: Erez Zadok [EMAIL PROTECTED]

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 02223e2..a57b024 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -329,9 +329,11 @@ static int sysfs_open_file(struct inode *inode, struct 
file *file)
struct sysfs_ops *ops;
int error = -EACCES;
char *p;
+   struct path sysfs_path;
 
-   p = d_path(file-f_dentry, sysfs_mount, last_sysfs_file,
-  sizeof(last_sysfs_file));
+   sysfs_path.dentry = file-f_dentry;
+   sysfs_path.mnt = sysfs_mount;
+   p = d_path(sysfs_path, last_sysfs_file, sizeof(last_sysfs_file));
if (p)
memmove(last_sysfs_file, p, strlen(p) + 1);
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL -mm] 00/17 Unionfs updates/fixes/cleanups

[Erez Zadok]

The following is a series of patchsets related to Unionfs.  The most
significant changes are several fixes to races/locking.  This release also
supports newer APIs in 2.6.25-rc: not using iget/read_inode, using the
changed d_path, using the revised nameidata which embeds a struct path, and
using path_get/put.

These patches were tested (where appropriate) on 2.6.25, MM, as well as the
backports to 2.6.{24,23,22,21,20,19,18,9} on ext2/3/4, xfs, reiserfs,
nfs2/3/4, jffs2, ramfs, tmpfs, cramfs, and squashfs (where available).  Also
tested with LTP-full and with a continuous parallel kernel compile (while
forcing cache flushing, manipulating lower branches, etc.).  See
http://unionfs.filesystems.org/ to download back-ported unionfs code.

Please pull from the 'master' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/ezk/unionfs.git

to receive the following:

Andrew Morton (1):
  Unionfs: embed a struct path into struct nameidata instead of nd dentrymnt

David Howells (1):
  Unionfs: stop using iget() and read_inode()

Erez Zadok (14):
  Unionfs: grab lower super_block references
  Unionfs: ensure consistent lower inodes types
  Unionfs: document behavior when the lower topology changes
  Unionfs: uninline unionfs_copy_attr_times and unionfs_copy_attr_all
  Unionfs: initialize path_save variable
  Unionfs: extend dentry branch configuration lock in open
  Unionfs: follow_link locking fixes
  Unionfs: improve debugging in copy_attr_times
  Unionfs: revalidation code cleanup and refactoring
  Unionfs: factor out revalidation routine
  Unionfs: lock parents' branch configuration fixes
  Unionfs: branch management/configuration fixes
  Unionfs: use dget_parent in revalidation code
  VFS/Unionfs: use generic path_get/path_put functions

Jan Blunck (1):
  Unionfs: use the new path_put

 Documentation/filesystems/unionfs/concepts.txt |   13 +
 fs/unionfs/commonfops.c|   54 ---
 fs/unionfs/copyup.c|3 
 fs/unionfs/dentry.c|  182 ++---
 fs/unionfs/fanout.h|   50 --
 fs/unionfs/inode.c |   78 +++---
 fs/unionfs/lookup.c|   13 +
 fs/unionfs/main.c  |   26 +--
 fs/unionfs/mmap.c  |   17 --
 fs/unionfs/rename.c|7 
 fs/unionfs/subr.c  |   56 +++
 fs/unionfs/super.c |   72 ++---
 fs/unionfs/union.h |   13 +
 fs/unionfs/unlink.c|   11 +
 include/linux/namei.h  |   12 -
 15 files changed, 366 insertions(+), 241 deletions(-)

Thanks.
---
Erez Zadok
[EMAIL PROTECTED]
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 01/17] Unionfs: grab lower super_block references

[Erez Zadok]

This prevents the lower super_block from being destroyed too early, when a
lower file system is being unmounted with MNT_FORCE or MNT_DETACH.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/main.c  |3 +++
 fs/unionfs/super.c |   14 ++
 fs/unionfs/union.h |2 +-
 3 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 23c18f7..ba3471d 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -636,6 +636,7 @@ static int unionfs_read_super(struct super_block *sb, void 
*raw_data,
sbend(sb) = bend = lower_root_info-bend;
for (bindex = bstart; bindex = bend; bindex++) {
struct dentry *d = lower_root_info-lower_paths[bindex].dentry;
+   atomic_inc(d-d_sb-s_active);
unionfs_set_lower_super_idx(sb, bindex, d-d_sb);
}
 
@@ -711,6 +712,8 @@ out_dput:
dput(d);
/* initializing: can't use unionfs_mntput here */
mntput(m);
+   /* drop refs we took earlier */
+   atomic_dec(d-d_sb-s_active);
}
kfree(lower_root_info-lower_paths);
kfree(lower_root_info);
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 986c980..175840f 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -116,6 +116,14 @@ static void unionfs_put_super(struct super_block *sb)
}
BUG_ON(leaks != 0);
 
+   /* decrement lower super references */
+   for (bindex = bstart; bindex = bend; bindex++) {
+   struct super_block *s;
+   s = unionfs_lower_super_idx(sb, bindex);
+   unionfs_set_lower_super_idx(sb, bindex, NULL);
+   atomic_dec(s-s_active);
+   }
+
kfree(spd-data);
kfree(spd);
sb-s_fs_info = NULL;
@@ -729,6 +737,12 @@ out_no_change:
 */
purge_sb_data(sb);
 
+   /* grab new lower super references; release old ones */
+   for (i = 0; i  new_branches; i++)
+   atomic_inc(new_data[i].sb-s_active);
+   for (i = 0; i  new_branches; i++)
+   atomic_dec(UNIONFS_SB(sb)-data[i].sb-s_active);
+
/* copy new vectors into their correct place */
tmp_data = UNIONFS_SB(sb)-data;
UNIONFS_SB(sb)-data = new_data;
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 4b4d6c9..786c4be 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -127,7 +127,7 @@ struct unionfs_dentry_info {
 
 /* These are the pointers to our various objects. */
 struct unionfs_data {
-   struct super_block *sb;
+   struct super_block *sb; /* lower super_block */
atomic_t open_files;/* number of open files on branch */
int branchperms;
int branch_id;  /* unique branch ID at re/mount time */
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 03/17] Unionfs: document behavior when the lower topology changes

[Erez Zadok]

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 Documentation/filesystems/unionfs/concepts.txt |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/Documentation/filesystems/unionfs/concepts.txt 
b/Documentation/filesystems/unionfs/concepts.txt
index bed69bd..8d9a1c5 100644
--- a/Documentation/filesystems/unionfs/concepts.txt
+++ b/Documentation/filesystems/unionfs/concepts.txt
@@ -210,4 +210,17 @@ there's a lot of concurrent activity on both the upper and 
lower objects,
 for the same file(s).  Lastly, this delayed time attribute detection is
 similar to how NFS clients operate (e.g., acregmin).
 
+Finally, there is no way currently in Linux to prevent lower directories
+from being moved around (i.e., topology changes); there's no way to prevent
+modifications to directory sub-trees of whole file systems which are mounted
+read-write.  It is therefore possible for in-flight operations in unionfs to
+take place, while a lower directory is being moved around.  Therefore, if
+you try to, say, create a new file in a directory through unionfs, while the
+directory is being moved around directly, then the new file may get created
+in the new location where that directory was moved to.  This is a somewhat
+similar behaviour in NFS: an NFS client could be creating a new file while
+th NFS server is moving th directory around; the file will get successfully
+created in the new location.  (The one exception in unionfs is that if the
+branch is marked read-only by unionfs, then a copyup will take place.)
+
 For more information, see http://unionfs.filesystems.org/.
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 07/17] Unionfs: follow_link locking fixes

[Erez Zadok]

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/inode.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 8d939dc..6377533 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -820,7 +820,11 @@ static void *unionfs_follow_link(struct dentry *dentry, 
struct nameidata *nd)
err = 0;
 
 out:
-   unionfs_check_dentry(dentry);
+   if (!err) {
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+   unionfs_check_dentry(dentry);
+   unionfs_unlock_dentry(dentry);
+   }
unionfs_check_nd(nd);
return ERR_PTR(err);
 }
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 09/17] Unionfs: revalidation code cleanup and refactoring

[Erez Zadok]

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/dentry.c |   55 --
 1 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index cd15243..afa2120 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -18,6 +18,39 @@
 
 #include union.h
 
+
+static inline void __dput_lowers(struct dentry *dentry, int start, int end)
+{
+   struct dentry *lower_dentry;
+   int bindex;
+
+   if (start  0)
+   return;
+   for (bindex = start; bindex = end; bindex++) {
+   lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+   if (!lower_dentry)
+   continue;
+   unionfs_set_lower_dentry_idx(dentry, bindex, NULL);
+   dput(lower_dentry);
+   }
+}
+
+static inline void __iput_lowers(struct inode *inode, int start, int end)
+{
+   struct inode *lower_inode;
+   int bindex;
+
+   if (start  0)
+   return;
+   for (bindex = start; bindex = end; bindex++) {
+   lower_inode = unionfs_lower_inode_idx(inode, bindex);
+   if (!lower_inode)
+   continue;
+   unionfs_set_lower_inode_idx(inode, bindex, NULL);
+   iput(lower_inode);
+   }
+}
+
 /*
  * Revalidate a single dentry.
  * Assume that dentry's info node is locked.
@@ -72,15 +105,7 @@ static bool __unionfs_d_revalidate_one(struct dentry 
*dentry,
/* Free the pointers for our inodes and this dentry. */
bstart = dbstart(dentry);
bend = dbend(dentry);
-   if (bstart = 0) {
-   struct dentry *lower_dentry;
-   for (bindex = bstart; bindex = bend; bindex++) {
-   lower_dentry =
-   unionfs_lower_dentry_idx(dentry,
-bindex);
-   dput(lower_dentry);
-   }
-   }
+   __dput_lowers(dentry, bstart, bend);
set_dbstart(dentry, -1);
set_dbend(dentry, -1);
 
@@ -90,17 +115,7 @@ static bool __unionfs_d_revalidate_one(struct dentry 
*dentry,
 
bstart = ibstart(dentry-d_inode);
bend = ibend(dentry-d_inode);
-   if (bstart = 0) {
-   struct inode *lower_inode;
-   for (bindex = bstart; bindex = bend;
-bindex++) {
-   lower_inode =
-   unionfs_lower_inode_idx(
-   dentry-d_inode,
-   bindex);
-   iput(lower_inode);
-   }
-   }
+   __iput_lowers(dentry-d_inode, bstart, bend);
kfree(UNIONFS_I(dentry-d_inode)-lower_inodes);
UNIONFS_I(dentry-d_inode)-lower_inodes = NULL;
ibstart(dentry-d_inode) = -1;
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 02/17] Unionfs: ensure consistent lower inodes types

[Erez Zadok]

When looking up a lower object in multiple branches, especially for
directories, ignore any existing entries whose type is different than the
type of the first found object (otherwise we'll be trying to, say, call
readdir on a non-dir inode).

Signed-off-by: Himanshu Kanda [EMAIL PROTECTED]
Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/lookup.c |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
index b9ee072..755158e 100644
--- a/fs/unionfs/lookup.c
+++ b/fs/unionfs/lookup.c
@@ -256,6 +256,19 @@ struct dentry *unionfs_lookup_backend(struct dentry 
*dentry,
continue;
}
 
+   /*
+* If we already found at least one positive dentry
+* (dentry_count is non-zero), then we skip all remaining
+* positive dentries if their type is a non-dir.  This is
+* because only directories are allowed to stack on multiple
+* branches, but we have to skip non-dirs (to avoid, say,
+* calling readdir on a regular file).
+*/
+   if (!S_ISDIR(lower_dentry-d_inode-i_mode)  dentry_count) {
+   dput(lower_dentry);
+   continue;
+   }
+
/* number of positive dentries */
dentry_count++;
 
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 04/17] Unionfs: uninline unionfs_copy_attr_times and unionfs_copy_attr_all

[Erez Zadok]

This reduces text size by about 6k.

Cc: Hugh Dickins [EMAIL PROTECTED]

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/fanout.h |   50 --
 fs/unionfs/subr.c   |   50 ++
 fs/unionfs/union.h  |2 ++
 3 files changed, 52 insertions(+), 50 deletions(-)

diff --git a/fs/unionfs/fanout.h b/fs/unionfs/fanout.h
index 4d9a45f..29d42fb 100644
--- a/fs/unionfs/fanout.h
+++ b/fs/unionfs/fanout.h
@@ -313,54 +313,4 @@ static inline void verify_locked(struct dentry *d)
BUG_ON(!mutex_is_locked(UNIONFS_D(d)-lock));
 }
 
-/* copy a/m/ctime from the lower branch with the newest times */
-static inline void unionfs_copy_attr_times(struct inode *upper)
-{
-   int bindex;
-   struct inode *lower;
-
-   if (!upper || ibstart(upper)  0)
-   return;
-   for (bindex = ibstart(upper); bindex = ibend(upper); bindex++) {
-   lower = unionfs_lower_inode_idx(upper, bindex);
-   if (!lower)
-   continue; /* not all lower dir objects may exist */
-   if (unlikely(timespec_compare(upper-i_mtime,
- lower-i_mtime)  0))
-   upper-i_mtime = lower-i_mtime;
-   if (unlikely(timespec_compare(upper-i_ctime,
- lower-i_ctime)  0))
-   upper-i_ctime = lower-i_ctime;
-   if (unlikely(timespec_compare(upper-i_atime,
- lower-i_atime)  0))
-   upper-i_atime = lower-i_atime;
-   }
-}
-
-/*
- * A unionfs/fanout version of fsstack_copy_attr_all.  Uses a
- * unionfs_get_nlinks to properly calcluate the number of links to a file.
- * Also, copies the max() of all a/m/ctimes for all lower inodes (which is
- * important if the lower inode is a directory type)
- */
-static inline void unionfs_copy_attr_all(struct inode *dest,
-const struct inode *src)
-{
-   dest-i_mode = src-i_mode;
-   dest-i_uid = src-i_uid;
-   dest-i_gid = src-i_gid;
-   dest-i_rdev = src-i_rdev;
-
-   unionfs_copy_attr_times(dest);
-
-   dest-i_blkbits = src-i_blkbits;
-   dest-i_flags = src-i_flags;
-
-   /*
-* Update the nlinks AFTER updating the above fields, because the
-* get_links callback may depend on them.
-*/
-   dest-i_nlink = unionfs_get_nlinks(dest);
-}
-
 #endif /* not _FANOUT_H */
diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
index 0a0fce9..68a6280 100644
--- a/fs/unionfs/subr.c
+++ b/fs/unionfs/subr.c
@@ -240,3 +240,53 @@ char *alloc_whname(const char *name, int len)
 
return buf;
 }
+
+/* copy a/m/ctime from the lower branch with the newest times */
+void unionfs_copy_attr_times(struct inode *upper)
+{
+   int bindex;
+   struct inode *lower;
+
+   if (!upper || ibstart(upper)  0)
+   return;
+   for (bindex = ibstart(upper); bindex = ibend(upper); bindex++) {
+   lower = unionfs_lower_inode_idx(upper, bindex);
+   if (!lower)
+   continue; /* not all lower dir objects may exist */
+   if (unlikely(timespec_compare(upper-i_mtime,
+ lower-i_mtime)  0))
+   upper-i_mtime = lower-i_mtime;
+   if (unlikely(timespec_compare(upper-i_ctime,
+ lower-i_ctime)  0))
+   upper-i_ctime = lower-i_ctime;
+   if (unlikely(timespec_compare(upper-i_atime,
+ lower-i_atime)  0))
+   upper-i_atime = lower-i_atime;
+   }
+}
+
+/*
+ * A unionfs/fanout version of fsstack_copy_attr_all.  Uses a
+ * unionfs_get_nlinks to properly calcluate the number of links to a file.
+ * Also, copies the max() of all a/m/ctimes for all lower inodes (which is
+ * important if the lower inode is a directory type)
+ */
+void unionfs_copy_attr_all(struct inode *dest,
+  const struct inode *src)
+{
+   dest-i_mode = src-i_mode;
+   dest-i_uid = src-i_uid;
+   dest-i_gid = src-i_gid;
+   dest-i_rdev = src-i_rdev;
+
+   unionfs_copy_attr_times(dest);
+
+   dest-i_blkbits = src-i_blkbits;
+   dest-i_flags = src-i_flags;
+
+   /*
+* Update the nlinks AFTER updating the above fields, because the
+* get_links callback may depend on them.
+*/
+   dest-i_nlink = unionfs_get_nlinks(dest);
+}
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 786c4be..0e0ccde 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -199,6 +199,8 @@ struct unionfs_dir_state {
 
 /* externs needed for fanout.h or sioq.h */
 extern int unionfs_get_nlinks(const struct inode *inode);
+extern void unionfs_copy_attr_times(struct

[PATCH 17/17] VFS/Unionfs: use generic path_get/path_put functions

[Erez Zadok]

Remove unionfs's versions thereof.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/main.c |   10 +-
 fs/unionfs/super.c|   27 ++-
 include/linux/namei.h |   12 
 3 files changed, 19 insertions(+), 30 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 3585b29..8f59fb5 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -230,11 +230,11 @@ void unionfs_reinterpose(struct dentry *dentry)
 int check_branch(struct nameidata *nd)
 {
/* XXX: remove in ODF code -- stacking unions allowed there */
-   if (!strcmp(nd-dentry-d_sb-s_type-name, UNIONFS_NAME))
+   if (!strcmp(nd-path.dentry-d_sb-s_type-name, UNIONFS_NAME))
return -EINVAL;
-   if (!nd-dentry-d_inode)
+   if (!nd-path.dentry-d_inode)
return -ENOENT;
-   if (!S_ISDIR(nd-dentry-d_inode-i_mode))
+   if (!S_ISDIR(nd-path.dentry-d_inode-i_mode))
return -ENOTDIR;
return 0;
 }
@@ -375,8 +375,8 @@ static int parse_dirs_option(struct super_block *sb, struct 
unionfs_dentry_info
goto out;
}
 
-   lower_root_info-lower_paths[bindex].dentry = nd.dentry;
-   lower_root_info-lower_paths[bindex].mnt = nd.mnt;
+   lower_root_info-lower_paths[bindex].dentry = nd.path.dentry;
+   lower_root_info-lower_paths[bindex].mnt = nd.path.mnt;
 
set_branchperms(sb, bindex, perms);
set_branch_count(sb, bindex, 0);
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index 773623e..fba1598 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -231,8 +231,8 @@ static noinline int do_remount_mode_option(char *optarg, 
int cur_branches,
goto out;
}
for (idx = 0; idx  cur_branches; idx++)
-   if (nd.mnt == new_lower_paths[idx].mnt 
-   nd.dentry == new_lower_paths[idx].dentry)
+   if (nd.path.mnt == new_lower_paths[idx].mnt 
+   nd.path.dentry == new_lower_paths[idx].dentry)
break;
path_put(nd.path); /* no longer needed */
if (idx == cur_branches) {
@@ -274,8 +274,8 @@ static noinline int do_remount_del_option(char *optarg, int 
cur_branches,
goto out;
}
for (idx = 0; idx  cur_branches; idx++)
-   if (nd.mnt == new_lower_paths[idx].mnt 
-   nd.dentry == new_lower_paths[idx].dentry)
+   if (nd.path.mnt == new_lower_paths[idx].mnt 
+   nd.path.dentry == new_lower_paths[idx].dentry)
break;
path_put(nd.path); /* no longer needed */
if (idx == cur_branches) {
@@ -358,8 +358,8 @@ static noinline int do_remount_add_option(char *optarg, int 
cur_branches,
goto out;
}
for (idx = 0; idx  cur_branches; idx++)
-   if (nd.mnt == new_lower_paths[idx].mnt 
-   nd.dentry == new_lower_paths[idx].dentry)
+   if (nd.path.mnt == new_lower_paths[idx].mnt 
+   nd.path.dentry == new_lower_paths[idx].dentry)
break;
path_put(nd.path); /* no longer needed */
if (idx == cur_branches) {
@@ -425,10 +425,10 @@ found_insertion_point:
memmove(new_lower_paths[idx+1], new_lower_paths[idx],
(cur_branches - idx) * sizeof(struct path));
}
-   new_lower_paths[idx].dentry = nd.dentry;
-   new_lower_paths[idx].mnt = nd.mnt;
+   new_lower_paths[idx].dentry = nd.path.dentry;
+   new_lower_paths[idx].mnt = nd.path.mnt;
 
-   new_data[idx].sb = nd.dentry-d_sb;
+   new_data[idx].sb = nd.path.dentry-d_sb;
atomic_set(new_data[idx].open_files, 0);
new_data[idx].branchperms = perms;
new_data[idx].branch_id = ++*high_branch_id; /* assign new branch ID */
@@ -577,7 +577,7 @@ static int unionfs_remount_fs(struct super_block *sb, int 
*flags,
memcpy(tmp_lower_paths, UNIONFS_D(sb-s_root)-lower_paths,
   cur_branches * sizeof(struct path));
for (i = 0; i  cur_branches; i++)
-   pathget(tmp_lower_paths[i]); /* drop refs at end of fxn */
+   path_get(tmp_lower_paths[i]); /* drop refs at end of fxn */
 
/***
 * For each branch command, do path_lookup on the requested branch,
@@ -1008,9 +1008,10 @@ static int unionfs_show_options(struct seq_file *m, 
struct vfsmount *mnt)
 
seq_printf(m, ,dirs=);
for (bindex = bstart; bindex = bend; bindex++) {
-   path = d_path(unionfs_lower_dentry_idx(sb-s_root, bindex),
- unionfs_lower_mnt_idx(sb-s_root, bindex),
- tmp_page, PAGE_SIZE);
+   struct path p;
+   p.dentry

[PATCH 16/17] Unionfs: use the new path_put

[Erez Zadok]

From: Jan Blunck [EMAIL PROTECTED]

* Add path_put() functions for releasing a reference to the dentry and
  vfsmount of a struct path in the right order

* Switch from path_release(nd) to path_put(nd-path)

* Rename dput_path() to path_put_conditional()

Signed-off-by: Jan Blunck [EMAIL PROTECTED]
Signed-off-by: Andreas Gruenbacher [EMAIL PROTECTED]
Acked-by: Christoph Hellwig [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/main.c  |2 +-
 fs/unionfs/super.c |   12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index 4bc2c66..3585b29 100644
--- a/fs/unionfs/main.c
+++ b/fs/unionfs/main.c
@@ -371,7 +371,7 @@ static int parse_dirs_option(struct super_block *sb, struct 
unionfs_dentry_info
if (err) {
printk(KERN_ERR unionfs: lower directory 
   '%s' is not a valid branch\n, name);
-   path_release(nd);
+   path_put(nd.path);
goto out;
}
 
diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
index b71fc2a..773623e 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -234,7 +234,7 @@ static noinline int do_remount_mode_option(char *optarg, 
int cur_branches,
if (nd.mnt == new_lower_paths[idx].mnt 
nd.dentry == new_lower_paths[idx].dentry)
break;
-   path_release(nd);  /* no longer needed */
+   path_put(nd.path); /* no longer needed */
if (idx == cur_branches) {
err = -ENOENT;  /* err may have been reset above */
printk(KERN_ERR unionfs: branch \%s\ 
@@ -277,7 +277,7 @@ static noinline int do_remount_del_option(char *optarg, int 
cur_branches,
if (nd.mnt == new_lower_paths[idx].mnt 
nd.dentry == new_lower_paths[idx].dentry)
break;
-   path_release(nd);  /* no longer needed */
+   path_put(nd.path); /* no longer needed */
if (idx == cur_branches) {
printk(KERN_ERR unionfs: branch \%s\ 
   not found\n, optarg);
@@ -296,7 +296,7 @@ static noinline int do_remount_del_option(char *optarg, int 
cur_branches,
 * new_data and new_lower_paths one to the left.  Finally, adjust
 * cur_branches.
 */
-   pathput(new_lower_paths[idx]);
+   path_put(new_lower_paths[idx]);
 
if (idx  cur_branches - 1) {
/* if idx==cur_branches-1, we delete last branch: easy */
@@ -361,7 +361,7 @@ static noinline int do_remount_add_option(char *optarg, int 
cur_branches,
if (nd.mnt == new_lower_paths[idx].mnt 
nd.dentry == new_lower_paths[idx].dentry)
break;
-   path_release(nd);  /* no longer needed */
+   path_put(nd.path); /* no longer needed */
if (idx == cur_branches) {
printk(KERN_ERR unionfs: branch \%s\ 
   not found\n, optarg);
@@ -408,7 +408,7 @@ found_insertion_point:
if (err) {
printk(KERN_ERR unionfs: lower directory 
   \%s\ is not a valid branch\n, optarg);
-   path_release(nd);
+   path_put(nd.path);
goto out;
}
 
@@ -818,7 +818,7 @@ out_release:
/* no need to cleanup/release anything in tmp_data */
if (tmp_lower_paths)
for (i = 0; i  new_branches; i++)
-   pathput(tmp_lower_paths[i]);
+   path_put(tmp_lower_paths[i]);
 out_free:
kfree(tmp_lower_paths);
kfree(tmp_data);
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 12/17] Unionfs: branch management/configuration fixes

[Erez Zadok]

Remove unnecessary calls to update branch m/ctimes, and use them only when
needed.  Update branch vfsmounts after operations that could cause a copyup.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/commonfops.c |9 +++--
 fs/unionfs/copyup.c |3 ++-
 fs/unionfs/dentry.c |1 +
 fs/unionfs/mmap.c   |   17 -
 fs/unionfs/rename.c |7 +--
 5 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 491e2ff..2add167 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -604,6 +604,7 @@ out:
}
 out_nofree:
if (!err) {
+   unionfs_postcopyup_setmnt(dentry);
unionfs_copy_attr_times(inode);
unionfs_check_file(file);
unionfs_check_inode(inode);
@@ -839,13 +840,9 @@ int unionfs_flush(struct file *file, fl_owner_t id)
 
}
 
-   /* on success, update our times */
-   unionfs_copy_attr_times(dentry-d_inode);
-   /* parent time could have changed too (async) */
-   unionfs_copy_attr_times(dentry-d_parent-d_inode);
-
 out:
-   unionfs_check_file(file);
+   if (!err)
+   unionfs_check_file(file);
unionfs_unlock_dentry(file-f_path.dentry);
unionfs_read_unlock(dentry-d_sb);
return err;
diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 9beac01..f71bddf 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -819,7 +819,8 @@ begin:
 * update times of this dentry, but also the parent, because if
 * we changed, the parent may have changed too.
 */
-   unionfs_copy_attr_times(parent_dentry-d_inode);
+   fsstack_copy_attr_times(parent_dentry-d_inode,
+   lower_parent_dentry-d_inode);
unionfs_copy_attr_times(child_dentry-d_inode);
 
parent_dentry = child_dentry;
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 17b297d..a956b94 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -482,6 +482,7 @@ static int unionfs_d_revalidate(struct dentry *dentry, 
struct nameidata *nd)
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
err = __unionfs_d_revalidate_chain(dentry, nd, false);
if (likely(err  0)) { /* true==1: dentry is valid */
+   unionfs_postcopyup_setmnt(dentry);
unionfs_check_dentry(dentry);
unionfs_check_nd(nd);
}
diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index ad770ac..d6ac61e 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -227,20 +227,11 @@ static int unionfs_prepare_write(struct file *file, 
struct page *page,
int err;
 
unionfs_read_lock(file-f_path.dentry-d_sb, UNIONFS_SMUTEX_PARENT);
-   /*
-* This is the only place where we unconditionally copy the lower
-* attribute times before calling unionfs_file_revalidate.  The
-* reason is that our -write calls do_sync_write which in turn will
-* call our -prepare_write and then -commit_write.  Before our
-* -write is called, the lower mtimes are in sync, but by the time
-* the VFS calls our -commit_write, the lower mtimes have changed.
-* Therefore, the only reasonable time for us to sync up from the
-* changed lower mtimes, and avoid an invariant violation warning,
-* is here, in -prepare_write.
-*/
-   unionfs_copy_attr_times(file-f_path.dentry-d_inode);
err = unionfs_file_revalidate(file, true);
-   unionfs_check_file(file);
+   if (!err) {
+   unionfs_copy_attr_times(file-f_path.dentry-d_inode);
+   unionfs_check_file(file);
+   }
unionfs_read_unlock(file-f_path.dentry-d_sb);
 
return err;
diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index 5ab13f9..cc16eb2 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -138,6 +138,11 @@ static int __unionfs_rename(struct inode *old_dir, struct 
dentry *old_dentry,
err = vfs_rename(lower_old_dir_dentry-d_inode, lower_old_dentry,
 lower_new_dir_dentry-d_inode, lower_new_dentry);
 out_err_unlock:
+   if (!err) {
+   /* update parent dir times */
+   fsstack_copy_attr_times(old_dir, lower_old_dir_dentry-d_inode);
+   fsstack_copy_attr_times(new_dir, lower_new_dir_dentry-d_inode);
+   }
unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
lockdep_on();
 
@@ -526,8 +531,6 @@ int unionfs_rename(struct inode *old_dir, struct dentry 
*old_dentry,
}
}
/* if all of this renaming succeeded, update our times */
-   unionfs_copy_attr_times(old_dir);
-   unionfs_copy_attr_times(new_dir);
unionfs_copy_attr_times(old_dentry-d_inode);
unionfs_copy_attr_times(new_dentry-d_inode);
unionfs_check_inode(old_dir);
-- 
1.5.2.2

[PATCH 10/17] Unionfs: factor out revalidation routine

[Erez Zadok]

To be used by rest of revalidation code, as well a callers who already
locked the child and parent dentry branch-configurations.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/dentry.c |   87 +++---
 fs/unionfs/union.h  |3 ++
 2 files changed, 57 insertions(+), 33 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index afa2120..3bd2dfb 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -285,6 +285,59 @@ void purge_sb_data(struct super_block *sb)
 }
 
 /*
+ * Revalidate a single file/symlink/special dentry.  Assume that info nodes
+ * of the dentry and its parent are locked.  Assume that parent(s) are all
+ * valid already, but the child may not yet be valid.  Returns true if
+ * valid, false otherwise.
+ */
+bool __unionfs_d_revalidate_one_locked(struct dentry *dentry,
+  struct nameidata *nd,
+  bool willwrite)
+{
+   bool valid = false; /* default is invalid */
+   int sbgen, dgen, bindex;
+
+   verify_locked(dentry);
+   verify_locked(dentry-d_parent);
+
+   sbgen = atomic_read(UNIONFS_SB(dentry-d_sb)-generation);
+   dgen = atomic_read(UNIONFS_D(dentry)-generation);
+
+   if (unlikely(is_newer_lower(dentry))) {
+   /* root dentry special case as aforementioned */
+   if (IS_ROOT(dentry)) {
+   unionfs_copy_attr_times(dentry-d_inode);
+   } else {
+   /*
+* reset generation number to zero, guaranteed to be
+* old
+*/
+   dgen = 0;
+   atomic_set(UNIONFS_D(dentry)-generation, dgen);
+   }
+   if (!willwrite)
+   purge_inode_data(dentry-d_inode);
+   }
+   valid = __unionfs_d_revalidate_one(dentry, nd);
+
+   /*
+* If __unionfs_d_revalidate_one() succeeded above, then it will
+* have incremented the refcnt of the mnt's, but also the branch
+* indices of the dentry will have been updated (to take into
+* account any branch insertions/deletion.  So the current
+* dbstart/dbend match the current, and new, indices of the mnts
+* which __unionfs_d_revalidate_one has incremented.  Note: the if
+* test below does not depend on whether chain_len was 0 or greater.
+*/
+   if (!valid || sbgen == dgen)
+   goto out;
+   for (bindex = dbstart(dentry); bindex = dbend(dentry); bindex++)
+   unionfs_mntput(dentry, bindex);
+out:
+   return valid;
+}
+
+/*
  * Revalidate a parent chain of dentries, then the actual node.
  * Assumes that dentry is locked, but will lock all parents if/when needed.
  *
@@ -404,42 +457,10 @@ out_this:
if (dentry != dentry-d_parent)
unionfs_lock_dentry(dentry-d_parent,
UNIONFS_DMUTEX_REVAL_PARENT);
-   dgen = atomic_read(UNIONFS_D(dentry)-generation);
-
-   if (unlikely(is_newer_lower(dentry))) {
-   /* root dentry special case as aforementioned */
-   if (IS_ROOT(dentry)) {
-   unionfs_copy_attr_times(dentry-d_inode);
-   } else {
-   /*
-* reset generation number to zero, guaranteed to be
-* old
-*/
-   dgen = 0;
-   atomic_set(UNIONFS_D(dentry)-generation, dgen);
-   }
-   if (!willwrite)
-   purge_inode_data(dentry-d_inode);
-   }
-   valid = __unionfs_d_revalidate_one(dentry, nd);
+   valid = __unionfs_d_revalidate_one_locked(dentry, nd, willwrite);
if (dentry != dentry-d_parent)
unionfs_unlock_dentry(dentry-d_parent);
 
-   /*
-* If __unionfs_d_revalidate_one() succeeded above, then it will
-* have incremented the refcnt of the mnt's, but also the branch
-* indices of the dentry will have been updated (to take into
-* account any branch insertions/deletion.  So the current
-* dbstart/dbend match the current, and new, indices of the mnts
-* which __unionfs_d_revalidate_one has incremented.  Note: the if
-* test below does not depend on whether chain_len was 0 or greater.
-*/
-   if (valid  sbgen != dgen)
-   for (bindex = dbstart(dentry);
-bindex = dbend(dentry);
-bindex++)
-   unionfs_mntput(dentry, bindex);
-
 out_free:
/* unlock/dput all dentries in chain and return status */
if (chain_len  0) {
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index 0e0ccde..5001b07 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -368,6 +368,9 @@ extern int

[PATCH 08/17] Unionfs: improve debugging in copy_attr_times

[Erez Zadok]

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/subr.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
index 68a6280..1a40f63 100644
--- a/fs/unionfs/subr.c
+++ b/fs/unionfs/subr.c
@@ -247,8 +247,14 @@ void unionfs_copy_attr_times(struct inode *upper)
int bindex;
struct inode *lower;
 
-   if (!upper || ibstart(upper)  0)
+   if (!upper)
return;
+   if (ibstart(upper)  0) {
+#ifdef CONFIG_UNION_FS_DEBUG
+   WARN_ON(ibstart(upper)  0);
+#endif /* CONFIG_UNION_FS_DEBUG */
+   return;
+   }
for (bindex = ibstart(upper); bindex = ibend(upper); bindex++) {
lower = unionfs_lower_inode_idx(upper, bindex);
if (!lower)
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 13/17] Unionfs: use dget_parent in revalidation code

[Erez Zadok]

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/dentry.c |   13 -
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index a956b94..f8f65e1 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -410,15 +410,10 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, 
struct nameidata *nd,
goto out;
}
 
-   /*
-* lock all dentries in chain, in child to parent order.
-* if failed, then sleep for a little, then retry.
-*/
-   dtmp = dentry-d_parent;
-   for (i = chain_len-1; i = 0; i--) {
-   chain[i] = dget(dtmp);
-   dtmp = dtmp-d_parent;
-   }
+   /* grab all dentries in chain, in child to parent order */
+   dtmp = dentry;
+   for (i = chain_len-1; i = 0; i--)
+   dtmp = chain[i] = dget_parent(dtmp);
 
/*
 * call __unionfs_d_revalidate_one() on each dentry, but in parent
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 05/17] Unionfs: initialize path_save variable

[Erez Zadok]

This is not strictly necessary, but it helps quiet a gcc-4.2 warning (a good
optimizer may optimize this initialization away).

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
Signed-off-by: Josef 'Jeff' Sipek [EMAIL PROTECTED]
---
 fs/unionfs/inode.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 0b92da2..8d939dc 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -246,7 +246,7 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 struct dentry *dentry,
 struct nameidata *nd)
 {
-   struct path path_save;
+   struct path path_save = {NULL, NULL};
struct dentry *ret;
 
unionfs_read_lock(dentry-d_sb, UNIONFS_SMUTEX_CHILD);
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 06/17] Unionfs: extend dentry branch configuration lock in open

[Erez Zadok]

Dentry branch configuration info node lock should extend to calls to
copy_attr_times.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/commonfops.c |   14 --
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index f37192f..96473c4 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -521,11 +521,12 @@ int unionfs_open(struct inode *inode, struct file *file)
 {
int err = 0;
struct file *lower_file = NULL;
-   struct dentry *dentry = NULL;
+   struct dentry *dentry = file-f_path.dentry;
int bindex = 0, bstart = 0, bend = 0;
int size;
 
unionfs_read_lock(inode-i_sb, UNIONFS_SMUTEX_PARENT);
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
file-private_data =
kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
@@ -551,9 +552,6 @@ int unionfs_open(struct inode *inode, struct file *file)
goto out;
}
 
-   dentry = file-f_path.dentry;
-   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
-
bstart = fbstart(file) = dbstart(dentry);
bend = fbend(file) = dbend(dentry);
 
@@ -573,15 +571,12 @@ int unionfs_open(struct inode *inode, struct file *file)
if (!lower_file)
continue;
 
-   branchput(file-f_path.dentry-d_sb, bindex);
+   branchput(dentry-d_sb, bindex);
/* fput calls dput for lower_dentry */
fput(lower_file);
}
}
 
-   /* XXX: should this unlock be moved to the function bottom? */
-   unionfs_unlock_dentry(dentry);
-
 out:
if (err) {
kfree(UNIONFS_F(file)-lower_files);
@@ -590,12 +585,11 @@ out:
}
 out_nofree:
if (!err) {
-   dentry = file-f_path.dentry;
-   unionfs_copy_attr_times(dentry-d_parent-d_inode);
unionfs_copy_attr_times(inode);
unionfs_check_file(file);
unionfs_check_inode(inode);
}
+   unionfs_unlock_dentry(dentry);
unionfs_read_unlock(inode-i_sb);
return err;
 }
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 15/17] Unionfs: embed a struct path into struct nameidata instead of nd dentrymnt

[Erez Zadok]

From: Andrew Morton [EMAIL PROTECTED]

Signed-off-by: Andrew Morton [EMAIL PROTECTED]
Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/inode.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 2e791fd..640969d 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -254,8 +254,8 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 
/* save the dentry  vfsmnt from namei */
if (nd) {
-   path_save.dentry = nd-dentry;
-   path_save.mnt = nd-mnt;
+   path_save.dentry = nd-path.dentry;
+   path_save.mnt = nd-path.mnt;
}
 
/*
@@ -266,8 +266,8 @@ static struct dentry *unionfs_lookup(struct inode *parent,
 
/* restore the dentry  vfsmnt in namei */
if (nd) {
-   nd-dentry = path_save.dentry;
-   nd-mnt = path_save.mnt;
+   nd-path.dentry = path_save.dentry;
+   nd-path.mnt = path_save.mnt;
}
if (!IS_ERR(ret)) {
if (ret)
@@ -885,7 +885,7 @@ static int unionfs_permission(struct inode *inode, int mask,
const int write_mask = (mask  MAY_WRITE)  !(mask  MAY_READ);
 
if (nd)
-   unionfs_lock_dentry(nd-dentry, UNIONFS_DMUTEX_CHILD);
+   unionfs_lock_dentry(nd-path.dentry, UNIONFS_DMUTEX_CHILD);
 
if (!UNIONFS_I(inode)-lower_inodes) {
if (is_file)/* dirs can be unlinked but chdir'ed to */
@@ -960,7 +960,7 @@ out:
unionfs_check_inode(inode);
unionfs_check_nd(nd);
if (nd)
-   unionfs_unlock_dentry(nd-dentry);
+   unionfs_unlock_dentry(nd-path.dentry);
return err;
 }
 
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 11/17] Unionfs: lock parents' branch configuration fixes

[Erez Zadok]

Ensure that we lock the branch configuration of parent and child dentries in
operations which need it, and in the right order.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/commonfops.c |   31 +---
 fs/unionfs/dentry.c |   26 +---
 fs/unionfs/inode.c  |   58 +++---
 fs/unionfs/union.h  |5 ++-
 fs/unionfs/unlink.c |   11 -
 5 files changed, 91 insertions(+), 40 deletions(-)

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
index 96473c4..491e2ff 100644
--- a/fs/unionfs/commonfops.c
+++ b/fs/unionfs/commonfops.c
@@ -300,8 +300,9 @@ out:
  * Revalidate the struct file
  * @file: file to revalidate
  * @willwrite: true if caller may cause changes to the file; false otherwise.
+ * Caller must lock/unlock dentry's branch configuration.
  */
-int unionfs_file_revalidate(struct file *file, bool willwrite)
+int unionfs_file_revalidate_locked(struct file *file, bool willwrite)
 {
struct super_block *sb;
struct dentry *dentry;
@@ -311,7 +312,6 @@ int unionfs_file_revalidate(struct file *file, bool 
willwrite)
int err = 0;
 
dentry = file-f_path.dentry;
-   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
sb = dentry-d_sb;
 
/*
@@ -416,7 +416,17 @@ out:
 out_nofree:
if (!err)
unionfs_check_file(file);
-   unionfs_unlock_dentry(dentry);
+   return err;
+}
+
+int unionfs_file_revalidate(struct file *file, bool willwrite)
+{
+   int err;
+
+   unionfs_lock_dentry(file-f_path.dentry, UNIONFS_DMUTEX_CHILD);
+   err = unionfs_file_revalidate_locked(file, willwrite);
+   unionfs_unlock_dentry(file-f_path.dentry);
+
return err;
 }
 
@@ -524,9 +534,18 @@ int unionfs_open(struct inode *inode, struct file *file)
struct dentry *dentry = file-f_path.dentry;
int bindex = 0, bstart = 0, bend = 0;
int size;
+   int valid = 0;
 
unionfs_read_lock(inode-i_sb, UNIONFS_SMUTEX_PARENT);
unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+   if (dentry != dentry-d_parent)
+   unionfs_lock_dentry(dentry-d_parent, UNIONFS_DMUTEX_PARENT);
+
+   valid = __unionfs_d_revalidate_chain(dentry-d_parent, NULL, false);
+   if (unlikely(!valid)) {
+   err = -ESTALE;
+   goto out_nofree;
+   }
 
file-private_data =
kzalloc(sizeof(struct unionfs_file_info), GFP_KERNEL);
@@ -589,6 +608,8 @@ out_nofree:
unionfs_check_file(file);
unionfs_check_inode(inode);
}
+   if (dentry != dentry-d_parent)
+   unionfs_unlock_dentry(dentry-d_parent);
unionfs_unlock_dentry(dentry);
unionfs_read_unlock(inode-i_sb);
return err;
@@ -797,8 +818,9 @@ int unionfs_flush(struct file *file, fl_owner_t id)
int bindex, bstart, bend;
 
unionfs_read_lock(dentry-d_sb, UNIONFS_SMUTEX_PARENT);
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
 
-   err = unionfs_file_revalidate(file, true);
+   err = unionfs_file_revalidate_locked(file, true);
if (unlikely(err))
goto out;
unionfs_check_file(file);
@@ -824,6 +846,7 @@ int unionfs_flush(struct file *file, fl_owner_t id)
 
 out:
unionfs_check_file(file);
+   unionfs_unlock_dentry(file-f_path.dentry);
unionfs_read_unlock(dentry-d_sb);
return err;
 }
diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
index 3bd2dfb..17b297d 100644
--- a/fs/unionfs/dentry.c
+++ b/fs/unionfs/dentry.c
@@ -363,6 +363,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, 
struct nameidata *nd,
chain_len = 0;
sbgen = atomic_read(UNIONFS_SB(dentry-d_sb)-generation);
dtmp = dentry-d_parent;
+   verify_locked(dentry);
if (dentry != dtmp)
unionfs_lock_dentry(dtmp, UNIONFS_DMUTEX_REVAL_PARENT);
dgen = atomic_read(UNIONFS_D(dtmp)-generation);
@@ -453,7 +454,7 @@ bool __unionfs_d_revalidate_chain(struct dentry *dentry, 
struct nameidata *nd,
 
 out_this:
/* finally, lock this dentry and revalidate it */
-   verify_locked(dentry);
+   verify_locked(dentry);  /* verify child is locked */
if (dentry != dentry-d_parent)
unionfs_lock_dentry(dentry-d_parent,
UNIONFS_DMUTEX_REVAL_PARENT);
@@ -491,24 +492,20 @@ static int unionfs_d_revalidate(struct dentry *dentry, 
struct nameidata *nd)
return err;
 }
 
-/*
- * At this point no one can reference this dentry, so we don't have to be
- * careful about concurrent access.
- */
 static void unionfs_d_release(struct dentry *dentry)
 {
int bindex, bstart, bend;
 
unionfs_read_lock(dentry-d_sb, UNIONFS_SMUTEX_CHILD);
+   /* must lock our branch configuration here */
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD

[PATCH 14/17] Unionfs: stop using iget() and read_inode()

[Erez Zadok]

From: David Howells [EMAIL PROTECTED]

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]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/main.c  |   11 +--
 fs/unionfs/super.c |   19 ++-
 fs/unionfs/union.h |1 +
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
index ba3471d..4bc2c66 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 175840f..b71fc2a 100644
--- a/fs/unionfs/super.c
+++ b/fs/unionfs/super.c
@@ -24,11 +24,19 @@
  */
 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)
 {
int size;
-   struct unionfs_inode_info *info = UNIONFS_I(inode);
+   struct unionfs_inode_info *info;
+   struct inode *inode;
 
+   inode = iget_locked(sb, ino);
+   if (!inode)
+   return ERR_PTR(-ENOMEM);
+   if (!(inode-i_state  I_NEW))
+   return inode;
+
+   info = UNIONFS_I(inode);
memset(info, 0, offsetof(struct unionfs_inode_info, vfs_inode));
info-bstart = -1;
info-bend = -1;
@@ -44,7 +52,8 @@ static void unionfs_read_inode(struct inode *inode)
if (unlikely(!info-lower_inodes)) {
printk(KERN_CRIT unionfs: no kernel memory when allocating 
   lower-pointer array!\n);
-   BUG();
+   iget_failed(inode);
+   return ERR_PTR(-ENOMEM);
}
 
inode-i_version++;
@@ -60,7 +69,8 @@ static void unionfs_read_inode(struct inode *inode)
inode-i_atime.tv_sec = inode-i_atime.tv_nsec = 0;
inode-i_mtime.tv_sec = inode-i_mtime.tv_nsec = 0;
inode-i_ctime.tv_sec = inode-i_ctime.tv_nsec = 0;
-
+   unlock_new_inode(inode);
+   return inode;
 }
 
 /*
@@ -1025,7 +1035,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 1bf0c09..533806c 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -364,6 +364,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);
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: unionfs_copy_attr_times oopses

[Erez Zadok]

In message [EMAIL PROTECTED], Hugh Dickins writes:

 Hi Erez,
 
 Aside from the occasional unionfs: new lower inode mtime messages
 on directories (which I've got into the habit of ignoring now), the
 only problem I'm still suffering with unionfs over tmpfs (not tested
 any other fs's below it recently) is oops in unionfs_copy_attr_times.
 
 I believe I'm working with your latest: 2.6.24-rc8-mm1 plus the four
 patches you posted to lkml on 26 Jan.  But this problem has been around
 for a while before that: I'd been hoping to debug it myself, but taken
 too long to make too little progress, so now handing over to you.
 
 The oops occurs while doing repeated make -j20 kernel builds in a
 unionfs mount of a tmpfs (though I doubt tmpfs is relevant): most of
 my testing was while swapping, but today I find that's irrelevant,
 and it should happen much quicker without.  SMP kernels (4 cpus),
 I haven't tried UP; happens with or without PREEMPT, may just be
 coincidence that it happens quicker on the machines with PREEMPT.
 
 Most commonly it's unionfs_copy_attr_times called from unionfs_create,
 but that's probably just the most common route in this workload:
 I've seen it also when called from unionfs_rename or unionfs_open or
 unionfs_unlink.  It looks like there's a locking or refcounting bug,
 hence a race: the unionfs_inode_info which unionfs_copy_attr_times
 is working on gets changed underneath it, so it oopses on NULL
 lower_inodes.
[...]

Hugh,

Check out my latest set of patches (which correspond to release 2.2.4 of
Unionfs).  Thanks to your info and the patch, I was able to trigger several
races more frequently, and fix them.  I've tested my code with make -j N
(for N=4 and N=20), on a 4 cpu machine a well as a 2 cpu machine (w/
different amounts of memory and CPU speeds, also 32-bit vs 64-bit); I ran a
kernel compile for ~10-12 hours.  With the patches I just posted, I wasn't
able to trigger any of the WARN_ON's in unionfs_copy_attr_times.  I also
tried it while flushing caches via /proc, and/or performing branch-mgmt
commands in unionfs.

Give it a good shake and let me know what you find.

Thanks,
Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] unhide CONFIG_DEBUG_SECTION_MISMATCH

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Sam Ravnborg writes:
> On Thu, Feb 14, 2008 at 03:54:04PM -0500, Erez Zadok wrote:
> > Using: v2.6.25-rc1-120-ge760e71
> > 
> > In a normal compilation, I might this message:
> > 
> > ...
> >   MODPOST vmlinux.o
> > WARNING: modpost: Found 4 section mismatch(es).
> > To see full details build your kernel with:
> > 'make CONFIG_DEBUG_SECTION_MISMATCH=y'
> > ...
> > 
> > I can indeed try to re-make, passing CONFIG_DEBUG_SECTION_MISMATCH=y on the
> > command line, but I can't turn on the option in my .config.  That's because
> > the option depends on "UNDEFINED".  (Was that an attempt to "hide" the
> > option?  Why?)  The following small patch allows me to set the option in my
> > .config.
> 
> It was done so on purpose.
> The rationale here is that we yet have too many section mismatch for
> a typical build to pass my "good stomach" test.
> So until we get down on an acceptable level we should not be
> too noisy.
> And I wanted it to be turned off also for allyesconfig builds.
> 
> I hope to spend time on the reaming warnings soon but the
> option will not be enabled until latest next mergewindow.
> 
> You could argue that I could have doen this in better ways
> The present version was just simple.
> 
>   Sam

Thanks, Sam.  That explains it.  Perhaps a small comment next to the Kconfig
entry might help explain why the option is "hidden" for now, so others won't
bother unhiding it again.

Of course, forcing it on might help people to fix those warnings sooner. :-)

Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] unhide CONFIG_DEBUG_SECTION_MISMATCH

[Erez Zadok]

Using: v2.6.25-rc1-120-ge760e71

In a normal compilation, I might this message:

...
  MODPOST vmlinux.o
WARNING: modpost: Found 4 section mismatch(es).
To see full details build your kernel with:
'make CONFIG_DEBUG_SECTION_MISMATCH=y'
...

I can indeed try to re-make, passing CONFIG_DEBUG_SECTION_MISMATCH=y on the
command line, but I can't turn on the option in my .config.  That's because
the option depends on "UNDEFINED".  (Was that an attempt to "hide" the
option?  Why?)  The following small patch allows me to set the option in my
.config.

Cheers,
Erez.



Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index a370fe8..3399d9d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -81,7 +81,6 @@ config HEADERS_CHECK
 
 config DEBUG_SECTION_MISMATCH
bool "Enable full Section mismatch analysis"
-   depends on UNDEFINED
help
  The section mismatch analysis checks if there are illegal
  references from one section to another section.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] unhide CONFIG_DEBUG_SECTION_MISMATCH

[Erez Zadok]

Using: v2.6.25-rc1-120-ge760e71

In a normal compilation, I might this message:

...
  MODPOST vmlinux.o
WARNING: modpost: Found 4 section mismatch(es).
To see full details build your kernel with:
'make CONFIG_DEBUG_SECTION_MISMATCH=y'
...

I can indeed try to re-make, passing CONFIG_DEBUG_SECTION_MISMATCH=y on the
command line, but I can't turn on the option in my .config.  That's because
the option depends on UNDEFINED.  (Was that an attempt to hide the
option?  Why?)  The following small patch allows me to set the option in my
.config.

Cheers,
Erez.



Signed-off-by: Erez Zadok [EMAIL PROTECTED]

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index a370fe8..3399d9d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -81,7 +81,6 @@ config HEADERS_CHECK
 
 config DEBUG_SECTION_MISMATCH
bool Enable full Section mismatch analysis
-   depends on UNDEFINED
help
  The section mismatch analysis checks if there are illegal
  references from one section to another section.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] unhide CONFIG_DEBUG_SECTION_MISMATCH

[Erez Zadok]

In message [EMAIL PROTECTED], Sam Ravnborg writes:
 On Thu, Feb 14, 2008 at 03:54:04PM -0500, Erez Zadok wrote:
  Using: v2.6.25-rc1-120-ge760e71
  
  In a normal compilation, I might this message:
  
  ...
MODPOST vmlinux.o
  WARNING: modpost: Found 4 section mismatch(es).
  To see full details build your kernel with:
  'make CONFIG_DEBUG_SECTION_MISMATCH=y'
  ...
  
  I can indeed try to re-make, passing CONFIG_DEBUG_SECTION_MISMATCH=y on the
  command line, but I can't turn on the option in my .config.  That's because
  the option depends on UNDEFINED.  (Was that an attempt to hide the
  option?  Why?)  The following small patch allows me to set the option in my
  .config.
 
 It was done so on purpose.
 The rationale here is that we yet have too many section mismatch for
 a typical build to pass my good stomach test.
 So until we get down on an acceptable level we should not be
 too noisy.
 And I wanted it to be turned off also for allyesconfig builds.
 
 I hope to spend time on the reaming warnings soon but the
 option will not be enabled until latest next mergewindow.
 
 You could argue that I could have doen this in better ways
 The present version was just simple.
 
   Sam

Thanks, Sam.  That explains it.  Perhaps a small comment next to the Kconfig
entry might help explain why the option is hidden for now, so others won't
bother unhiding it again.

Of course, forcing it on might help people to fix those warnings sooner. :-)

Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] fix up kerneldoc in fs/ioctl.c a little bit

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Christoph Hellwig writes:
>  - remove non-standard in/out markers
>  - use tabs for formatting
> 
> 
> Signed-off-by: Christoph Hellwig <[EMAIL PROTECTED]>
> 
> Index: linux-2.6/fs/ioctl.c
> ===
> --- linux-2.6.orig/fs/ioctl.c 2008-02-09 07:49:02.0 +0100
> +++ linux-2.6/fs/ioctl.c  2008-02-09 07:49:20.0 +0100
> @@ -18,9 +18,9 @@
>  
>  /**
>   * vfs_ioctl - call filesystem specific ioctl methods
> - * @filp: [in] open file to invoke ioctl method on
> - * @cmd:  [in] ioctl command to execute
> - * @arg:  [in/out] command-specific argument for ioctl
> + * @filp:open file to invoke ioctl method on
> + * @cmd: ioctl command to execute
> + * @arg: command-specific argument for ioctl
>   *
>   * Invokes filesystem specific ->unlocked_ioctl, if one exists; otherwise
>   * invokes * filesystem specific ->ioctl method.  If neither method exists,
  ^

I also think this extra '*' in the last comment line above is spurious,
perhaps the result of a paragraph reformatting command that mixed comment
*'s with text.

Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] fix up kerneldoc in fs/ioctl.c a little bit

[Erez Zadok]

In message [EMAIL PROTECTED], Christoph Hellwig writes:
  - remove non-standard in/out markers
  - use tabs for formatting
 
 
 Signed-off-by: Christoph Hellwig [EMAIL PROTECTED]
 
 Index: linux-2.6/fs/ioctl.c
 ===
 --- linux-2.6.orig/fs/ioctl.c 2008-02-09 07:49:02.0 +0100
 +++ linux-2.6/fs/ioctl.c  2008-02-09 07:49:20.0 +0100
 @@ -18,9 +18,9 @@
  
  /**
   * vfs_ioctl - call filesystem specific ioctl methods
 - * @filp: [in] open file to invoke ioctl method on
 - * @cmd:  [in] ioctl command to execute
 - * @arg:  [in/out] command-specific argument for ioctl
 + * @filp:open file to invoke ioctl method on
 + * @cmd: ioctl command to execute
 + * @arg: command-specific argument for ioctl
   *
   * Invokes filesystem specific -unlocked_ioctl, if one exists; otherwise
   * invokes * filesystem specific -ioctl method.  If neither method exists,
  ^

I also think this extra '*' in the last comment line above is spurious,
perhaps the result of a paragraph reformatting command that mixed comment
*'s with text.

Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [UNIONFS] 00/29 Unionfs and related patches pre-merge review (v2)

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Al Viro writes:
> On Sat, Jan 26, 2008 at 12:08:30AM -0500, Erez Zadok wrote:

[concerns about lower directories moving around...]

> You are thinking about non-interesting case.  _Files_ are not much
> of a problem.  Directory tree is.  The real problems with all unionfs and
> stacking implementations I've seen so far, all way back to Heidemann et.al.
> start when topology of the underlying layer changes.

OK, so if I understand you, your concerns center around the fact that lower
directories can be moved around (i.e., topology changes), what happens then
for operations that go through the stackable f/s, and what should users
expect to see.

> If you have clear
> semantics for unionfs behaviour in presence of such things, by all means,
> publish it - as far as I know *nobody* had done that; not even on the
> "what should we see when..." level, nevermind the implementation.

Since stacking and NFS have some similarities, I first checked w/ the NFS
people to see what are their semantics in a similar scenario: an NFS client
could be validating a directory, then issue, say, a ->create; but in
between, the server could have moved the directory that was validated.  In
NFS, the ->create operation succeeds, and creates the file in the new
location of the directory which was validated.

Unionfs's behavior is similar: the newly created file will be successfully
created in the moved directory.  The only exception is that if a lower
branch is marked readonly by unionfs, a copyup will take place.

This had not been a problem for unionfs users to date.  The main reason is
that when unionfs users modify lower files, they often do so while there's
little to no activity going through the union itself.  And while it doesn't
prevent directories from being moved around, this common usage mode does
reduce the frequency in which topology changes can be an issue for unionfs
users.

I'll submit a patch to document this behavior.

> > Perhaps this general topic is a good one to discuss at more length at LSF?
> > Suggestions are welcome.
> 
> It would; I honestly do not know if the problem is solvable with the
> (lack of) constraints you apparently want.  Again, the real PITA begins
> when you start dealing with pieces of underlying trees getting moved
> around, changing parents, etc.  Cross-directory rename() certainly rates
> very high on the list of "WTF had they been smoking in UCB?" misfeatures,
> but it's there and it has to be dealt with.

Well, it was UCB, UCLA, and Sun.  I don't think back in the early 90s they
were too concerned about topology changes; f/s stacking was a new idea and
they wanted to explore what can be done with it conceptually, not produce
commercial-grade software (still, remind me to tell you the first-hand story
I've learned about of how full-blown stacking _almost_ made it into Solaris
2.0 :-)

The only known reference to try and address this coherency problem was
Heidemann's SOSP'95 paper titled "Performance of cache coherence in
stackable filing."  The paper advocated changing the whole VFS and the
caches (page cache + dnlc) to create a "unified cache manager" that was
aware of complex layering topologies (including fan-out and fan-in).  It was
even able to handle compression layers, where file data offsets change b/t
the layers (a nasty problem).  Code for this unified cache manager was never
released AFAIK.  I think Heidemann's approach was elegant, but I don't think
it was practical as it required radical VFS/VM surgery.  Ironically, MS
Windows has a single I/O cache manager that all storage and filesystem
modules talk to directly (they're not allowed to pass IRPs directly b/t
layers): so Windows can handle such coherency better than most Unix systems
can today.

I've always thought of a different way to allow users to write to lower
branches -- through the union.  This is similar to what an old AT
unioning-like file system named "3DFS" did.  3DFS introduced a new directory
called "..." so if you cd to /mntpt/... then you got to the next level down
the stack (as if you popped the top one and now you see how the union looks
like without the top layer).  And if you "cd /mntpt/.../..." then you see
the view without the top two layers, etc.

So my idea is similar: to introduce virtual directory views that restrict
access to a single lower branch within the union.  So if someone does a "cd
/mnt/unionfs/.1" then they get access to branch 1; "cd /mnt/unionfs/.2" gets
access to branch 2; etc.  While this technique will waste a few names, it's
probably worth the savings in terms of cache-coherency pains (plus, the
actual virtual directory names can be configurable at mount time to allow
users to choose a non-conflicting dir name).  With this idea, users will be
actually accessing a one-branch union, but all ops

Re: [UNIONFS] 00/29 Unionfs and related patches pre-merge review (v2)

[Erez Zadok]

In message [EMAIL PROTECTED], Al Viro writes:
 On Sat, Jan 26, 2008 at 12:08:30AM -0500, Erez Zadok wrote:

[concerns about lower directories moving around...]

 You are thinking about non-interesting case.  _Files_ are not much
 of a problem.  Directory tree is.  The real problems with all unionfs and
 stacking implementations I've seen so far, all way back to Heidemann et.al.
 start when topology of the underlying layer changes.

OK, so if I understand you, your concerns center around the fact that lower
directories can be moved around (i.e., topology changes), what happens then
for operations that go through the stackable f/s, and what should users
expect to see.

 If you have clear
 semantics for unionfs behaviour in presence of such things, by all means,
 publish it - as far as I know *nobody* had done that; not even on the
 what should we see when... level, nevermind the implementation.

Since stacking and NFS have some similarities, I first checked w/ the NFS
people to see what are their semantics in a similar scenario: an NFS client
could be validating a directory, then issue, say, a -create; but in
between, the server could have moved the directory that was validated.  In
NFS, the -create operation succeeds, and creates the file in the new
location of the directory which was validated.

Unionfs's behavior is similar: the newly created file will be successfully
created in the moved directory.  The only exception is that if a lower
branch is marked readonly by unionfs, a copyup will take place.

This had not been a problem for unionfs users to date.  The main reason is
that when unionfs users modify lower files, they often do so while there's
little to no activity going through the union itself.  And while it doesn't
prevent directories from being moved around, this common usage mode does
reduce the frequency in which topology changes can be an issue for unionfs
users.

I'll submit a patch to document this behavior.

  Perhaps this general topic is a good one to discuss at more length at LSF?
  Suggestions are welcome.
 
 It would; I honestly do not know if the problem is solvable with the
 (lack of) constraints you apparently want.  Again, the real PITA begins
 when you start dealing with pieces of underlying trees getting moved
 around, changing parents, etc.  Cross-directory rename() certainly rates
 very high on the list of WTF had they been smoking in UCB? misfeatures,
 but it's there and it has to be dealt with.

Well, it was UCB, UCLA, and Sun.  I don't think back in the early 90s they
were too concerned about topology changes; f/s stacking was a new idea and
they wanted to explore what can be done with it conceptually, not produce
commercial-grade software (still, remind me to tell you the first-hand story
I've learned about of how full-blown stacking _almost_ made it into Solaris
2.0 :-)

The only known reference to try and address this coherency problem was
Heidemann's SOSP'95 paper titled Performance of cache coherence in
stackable filing.  The paper advocated changing the whole VFS and the
caches (page cache + dnlc) to create a unified cache manager that was
aware of complex layering topologies (including fan-out and fan-in).  It was
even able to handle compression layers, where file data offsets change b/t
the layers (a nasty problem).  Code for this unified cache manager was never
released AFAIK.  I think Heidemann's approach was elegant, but I don't think
it was practical as it required radical VFS/VM surgery.  Ironically, MS
Windows has a single I/O cache manager that all storage and filesystem
modules talk to directly (they're not allowed to pass IRPs directly b/t
layers): so Windows can handle such coherency better than most Unix systems
can today.

I've always thought of a different way to allow users to write to lower
branches -- through the union.  This is similar to what an old ATT
unioning-like file system named 3DFS did.  3DFS introduced a new directory
called ... so if you cd to /mntpt/... then you got to the next level down
the stack (as if you popped the top one and now you see how the union looks
like without the top layer).  And if you cd /mntpt/.../... then you see
the view without the top two layers, etc.

So my idea is similar: to introduce virtual directory views that restrict
access to a single lower branch within the union.  So if someone does a cd
/mnt/unionfs/.1 then they get access to branch 1; cd /mnt/unionfs/.2 gets
access to branch 2; etc.  While this technique will waste a few names, it's
probably worth the savings in terms of cache-coherency pains (plus, the
actual virtual directory names can be configurable at mount time to allow
users to choose a non-conflicting dir name).  With this idea, users will be
actually accessing a one-branch union, but all ops and locks will have to go
through the union: no one would be able to modify lower files directly.

However, for this view idea to work, I'll need a way to lock-out or hide
the lower directories from

Re: unionfs_copy_attr_times oopses

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Hugh Dickins writes:
> Hi Erez,
> 
> Aside from the occasional "unionfs: new lower inode mtime" messages
> on directories (which I've got into the habit of ignoring now), the
> only problem I'm still suffering with unionfs over tmpfs (not tested
> any other fs's below it recently) is oops in unionfs_copy_attr_times.
[...]

Thanks for the report and patch(es).  I'll start looking at this over the
weekend.  Could you do me a favor and open a bugzilla report
(https://bugzilla.filesystems.org/), upload your patches, etc.?  It'd be
easier for us to exchange info/patches and track the problem that way.

Thanks,
Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: unionfs_copy_attr_times oopses

[Erez Zadok]

In message [EMAIL PROTECTED], Hugh Dickins writes:
 Hi Erez,
 
 Aside from the occasional unionfs: new lower inode mtime messages
 on directories (which I've got into the habit of ignoring now), the
 only problem I'm still suffering with unionfs over tmpfs (not tested
 any other fs's below it recently) is oops in unionfs_copy_attr_times.
[...]

Thanks for the report and patch(es).  I'll start looking at this over the
weekend.  Could you do me a favor and open a bugzilla report
(https://bugzilla.filesystems.org/), upload your patches, etc.?  It'd be
easier for us to exchange info/patches and track the problem that way.

Thanks,
Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [UNIONFS] 00/29 Unionfs and related patches pre-merge review (v2)

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Al Viro writes:
> After grep for locking-related things:
> 
>   * lock_parent(): who said that you won't get dentry moved
> before managing to grab i_mutex on parent?  While we are at it,
> who said that you won't get dentry moved between fetching d_parent
> and doing dget()?  In that case parent could've been _freed_ before
> you get to dget().

OK, so looks like I should use dget_parent() in my lock_parent(), as I've
done elsewhere.  I'll also take a look at all instances in which I get
dentry->d_parent and see if a d_lock is needed there.

>   * in create_parents():
> +   struct inode *inode = lower_dentry->d_inode;
> +   /*
> +* If we get here, it means that we created a new
> +* dentry+inode, but copying permissions failed.
> +* Therefore, we should delete this inode and dput
> +* the dentry so as not to leave cruft behind.
> +*/
> +   if (lower_dentry->d_op && lower_dentry->d_op->d_iput)
> +   lower_dentry->d_op->d_iput(lower_dentry,
> +  inode);
> +   else
> +   iput(inode);
> +   lower_dentry->d_inode = NULL;
> +   dput(lower_dentry);
> +   lower_dentry = ERR_PTR(err);
> +   goto out;
> Really?  So what happens if it had become positive after your test and
> somebody had looked it up in lower layer and just now happens to be
> in the middle of operations on it?  Will be thucking frilled by that...

Good catch.  That ->d_iput call was an old fix to a bug that has since been
fixed more cleanly and generically in our copyup_permission routine and our
unionfs_d_iput.  I've removed the above ->d_iput "if" and tested to verify
that it's indeed unnecessary.

>   * __unionfs_rename():
> +   lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
> +   err = vfs_rename(lower_old_dir_dentry->d_inode, lower_old_dentry,
> +lower_new_dir_dentry->d_inode, lower_new_dentry);
> +   unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
> 
> Uh-huh...  To start with, what guarantees that your lower_old_dentry
> is still a child of your lower_old_dir_dentry?

We dget/dget_parent the old/new dentry and parents a few lines above
(actually, it looked like I forgot to dget(lower_new_dentry) -- fixed).
This is a generic stackable f/s issue: ecryptfs does the same stuff before
calling vfs_rename() on the lower objects.

> What's more, you are
> not checking the result of lock_rename(), i.e. asking for serious trouble.

OK.  I'm now checking for the return from lock_rename for ancestor/rename
rules.  I'm CC'ing Mike Halcrow so he can do the same for ecryptfs.

>   * revalidation stuff: err...  how the devil can it work for
> directories, when there's nothing to prevent changes in underlying
> layers between ->d_revalidate() and operation itself?  For the upper
> layer (unionfs itself) everything's more or less fine, but the rest
> of that...

In a stacked f/s, we keep references to the lower dentries/inodes, so they
can't disappear on us (that happens in our interpose function, called from
our ->lookup).  On entry to every f/s method in unionfs, we first perform
lightweight revalidation of our dentry against the lower ones: we check if
m/ctime changed (users modifying lower files) or if the generation# b/t our
super and the our dentries have changed (branch-management took place); if
needed, then we perform a full revalidation of all lower objects (while
holding a lock on the branch configuration).  If we have to do a full reval
upon entry to our ->op, and the reval failed, then we return an appropriate
error; o/w we proceed.  (In certain cases, the VFS re-issues a lookup if the
f/s says that it's dentry is invalid.)

Without changes to the VFS, I don't see how else I can ensure cache
coherency cleanly, while allowing users to modify lower files; this feature
is very useful to some unionfs users, who depend on it (so even if I could
"lock out" the lower directories from being modified, there will be users
who'd still want to be able to modify lower files).

BTW, my sense of the relationship b/t upper and lower objects and their
validity in a stackable f/s, is that it's similar to the relationship b/t
the NFS client and server -- the client can't be sure that a file on the
server doesn't change b/t ->revalidate and ->op (hence nfs's reliance on dir
mtime checks).

Perhaps this general topic is a good one to discuss at more length at LSF?
Suggestions are welcome.

Thanks,
Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  

[PATCH 4/4] Unionfs: lock_rename related locking fixes

[Erez Zadok]

CC: Mike Halcrow <[EMAIL PROTECTED]>

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/rename.c |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index 9306a2b..5ab13f9 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -29,6 +29,7 @@ static int __unionfs_rename(struct inode *old_dir, struct 
dentry *old_dentry,
struct dentry *lower_new_dir_dentry;
struct dentry *lower_wh_dentry;
struct dentry *lower_wh_dir_dentry;
+   struct dentry *trap;
char *wh_name = NULL;
 
lower_new_dentry = unionfs_lower_dentry_idx(new_dentry, bindex);
@@ -95,6 +96,7 @@ static int __unionfs_rename(struct inode *old_dir, struct 
dentry *old_dentry,
goto out;
 
dget(lower_old_dentry);
+   dget(lower_new_dentry);
lower_old_dir_dentry = dget_parent(lower_old_dentry);
lower_new_dir_dentry = dget_parent(lower_new_dentry);
 
@@ -122,9 +124,20 @@ static int __unionfs_rename(struct inode *old_dir, struct 
dentry *old_dentry,
 
/* see Documentation/filesystems/unionfs/issues.txt */
lockdep_off();
-   lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
+   trap = lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
+   /* source should not be ancenstor of target */
+   if (trap == lower_old_dentry) {
+   err = -EINVAL;
+   goto out_err_unlock;
+   }
+   /* target should not be ancenstor of source */
+   if (trap == lower_new_dentry) {
+   err = -ENOTEMPTY;
+   goto out_err_unlock;
+   }
err = vfs_rename(lower_old_dir_dentry->d_inode, lower_old_dentry,
 lower_new_dir_dentry->d_inode, lower_new_dentry);
+out_err_unlock:
unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
lockdep_on();
 
@@ -132,6 +145,7 @@ out_dput:
dput(lower_old_dir_dentry);
dput(lower_new_dir_dentry);
dput(lower_old_dentry);
+   dput(lower_new_dentry);
 
 out:
if (!err) {
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] Unionfs: remove unnecessary call to d_iput

[Erez Zadok]

This old code was to fix a bug which has long since been fixed in our
copyup_permission and unionfs_d_iput.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/copyup.c |   13 -
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 16b2c7c..8663224 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -807,19 +807,6 @@ begin:
 lower_dentry);
unlock_dir(lower_parent_dentry);
if (err) {
-   struct inode *inode = lower_dentry->d_inode;
-   /*
-* If we get here, it means that we created a new
-* dentry+inode, but copying permissions failed.
-* Therefore, we should delete this inode and dput
-* the dentry so as not to leave cruft behind.
-*/
-   if (lower_dentry->d_op && lower_dentry->d_op->d_iput)
-   lower_dentry->d_op->d_iput(lower_dentry,
-  inode);
-   else
-   iput(inode);
-   lower_dentry->d_inode = NULL;
dput(lower_dentry);
lower_dentry = ERR_PTR(err);
goto out;
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/4] Unionfs: d_parent related locking fixes

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/copyup.c |3 +--
 fs/unionfs/union.h  |4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 8663224..9beac01 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -716,8 +716,7 @@ struct dentry *create_parents(struct inode *dir, struct 
dentry *dentry,
child_dentry = parent_dentry;
 
/* find the parent directory dentry in unionfs */
-   parent_dentry = child_dentry->d_parent;
-   dget(parent_dentry);
+   parent_dentry = dget_parent(child_dentry);
 
/* find out the lower_parent_dentry in the given branch */
lower_parent_dentry =
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index d324f83..4b4d6c9 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -487,13 +487,13 @@ extern int parse_branch_mode(const char *name, int 
*perms);
 /* locking helpers */
 static inline struct dentry *lock_parent(struct dentry *dentry)
 {
-   struct dentry *dir = dget(dentry->d_parent);
+   struct dentry *dir = dget_parent(dentry);
mutex_lock_nested(>d_inode->i_mutex, I_MUTEX_PARENT);
return dir;
 }
 static inline struct dentry *lock_parent_wh(struct dentry *dentry)
 {
-   struct dentry *dir = dget(dentry->d_parent);
+   struct dentry *dir = dget_parent(dentry);
 
mutex_lock_nested(>d_inode->i_mutex, UNIONFS_DMUTEX_WHITEOUT);
return dir;
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] Unionfs: use first writable branch (fix/cleanup)

[Erez Zadok]

Cleanup code in ->create, ->symlink, and ->mknod: refactor common code into
helper functions.  Also, this allows writing to multiple branches again,
which was broken by an earlier patch.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/inode.c |  395 +---
 1 files changed, 156 insertions(+), 239 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index e15ddb9..0b92da2 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -18,14 +18,159 @@
 
 #include "union.h"
 
+/*
+ * Helper function when creating new objects (create, symlink, and mknod).
+ * Checks to see if there's a whiteout in @lower_dentry's parent directory,
+ * whose name is taken from @dentry.  Then tries to remove that whiteout, if
+ * found.
+ *
+ * Return 0 if no whiteout was found, or if one was found and successfully
+ * removed (a zero tells the caller that @lower_dentry belongs to a good
+ * branch to create the new object in).  Return -ERRNO if an error occurred
+ * during whiteout lookup or in trying to unlink the whiteout.
+ */
+static int check_for_whiteout(struct dentry *dentry,
+ struct dentry *lower_dentry)
+{
+   int err = 0;
+   struct dentry *wh_dentry = NULL;
+   struct dentry *lower_dir_dentry;
+   char *name = NULL;
+
+   /*
+* check if whiteout exists in this branch, i.e. lookup .wh.foo
+* first.
+*/
+   name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
+   if (unlikely(IS_ERR(name))) {
+   err = PTR_ERR(name);
+   goto out;
+   }
+
+   wh_dentry = lookup_one_len(name, lower_dentry->d_parent,
+  dentry->d_name.len + UNIONFS_WHLEN);
+   if (IS_ERR(wh_dentry)) {
+   err = PTR_ERR(wh_dentry);
+   wh_dentry = NULL;
+   goto out;
+   }
+
+   if (!wh_dentry->d_inode) /* no whiteout exists */
+   goto out;
+
+   /* .wh.foo has been found, so let's unlink it */
+   lower_dir_dentry = lock_parent_wh(wh_dentry);
+   /* see Documentation/filesystems/unionfs/issues.txt */
+   lockdep_off();
+   err = vfs_unlink(lower_dir_dentry->d_inode, wh_dentry);
+   lockdep_on();
+   unlock_dir(lower_dir_dentry);
+
+   /*
+* Whiteouts are special files and should be deleted no matter what
+* (as if they never existed), in order to allow this create
+* operation to succeed.  This is especially important in sticky
+* directories: a whiteout may have been created by one user, but
+* the newly created file may be created by another user.
+* Therefore, in order to maintain Unix semantics, if the vfs_unlink
+* above failed, then we have to try to directly unlink the
+* whiteout.  Note: in the ODF version of unionfs, whiteout are
+* handled much more cleanly.
+*/
+   if (err == -EPERM) {
+   struct inode *inode = lower_dir_dentry->d_inode;
+   err = inode->i_op->unlink(inode, wh_dentry);
+   }
+   if (err)
+   printk(KERN_ERR "unionfs: could not "
+  "unlink whiteout, err = %d\n", err);
+
+out:
+   dput(wh_dentry);
+   kfree(name);
+   return err;
+}
+
+/*
+ * Find a writeable branch to create new object in.  Checks all writeble
+ * branches of the parent inode, from istart to iend order; if none are
+ * suitable, also tries branch 0 (which may require a copyup).
+ *
+ * Return a lower_dentry we can use to create object in, or ERR_PTR.
+ */
+static struct dentry *find_writeable_branch(struct inode *parent,
+   struct dentry *dentry)
+{
+   int err = -EINVAL;
+   int bindex, istart, iend;
+   struct dentry *lower_dentry = NULL;
+
+   istart = ibstart(parent);
+   iend = ibend(parent);
+   if (istart < 0)
+   goto out;
+
+begin:
+   for (bindex = istart; bindex <= iend; bindex++) {
+   /* skip non-writeable branches */
+   err = is_robranch_super(dentry->d_sb, bindex);
+   if (err) {
+   err = -EROFS;
+   continue;
+   }
+   lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+   if (!lower_dentry)
+   continue;
+   /*
+* check for whiteouts in writeable branch, and remove them
+* if necessary.
+*/
+   err = check_for_whiteout(dentry, lower_dentry);
+   if (err)
+   continue;
+   }
+   /*
+* If istart wasn't already branch 0, and we got any error, then try
+* branch 0 (which may require copyup)
+*/
+   if (e

[GIT PULL -mm] 0/4 Unionfs updates/fixes/cleanups

[Erez Zadok]

The following is a series of patchsets related to Unionfs.  This is the
fifth set of patchsets resulting from an lkml review of the entire unionfs
code base, in preparation for a merge into mainline.  The most significant
changes here are a few locking related fixes, and a correction to broken
logic which didn't allow writing to the first available writable branch.

These patches were tested (where appropriate) on 2.6.24, MM, as well as the
backports to 2.6.{23,22,21,20,19,18,9} on ext2/3/4, xfs, reiserfs, nfs2/3/4,
jffs2, ramfs, tmpfs, cramfs, and squashfs (where available).  Also tested
with LTP-full and with a continuous parallel kernel compile (while forcing
cache flushing, manipulating lower branches, etc.).  See
http://unionfs.filesystems.org/ to download back-ported unionfs code.

Please pull from the 'master' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/ezk/unionfs.git

to receive the following:

Erez Zadok (4):
  Unionfs: use first writable branch (fix/cleanup)
  Unionfs: remove unnecessary call to d_iput
  Unionfs: d_parent related locking fixes
  Unionfs: lock_rename related locking fixes

 copyup.c |   16 --
 inode.c  |  395 ---
 rename.c |   16 ++
 union.h  |4 
 4 files changed, 174 insertions(+), 257 deletions(-)

---
Erez Zadok
[EMAIL PROTECTED]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [UNIONFS] 00/29 Unionfs and related patches pre-merge review (v2)

[Erez Zadok]

In message [EMAIL PROTECTED], Al Viro writes:
 After grep for locking-related things:
 
   * lock_parent(): who said that you won't get dentry moved
 before managing to grab i_mutex on parent?  While we are at it,
 who said that you won't get dentry moved between fetching d_parent
 and doing dget()?  In that case parent could've been _freed_ before
 you get to dget().

OK, so looks like I should use dget_parent() in my lock_parent(), as I've
done elsewhere.  I'll also take a look at all instances in which I get
dentry-d_parent and see if a d_lock is needed there.

   * in create_parents():
 +   struct inode *inode = lower_dentry-d_inode;
 +   /*
 +* If we get here, it means that we created a new
 +* dentry+inode, but copying permissions failed.
 +* Therefore, we should delete this inode and dput
 +* the dentry so as not to leave cruft behind.
 +*/
 +   if (lower_dentry-d_op  lower_dentry-d_op-d_iput)
 +   lower_dentry-d_op-d_iput(lower_dentry,
 +  inode);
 +   else
 +   iput(inode);
 +   lower_dentry-d_inode = NULL;
 +   dput(lower_dentry);
 +   lower_dentry = ERR_PTR(err);
 +   goto out;
 Really?  So what happens if it had become positive after your test and
 somebody had looked it up in lower layer and just now happens to be
 in the middle of operations on it?  Will be thucking frilled by that...

Good catch.  That -d_iput call was an old fix to a bug that has since been
fixed more cleanly and generically in our copyup_permission routine and our
unionfs_d_iput.  I've removed the above -d_iput if and tested to verify
that it's indeed unnecessary.

   * __unionfs_rename():
 +   lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
 +   err = vfs_rename(lower_old_dir_dentry-d_inode, lower_old_dentry,
 +lower_new_dir_dentry-d_inode, lower_new_dentry);
 +   unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
 
 Uh-huh...  To start with, what guarantees that your lower_old_dentry
 is still a child of your lower_old_dir_dentry?

We dget/dget_parent the old/new dentry and parents a few lines above
(actually, it looked like I forgot to dget(lower_new_dentry) -- fixed).
This is a generic stackable f/s issue: ecryptfs does the same stuff before
calling vfs_rename() on the lower objects.

 What's more, you are
 not checking the result of lock_rename(), i.e. asking for serious trouble.

OK.  I'm now checking for the return from lock_rename for ancestor/rename
rules.  I'm CC'ing Mike Halcrow so he can do the same for ecryptfs.

   * revalidation stuff: err...  how the devil can it work for
 directories, when there's nothing to prevent changes in underlying
 layers between -d_revalidate() and operation itself?  For the upper
 layer (unionfs itself) everything's more or less fine, but the rest
 of that...

In a stacked f/s, we keep references to the lower dentries/inodes, so they
can't disappear on us (that happens in our interpose function, called from
our -lookup).  On entry to every f/s method in unionfs, we first perform
lightweight revalidation of our dentry against the lower ones: we check if
m/ctime changed (users modifying lower files) or if the generation# b/t our
super and the our dentries have changed (branch-management took place); if
needed, then we perform a full revalidation of all lower objects (while
holding a lock on the branch configuration).  If we have to do a full reval
upon entry to our -op, and the reval failed, then we return an appropriate
error; o/w we proceed.  (In certain cases, the VFS re-issues a lookup if the
f/s says that it's dentry is invalid.)

Without changes to the VFS, I don't see how else I can ensure cache
coherency cleanly, while allowing users to modify lower files; this feature
is very useful to some unionfs users, who depend on it (so even if I could
lock out the lower directories from being modified, there will be users
who'd still want to be able to modify lower files).

BTW, my sense of the relationship b/t upper and lower objects and their
validity in a stackable f/s, is that it's similar to the relationship b/t
the NFS client and server -- the client can't be sure that a file on the
server doesn't change b/t -revalidate and -op (hence nfs's reliance on dir
mtime checks).

Perhaps this general topic is a good one to discuss at more length at LSF?
Suggestions are welcome.

Thanks,
Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] Unionfs: lock_rename related locking fixes

[Erez Zadok]

CC: Mike Halcrow [EMAIL PROTECTED]

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/rename.c |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
index 9306a2b..5ab13f9 100644
--- a/fs/unionfs/rename.c
+++ b/fs/unionfs/rename.c
@@ -29,6 +29,7 @@ static int __unionfs_rename(struct inode *old_dir, struct 
dentry *old_dentry,
struct dentry *lower_new_dir_dentry;
struct dentry *lower_wh_dentry;
struct dentry *lower_wh_dir_dentry;
+   struct dentry *trap;
char *wh_name = NULL;
 
lower_new_dentry = unionfs_lower_dentry_idx(new_dentry, bindex);
@@ -95,6 +96,7 @@ static int __unionfs_rename(struct inode *old_dir, struct 
dentry *old_dentry,
goto out;
 
dget(lower_old_dentry);
+   dget(lower_new_dentry);
lower_old_dir_dentry = dget_parent(lower_old_dentry);
lower_new_dir_dentry = dget_parent(lower_new_dentry);
 
@@ -122,9 +124,20 @@ static int __unionfs_rename(struct inode *old_dir, struct 
dentry *old_dentry,
 
/* see Documentation/filesystems/unionfs/issues.txt */
lockdep_off();
-   lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
+   trap = lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
+   /* source should not be ancenstor of target */
+   if (trap == lower_old_dentry) {
+   err = -EINVAL;
+   goto out_err_unlock;
+   }
+   /* target should not be ancenstor of source */
+   if (trap == lower_new_dentry) {
+   err = -ENOTEMPTY;
+   goto out_err_unlock;
+   }
err = vfs_rename(lower_old_dir_dentry-d_inode, lower_old_dentry,
 lower_new_dir_dentry-d_inode, lower_new_dentry);
+out_err_unlock:
unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
lockdep_on();
 
@@ -132,6 +145,7 @@ out_dput:
dput(lower_old_dir_dentry);
dput(lower_new_dir_dentry);
dput(lower_old_dentry);
+   dput(lower_new_dentry);
 
 out:
if (!err) {
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/4] Unionfs: d_parent related locking fixes

[Erez Zadok]

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/copyup.c |3 +--
 fs/unionfs/union.h  |4 ++--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 8663224..9beac01 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -716,8 +716,7 @@ struct dentry *create_parents(struct inode *dir, struct 
dentry *dentry,
child_dentry = parent_dentry;
 
/* find the parent directory dentry in unionfs */
-   parent_dentry = child_dentry-d_parent;
-   dget(parent_dentry);
+   parent_dentry = dget_parent(child_dentry);
 
/* find out the lower_parent_dentry in the given branch */
lower_parent_dentry =
diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
index d324f83..4b4d6c9 100644
--- a/fs/unionfs/union.h
+++ b/fs/unionfs/union.h
@@ -487,13 +487,13 @@ extern int parse_branch_mode(const char *name, int 
*perms);
 /* locking helpers */
 static inline struct dentry *lock_parent(struct dentry *dentry)
 {
-   struct dentry *dir = dget(dentry-d_parent);
+   struct dentry *dir = dget_parent(dentry);
mutex_lock_nested(dir-d_inode-i_mutex, I_MUTEX_PARENT);
return dir;
 }
 static inline struct dentry *lock_parent_wh(struct dentry *dentry)
 {
-   struct dentry *dir = dget(dentry-d_parent);
+   struct dentry *dir = dget_parent(dentry);
 
mutex_lock_nested(dir-d_inode-i_mutex, UNIONFS_DMUTEX_WHITEOUT);
return dir;
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL -mm] 0/4 Unionfs updates/fixes/cleanups

[Erez Zadok]

The following is a series of patchsets related to Unionfs.  This is the
fifth set of patchsets resulting from an lkml review of the entire unionfs
code base, in preparation for a merge into mainline.  The most significant
changes here are a few locking related fixes, and a correction to broken
logic which didn't allow writing to the first available writable branch.

These patches were tested (where appropriate) on 2.6.24, MM, as well as the
backports to 2.6.{23,22,21,20,19,18,9} on ext2/3/4, xfs, reiserfs, nfs2/3/4,
jffs2, ramfs, tmpfs, cramfs, and squashfs (where available).  Also tested
with LTP-full and with a continuous parallel kernel compile (while forcing
cache flushing, manipulating lower branches, etc.).  See
http://unionfs.filesystems.org/ to download back-ported unionfs code.

Please pull from the 'master' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/ezk/unionfs.git

to receive the following:

Erez Zadok (4):
  Unionfs: use first writable branch (fix/cleanup)
  Unionfs: remove unnecessary call to d_iput
  Unionfs: d_parent related locking fixes
  Unionfs: lock_rename related locking fixes

 copyup.c |   16 --
 inode.c  |  395 ---
 rename.c |   16 ++
 union.h  |4 
 4 files changed, 174 insertions(+), 257 deletions(-)

---
Erez Zadok
[EMAIL PROTECTED]
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/4] Unionfs: remove unnecessary call to d_iput

[Erez Zadok]

This old code was to fix a bug which has long since been fixed in our
copyup_permission and unionfs_d_iput.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/copyup.c |   13 -
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/fs/unionfs/copyup.c b/fs/unionfs/copyup.c
index 16b2c7c..8663224 100644
--- a/fs/unionfs/copyup.c
+++ b/fs/unionfs/copyup.c
@@ -807,19 +807,6 @@ begin:
 lower_dentry);
unlock_dir(lower_parent_dentry);
if (err) {
-   struct inode *inode = lower_dentry-d_inode;
-   /*
-* If we get here, it means that we created a new
-* dentry+inode, but copying permissions failed.
-* Therefore, we should delete this inode and dput
-* the dentry so as not to leave cruft behind.
-*/
-   if (lower_dentry-d_op  lower_dentry-d_op-d_iput)
-   lower_dentry-d_op-d_iput(lower_dentry,
-  inode);
-   else
-   iput(inode);
-   lower_dentry-d_inode = NULL;
dput(lower_dentry);
lower_dentry = ERR_PTR(err);
goto out;
-- 
1.5.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] Unionfs: use first writable branch (fix/cleanup)

[Erez Zadok]

Cleanup code in -create, -symlink, and -mknod: refactor common code into
helper functions.  Also, this allows writing to multiple branches again,
which was broken by an earlier patch.

Signed-off-by: Erez Zadok [EMAIL PROTECTED]
---
 fs/unionfs/inode.c |  395 +---
 1 files changed, 156 insertions(+), 239 deletions(-)

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index e15ddb9..0b92da2 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -18,14 +18,159 @@
 
 #include union.h
 
+/*
+ * Helper function when creating new objects (create, symlink, and mknod).
+ * Checks to see if there's a whiteout in @lower_dentry's parent directory,
+ * whose name is taken from @dentry.  Then tries to remove that whiteout, if
+ * found.
+ *
+ * Return 0 if no whiteout was found, or if one was found and successfully
+ * removed (a zero tells the caller that @lower_dentry belongs to a good
+ * branch to create the new object in).  Return -ERRNO if an error occurred
+ * during whiteout lookup or in trying to unlink the whiteout.
+ */
+static int check_for_whiteout(struct dentry *dentry,
+ struct dentry *lower_dentry)
+{
+   int err = 0;
+   struct dentry *wh_dentry = NULL;
+   struct dentry *lower_dir_dentry;
+   char *name = NULL;
+
+   /*
+* check if whiteout exists in this branch, i.e. lookup .wh.foo
+* first.
+*/
+   name = alloc_whname(dentry-d_name.name, dentry-d_name.len);
+   if (unlikely(IS_ERR(name))) {
+   err = PTR_ERR(name);
+   goto out;
+   }
+
+   wh_dentry = lookup_one_len(name, lower_dentry-d_parent,
+  dentry-d_name.len + UNIONFS_WHLEN);
+   if (IS_ERR(wh_dentry)) {
+   err = PTR_ERR(wh_dentry);
+   wh_dentry = NULL;
+   goto out;
+   }
+
+   if (!wh_dentry-d_inode) /* no whiteout exists */
+   goto out;
+
+   /* .wh.foo has been found, so let's unlink it */
+   lower_dir_dentry = lock_parent_wh(wh_dentry);
+   /* see Documentation/filesystems/unionfs/issues.txt */
+   lockdep_off();
+   err = vfs_unlink(lower_dir_dentry-d_inode, wh_dentry);
+   lockdep_on();
+   unlock_dir(lower_dir_dentry);
+
+   /*
+* Whiteouts are special files and should be deleted no matter what
+* (as if they never existed), in order to allow this create
+* operation to succeed.  This is especially important in sticky
+* directories: a whiteout may have been created by one user, but
+* the newly created file may be created by another user.
+* Therefore, in order to maintain Unix semantics, if the vfs_unlink
+* above failed, then we have to try to directly unlink the
+* whiteout.  Note: in the ODF version of unionfs, whiteout are
+* handled much more cleanly.
+*/
+   if (err == -EPERM) {
+   struct inode *inode = lower_dir_dentry-d_inode;
+   err = inode-i_op-unlink(inode, wh_dentry);
+   }
+   if (err)
+   printk(KERN_ERR unionfs: could not 
+  unlink whiteout, err = %d\n, err);
+
+out:
+   dput(wh_dentry);
+   kfree(name);
+   return err;
+}
+
+/*
+ * Find a writeable branch to create new object in.  Checks all writeble
+ * branches of the parent inode, from istart to iend order; if none are
+ * suitable, also tries branch 0 (which may require a copyup).
+ *
+ * Return a lower_dentry we can use to create object in, or ERR_PTR.
+ */
+static struct dentry *find_writeable_branch(struct inode *parent,
+   struct dentry *dentry)
+{
+   int err = -EINVAL;
+   int bindex, istart, iend;
+   struct dentry *lower_dentry = NULL;
+
+   istart = ibstart(parent);
+   iend = ibend(parent);
+   if (istart  0)
+   goto out;
+
+begin:
+   for (bindex = istart; bindex = iend; bindex++) {
+   /* skip non-writeable branches */
+   err = is_robranch_super(dentry-d_sb, bindex);
+   if (err) {
+   err = -EROFS;
+   continue;
+   }
+   lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+   if (!lower_dentry)
+   continue;
+   /*
+* check for whiteouts in writeable branch, and remove them
+* if necessary.
+*/
+   err = check_for_whiteout(dentry, lower_dentry);
+   if (err)
+   continue;
+   }
+   /*
+* If istart wasn't already branch 0, and we got any error, then try
+* branch 0 (which may require copyup)
+*/
+   if (err  istart  0) {
+   istart = iend = 0;
+   goto begin;
+   }
+
+   /*
+* If we tried even branch 0

Re: [patch 01/26] mount options: add documentation

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Miklos Szeredi writes:
> From: Miklos Szeredi <[EMAIL PROTECTED]>
> 
> This series addresses the problem of showing mount options in
> /proc/mounts.
> 
> Several filesystems which use mount options, have not implemented a
> .show_options superblock operation.  Several others have implemented
> this callback, but have not kept it fully up to date with the parsed
> options.
[...]

> The following filesystems still need fixing: CIFS, NFS, XFS, Unionfs,
> Reiser4.  For CIFS, NFS and XFS I wasn't able to understand how some
> of the options are used.  The last two are not yet in mainline, so I
> leave fixing those to their respective maintainers out of pure
> laziness.
> 
> Table displaying status of all in-kernel filesystems:
> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> legend:
> 
>   none - fs has options, but doesn't define ->show_options()
>   some - fs defines ->show_options(), but some only options are shown
>   most - fs defines ->show_options(), and shows most of them
>   good - fs shows all options
>   noopt - fs does not have options
>   patch - a patch will be posted
[...]

> in -mm:
> 
> reiser4 some
> unionfs none

Hi Miklos,

Where did you check for the existence of a ->show_options method for
unionfs?  Unionfs does implement ->show_options and supports all of the
mount/remount options.  See:



The unionfs ->remount code supports branch-management options which can
add/del/change a branch, but we don't show those directly in ->show_options;
it makes more sense to show the final (and thus most current) branch
configuration.

Could you update your records please?

BTW, I should be able to use your save_mount_options().

Cheers,
Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Fix procfs task exe symlink

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Jan Engelhardt writes:
> 
> On Jan 23 2008 10:29, Matt Helsley wrote:
> >
> >For executables on the stackable MVFS filesystem the current procfs
> >methods for implementing a task's exe symlink do not point to the
> >correct file and applications relying on the symlink fail (see the
> >java example below).
> 
> This reminds me of unoionfs - it also had this issue, but IIRC,
> it eventually got solved. (Should now display /union/exe rather
> than /lowerlevel/exe.) Unionfs developers should know more.

Unionfs resolved this by fully implementing the address_space ops, as well
as ->mmap (i.e., one cannot "cheat" and inherit the lower address_space).

Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 01/26] mount options: add documentation

[Erez Zadok]

In message [EMAIL PROTECTED], Miklos Szeredi writes:
 From: Miklos Szeredi [EMAIL PROTECTED]
 
 This series addresses the problem of showing mount options in
 /proc/mounts.
 
 Several filesystems which use mount options, have not implemented a
 .show_options superblock operation.  Several others have implemented
 this callback, but have not kept it fully up to date with the parsed
 options.
[...]

 The following filesystems still need fixing: CIFS, NFS, XFS, Unionfs,
 Reiser4.  For CIFS, NFS and XFS I wasn't able to understand how some
 of the options are used.  The last two are not yet in mainline, so I
 leave fixing those to their respective maintainers out of pure
 laziness.
 
 Table displaying status of all in-kernel filesystems:
 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 legend:
 
   none - fs has options, but doesn't define -show_options()
   some - fs defines -show_options(), but some only options are shown
   most - fs defines -show_options(), and shows most of them
   good - fs shows all options
   noopt - fs does not have options
   patch - a patch will be posted
[...]

 in -mm:
 
 reiser4 some
 unionfs none

Hi Miklos,

Where did you check for the existence of a -show_options method for
unionfs?  Unionfs does implement -show_options and supports all of the
mount/remount options.  See:

http://git.kernel.org/?p=linux/kernel/git/ezk/unionfs.git;a=blob;f=fs/unionfs/super.c;h=986c980261a5b171147d66ac05bf08423e2fd6b6;hb=HEAD#l963

The unionfs -remount code supports branch-management options which can
add/del/change a branch, but we don't show those directly in -show_options;
it makes more sense to show the final (and thus most current) branch
configuration.

Could you update your records please?

BTW, I should be able to use your save_mount_options().

Cheers,
Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] block2mtd lockdep_init_map warning

[Erez Zadok]

In message <[EMAIL PROTECTED]>, =?utf-8?B?SsO2cm4=?= Engel writes:
[...]
> Patch didn't compile due to function ordering.  Here is an updated version.

Joern/Peter, I've tested this updated patch with v2.6.24-rc8-74-ga7da60f.
It worked fine for me.

Thanks,
Erez.

> Acked-and-tested-by: Joern Engel <[EMAIL PROTECTED]>
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index c2e3e2e..0397b1e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1645,6 +1645,17 @@ static inline void add_kallsyms(struct module *mod,
>  }
>  #endif /* CONFIG_KALLSYMS */
>  
> +/*
> + * link the module with the whole machine is stopped with interrupts off
> + * - this defends against kallsyms not taking locks
> + */
> +static int __link_module(void *_mod)
> +{
> + struct module *mod = _mod;
> + list_add(>list, );
> + return 0;
> +}
> +
>  /* Allocate and load the module: note that size of section 0 is always
> zero, and we rely on this for optional sections. */
>  static struct module *load_module(void __user *umod,
> @@ -2023,6 +2034,11 @@ static struct module *load_module(void __user *umod,
>   printk(KERN_WARNING "%s: Ignoring obsolete parameters\n",
>  mod->name);
>  
> + /* Now sew it into the lists so we can get lockdep and oops
> + * info during argument parsing.  Noone should access us, since
> + * strong_try_module_get() will fail. */
> + stop_machine_run(__link_module, mod, NR_CPUS);
> +
>   /* Size of section 0 is 0, so this works well if no params */
>   err = parse_args(mod->name, mod->args,
>(struct kernel_param *)
> @@ -2031,7 +2047,7 @@ static struct module *load_module(void __user *umod,
>/ sizeof(struct kernel_param),
>NULL);
>   if (err < 0)
> - goto arch_cleanup;
> + goto unlink;
>  
>   err = mod_sysfs_setup(mod,
> (struct kernel_param *)
> @@ -2039,7 +2055,7 @@ static struct module *load_module(void __user *umod,
> sechdrs[setupindex].sh_size
> / sizeof(struct kernel_param));
>   if (err < 0)
> - goto arch_cleanup;
> + goto unlink;
>   add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
>   add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
>  
> @@ -2054,7 +2070,8 @@ static struct module *load_module(void __user *umod,
>   /* Done! */
>   return mod;
>  
> - arch_cleanup:
> + unlink:
> + stop_machine_run(__unlink_module, mod, NR_CPUS);
>   module_arch_cleanup(mod);
>   cleanup:
>   module_unload_free(mod);
> @@ -2076,17 +2093,6 @@ static struct module *load_module(void __user *umod,
>   goto free_hdr;
>  }
>  
> -/*
> - * link the module with the whole machine is stopped with interrupts off
> - * - this defends against kallsyms not taking locks
> - */
> -static int __link_module(void *_mod)
> -{
> - struct module *mod = _mod;
> - list_add(>list, );
> - return 0;
> -}
> -
>  /* This is where the real work happens */
>  asmlinkage long
>  sys_init_module(void __user *umod,
> @@ -2111,10 +2117,6 @@ sys_init_module(void __user *umod,
>   return PTR_ERR(mod);
>   }
>  
> - /* Now sew it into the lists.  They won't access us, since
> -   strong_try_module_get() will fail. */
> - stop_machine_run(__link_module, mod, NR_CPUS);
> -
>   /* Drop lock so they can recurse */
>   mutex_unlock(_mutex);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [UNIONFS] 00/29 Unionfs and related patches pre-merge review (v2)

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Al Viro writes:
> After grep for locking-related things:
[...]

Thanks.  I'll start looking at these issues asap.

Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [UNIONFS] 00/29 Unionfs and related patches pre-merge review (v2)

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Michael Halcrow writes:
> On Thu, Jan 10, 2008 at 10:57:46AM -0500, Erez Zadok wrote:
[...]
> Would the inclusion of Unionfs in mainline really slow down or damage
> the union mount effort? If not, then I think the pragmatic approach
> would be to make it available in mainline for all of the users who are
> already successfully running it today. We can then focus future
> efforts on the VFS-level modifications that address the remaining
> issues, limiting Unionfs in the future to only those problems that are
> best solved in a stacked filesystem layer.

Mike, this is indeed the pragmatic approach I've advocated: as the VFS would
come up with more unioning-related functionality, I could easily make use of
it in unionfs, thus shrinking the code base in unionfs (while keeping the
user API unchanged).  In the end, what'll be left over is probably a smaller
standalone file system that offers the kind of features that aren't likely
to show up at the VFS level (e.g., a persistent cache of unified dir
contents, persistent inode numbers, whiteouts that work with any "obscure"
filesystem, and such).

> Mike

Cheers,
Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: unionfs, cow, and whiteout

[Erez Zadok]

[I recommend we direct future discussions in this thread to the unionfs
ML. -ezk]

In message <[EMAIL PROTECTED]>, Paul Albrecht writes:
[...]
> I'm not sure we're talking about the same problem. What I do is union
> mount a write enabled file system like tmpfs over a read only file
> system like squashfs.
> 
> There's no way to create, modify, or delete files in a squashed file
> system; they can be copied up when they're modified; or, they can be
> whited out when they're deleted.
> 
> Whenever a file is created in the union mount, it necessarily gets
> created in tmpfs. When that file gets deleted, it gets whited out which
> doesn't make sense because it doesn't exist in the other layer.
> 
> This is a problem because over time as files are created, modified, and
> deleted whiteout cruft accumulates in the cow layer of the union mount.
> 
> Fixing the problem doesn't seem that complex and shouldn't require
> searching all the layers of the union mount.

Paul, you're looking into a specific 2-branch configuration where one branch
is r-o and the other is r-w.  Yes, in that specific case, one could argue
that a whiteout isn't needed.  But what if I have N branches, with a mix of
rw/ro branches, where a file or its whiteout could exist in any branch?  If
I don't create a whiteout, then I have to scan all N branches and remove the
same file from there (assuming the file doesn't exist on a r-o branch --
then I have to abort).

Note also that branches could be dynamically marked r-o or r-w over the
lifetime of the union: so a file which was deletable before may not be
deletable in the future.

We used to have several modes of operations, including one called
DELETE_ALL, which was similar to what you're asking for.  But it complicated
the code considerably and most users didn't use that mode.  So we opted for
simplicity and clarity of code, rather than having special cases for
different branch configurations.

If you're willing to open a feature-request report on
https://bugzilla.filesystems.org/, then we'll be happy to consider your
request and see how it can be incorporated while keeping the base code
devoid of special cases.  Thanks.

> If the union file system simply took note of whether a file was created
> in the cow layer because it's new or because it's been modified and
> copied up from the read only file system, then it would simply delete
> the file in the former case and and use whiteout in the latter.

Taking that "note" requires that the information survives a reboot; so I
can't store it in memory, but it has to be stored persistently.  That would
complicate the code and one might as well use unionfs-odf instead.

> > Another possible problem is that if you choose to insert a new branch in
> > the middle, and you didn't have the whiteout, you may re-expose the file
> > name unintentionally.
> > 
> 
> I don't see how the a "deleted" file in a read only file system could be
> re-exposed unless its whiteout in the cow layer was deleted, but that's
> really not the issue.

Suppose you have your two branches, you created a file X and deleted it.
Now, you insert a new branch in the *middle*, which has file X in it: do you
want that new file to show up in /bin/ls, or not?  If you didn't create a
whiteout in the /cow layer, then file X will re-appear after the user
supposedly deleted it.  (To be fair, the desired semantics here are not
clear -- some users may want it one way or another -- but I want to ensure a
*consistent* semantics that is simple to understand).

> What I'm objecting to is creating the whiteout in the cow layer when the
> file didn't get there via a copy up from a read only file system. In
> this case there's no worry about re-exposing the deleted file because
> it's really deleted.

Paul, it really looks to me that you'd prefer the unionfs-odf version: it
has a flavor of the older delete-all mode.  In unionfs-odf, we first try to
delete the file from all branches.  If we can't (b/c of r-o branches/media),
then we create a whiteout in the (small) /odf partition.  Therefore,
whiteouts are never stored in the main union'ed branches.

Cheers,
Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: unionfs, cow, and whiteout

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Paul Albrecht writes:
> Hi,
> 
> I have a question about how unionfs handles file deletion when a write
> enabled file system is union mounted over a read only file system. For
> example, I do something like the following:
> 
> mount -t unionfs -o dirs=/cow=rw:/rofs=ro unionfs /mnt
> 
> If I create and delete a file in /mnt which is not present in /rofs it
> persists as whiteout in the cow file system which is not what I would
> have expected.

Paul, the alternative is to scan all branches (there could be many) to
ensure that the file may not exist by that name anywhere else, and if so,
try to delete all instances of it.  This was deemed too expensive and
complex.

Another possible problem is that if you choose to insert a new branch in the
middle, and you didn't have the whiteout, you may re-expose the file name
unintentionally.

You might want to take a look at our unionfs-odf version: it places
whiteouts in a separate persistent store outside the branches, not as .wh.*
files in individual branches.

> Why does the deleted file persist as whiteout in the /cow file system of
> the union mount?
> 
> Please cc me in your response as I'm not subscribed to the lkml.
> 
> -- 
> 
> Paul Albrecht

Cheers,
Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: unionfs, cow, and whiteout

[Erez Zadok]

In message [EMAIL PROTECTED], Paul Albrecht writes:
 Hi,
 
 I have a question about how unionfs handles file deletion when a write
 enabled file system is union mounted over a read only file system. For
 example, I do something like the following:
 
 mount -t unionfs -o dirs=/cow=rw:/rofs=ro unionfs /mnt
 
 If I create and delete a file in /mnt which is not present in /rofs it
 persists as whiteout in the cow file system which is not what I would
 have expected.

Paul, the alternative is to scan all branches (there could be many) to
ensure that the file may not exist by that name anywhere else, and if so,
try to delete all instances of it.  This was deemed too expensive and
complex.

Another possible problem is that if you choose to insert a new branch in the
middle, and you didn't have the whiteout, you may re-expose the file name
unintentionally.

You might want to take a look at our unionfs-odf version: it places
whiteouts in a separate persistent store outside the branches, not as .wh.*
files in individual branches.

 Why does the deleted file persist as whiteout in the /cow file system of
 the union mount?
 
 Please cc me in your response as I'm not subscribed to the lkml.
 
 -- 
 
 Paul Albrecht

Cheers,
Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: unionfs, cow, and whiteout

[Erez Zadok]

[I recommend we direct future discussions in this thread to the unionfs
ML. -ezk]

In message [EMAIL PROTECTED], Paul Albrecht writes:
[...]
 I'm not sure we're talking about the same problem. What I do is union
 mount a write enabled file system like tmpfs over a read only file
 system like squashfs.
 
 There's no way to create, modify, or delete files in a squashed file
 system; they can be copied up when they're modified; or, they can be
 whited out when they're deleted.
 
 Whenever a file is created in the union mount, it necessarily gets
 created in tmpfs. When that file gets deleted, it gets whited out which
 doesn't make sense because it doesn't exist in the other layer.
 
 This is a problem because over time as files are created, modified, and
 deleted whiteout cruft accumulates in the cow layer of the union mount.
 
 Fixing the problem doesn't seem that complex and shouldn't require
 searching all the layers of the union mount.

Paul, you're looking into a specific 2-branch configuration where one branch
is r-o and the other is r-w.  Yes, in that specific case, one could argue
that a whiteout isn't needed.  But what if I have N branches, with a mix of
rw/ro branches, where a file or its whiteout could exist in any branch?  If
I don't create a whiteout, then I have to scan all N branches and remove the
same file from there (assuming the file doesn't exist on a r-o branch --
then I have to abort).

Note also that branches could be dynamically marked r-o or r-w over the
lifetime of the union: so a file which was deletable before may not be
deletable in the future.

We used to have several modes of operations, including one called
DELETE_ALL, which was similar to what you're asking for.  But it complicated
the code considerably and most users didn't use that mode.  So we opted for
simplicity and clarity of code, rather than having special cases for
different branch configurations.

If you're willing to open a feature-request report on
https://bugzilla.filesystems.org/, then we'll be happy to consider your
request and see how it can be incorporated while keeping the base code
devoid of special cases.  Thanks.

 If the union file system simply took note of whether a file was created
 in the cow layer because it's new or because it's been modified and
 copied up from the read only file system, then it would simply delete
 the file in the former case and and use whiteout in the latter.

Taking that note requires that the information survives a reboot; so I
can't store it in memory, but it has to be stored persistently.  That would
complicate the code and one might as well use unionfs-odf instead.

  Another possible problem is that if you choose to insert a new branch in
  the middle, and you didn't have the whiteout, you may re-expose the file
  name unintentionally.
  
 
 I don't see how the a deleted file in a read only file system could be
 re-exposed unless its whiteout in the cow layer was deleted, but that's
 really not the issue.

Suppose you have your two branches, you created a file X and deleted it.
Now, you insert a new branch in the *middle*, which has file X in it: do you
want that new file to show up in /bin/ls, or not?  If you didn't create a
whiteout in the /cow layer, then file X will re-appear after the user
supposedly deleted it.  (To be fair, the desired semantics here are not
clear -- some users may want it one way or another -- but I want to ensure a
*consistent* semantics that is simple to understand).

 What I'm objecting to is creating the whiteout in the cow layer when the
 file didn't get there via a copy up from a read only file system. In
 this case there's no worry about re-exposing the deleted file because
 it's really deleted.

Paul, it really looks to me that you'd prefer the unionfs-odf version: it
has a flavor of the older delete-all mode.  In unionfs-odf, we first try to
delete the file from all branches.  If we can't (b/c of r-o branches/media),
then we create a whiteout in the (small) /odf partition.  Therefore,
whiteouts are never stored in the main union'ed branches.

Cheers,
Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [UNIONFS] 00/29 Unionfs and related patches pre-merge review (v2)

[Erez Zadok]

In message [EMAIL PROTECTED], Michael Halcrow writes:
 On Thu, Jan 10, 2008 at 10:57:46AM -0500, Erez Zadok wrote:
[...]
 Would the inclusion of Unionfs in mainline really slow down or damage
 the union mount effort? If not, then I think the pragmatic approach
 would be to make it available in mainline for all of the users who are
 already successfully running it today. We can then focus future
 efforts on the VFS-level modifications that address the remaining
 issues, limiting Unionfs in the future to only those problems that are
 best solved in a stacked filesystem layer.

Mike, this is indeed the pragmatic approach I've advocated: as the VFS would
come up with more unioning-related functionality, I could easily make use of
it in unionfs, thus shrinking the code base in unionfs (while keeping the
user API unchanged).  In the end, what'll be left over is probably a smaller
standalone file system that offers the kind of features that aren't likely
to show up at the VFS level (e.g., a persistent cache of unified dir
contents, persistent inode numbers, whiteouts that work with any obscure
filesystem, and such).

 Mike

Cheers,
Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [UNIONFS] 00/29 Unionfs and related patches pre-merge review (v2)

[Erez Zadok]

In message [EMAIL PROTECTED], Al Viro writes:
 After grep for locking-related things:
[...]

Thanks.  I'll start looking at these issues asap.

Erez.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [UNIONFS] 00/29 Unionfs and related patches pre-merge review (v2)

[Erez Zadok]

In message <[EMAIL PROTECTED]>, Christoph Hellwig writes:
> On Thu, Jan 10, 2008 at 09:59:19AM -0500, Erez Zadok wrote:
> > 
> > Dear Linus, Al, Christoph, and Andrew,
> > 
> > As per your request, I'm posting for review the unionfs code (and related
> > code) that's in my korg tree against mainline (v2.6.24-rc7-71-gfd0b45d).
> > This is in preparation for merge in 2.6.25.
> 
> Huh?  There's still aboslutely not fix to the underlying problems of
> the whole idea.   I think we made it pretty clear that unionfs is not
> the way to go, and that we'll get the union mount patches clear once
> the per-mountpoint r/o and unprivilegued mount patches series are in
> and stable.

I'll reiterate what I've said before: unionfs is used today by many users,
it works, and is stable.  After years of working with unionfs, we've settled
on a set of features that users actually use.  This functionality can be in
mainline today.

Unioning at the VFS level, will take a long time to reach the same level of
maturity and support the same set of features.  Based on my years of
practical experience with it, unioning directories seems like a simple idea,
but in practice it's quite hard no matter the approach taken to implement
it.

Existing users of unioning aren't likely to switch to Union Mounts unless it
supports the same set of features.  How long will it realistically take to
get whiteout support in every lower file system that's used by Unionfs
users?  How will Union Mounts support persistent inode numbers at the VFS
level?  Those are just a few of the questions.

I think a better approach would be to start with Unionfs (a standalone file
system that doesn't touch the rest of the kernel).  And as Linux gradually
starts supporting more and more features that help unioning/stacking in
general, to change Unionfs to use those features (e.g., native whiteout
support).  Eventually there could be basic unioning support at the VFS
level, and concurrently a file-system which offers the extra features (e.g.,
persistency).  This can be done w/o affecting user-visible APIs.

Cheers,
Erez.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 20/29] Unionfs: super_block operations

[Erez Zadok]

Includes read_inode, delete_inode, put_super, statfs, remount_fs (which
supports branch-management ops), clear_inode, alloc_inode, destroy_inode,
write_inode, umount_begin, and show_options.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/super.c | 1025 
 1 files changed, 1025 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/super.c

diff --git a/fs/unionfs/super.c b/fs/unionfs/super.c
new file mode 100644
index 000..986c980
--- /dev/null
+++ b/fs/unionfs/super.c
@@ -0,0 +1,1025 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+/*
+ * The inode cache is used with alloc_inode for both our inode info and the
+ * vfs inode.
+ */
+static struct kmem_cache *unionfs_inode_cachep;
+
+static void unionfs_read_inode(struct inode *inode)
+{
+   int size;
+   struct unionfs_inode_info *info = UNIONFS_I(inode);
+
+   memset(info, 0, offsetof(struct unionfs_inode_info, vfs_inode));
+   info->bstart = -1;
+   info->bend = -1;
+   atomic_set(>generation,
+  atomic_read(_SB(inode->i_sb)->generation));
+   spin_lock_init(>rdlock);
+   info->rdcount = 1;
+   info->hashsize = -1;
+   INIT_LIST_HEAD(>readdircache);
+
+   size = sbmax(inode->i_sb) * sizeof(struct inode *);
+   info->lower_inodes = kzalloc(size, GFP_KERNEL);
+   if (unlikely(!info->lower_inodes)) {
+   printk(KERN_CRIT "unionfs: no kernel memory when allocating "
+  "lower-pointer array!\n");
+   BUG();
+   }
+
+   inode->i_version++;
+   inode->i_op = _main_iops;
+   inode->i_fop = _main_fops;
+
+   inode->i_mapping->a_ops = _aops;
+
+   /*
+* reset times so unionfs_copy_attr_all can keep out time invariants
+* right (upper inode time being the max of all lower ones).
+*/
+   inode->i_atime.tv_sec = inode->i_atime.tv_nsec = 0;
+   inode->i_mtime.tv_sec = inode->i_mtime.tv_nsec = 0;
+   inode->i_ctime.tv_sec = inode->i_ctime.tv_nsec = 0;
+
+}
+
+/*
+ * we now define delete_inode, because there are two VFS paths that may
+ * destroy an inode: one of them calls clear inode before doing everything
+ * else that's needed, and the other is fine.  This way we truncate the inode
+ * size (and its pages) and then clear our own inode, which will do an iput
+ * on our and the lower inode.
+ *
+ * No need to lock sb info's rwsem.
+ */
+static void unionfs_delete_inode(struct inode *inode)
+{
+#if BITS_PER_LONG == 32 && defined(CONFIG_SMP)
+   spin_lock(>i_lock);
+#endif
+   i_size_write(inode, 0); /* every f/s seems to do that */
+#if BITS_PER_LONG == 32 && defined(CONFIG_SMP)
+   spin_unlock(>i_lock);
+#endif
+
+   if (inode->i_data.nrpages)
+   truncate_inode_pages(>i_data, 0);
+
+   clear_inode(inode);
+}
+
+/*
+ * final actions when unmounting a file system
+ *
+ * No need to lock rwsem.
+ */
+static void unionfs_put_super(struct super_block *sb)
+{
+   int bindex, bstart, bend;
+   struct unionfs_sb_info *spd;
+   int leaks = 0;
+
+   spd = UNIONFS_SB(sb);
+   if (!spd)
+   return;
+
+   bstart = sbstart(sb);
+   bend = sbend(sb);
+
+   /* Make sure we have no leaks of branchget/branchput. */
+   for (bindex = bstart; bindex <= bend; bindex++)
+   if (unlikely(branch_count(sb, bindex) != 0)) {
+   printk(KERN_CRIT
+  "unionfs: branch %d has %d references left!\n",
+  bindex, branch_count(sb, bindex));
+   leaks = 1;
+   }
+   BUG_ON(leaks != 0);
+
+   kfree(spd->data);
+   kfree(spd);
+   sb->s_fs_info = NULL;
+}
+
+/*
+ * Since people use this to answer the "How big of a file can I write?"
+ * question, we report the size of the highest priority branch as the size of
+ * the union.
+ */
+static int unionfs_statfs(struct dentry *dentry, struct kstatfs *buf)
+{
+   int err = 0;
+   struct super_block *sb;
+   struct dentry *lower_dentry;
+
+

[PATCH 11/29] Unionfs: lower-level lookup routines

[Erez Zadok]

Includes lower nameidata support routines.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/lookup.c |  652 +++
 1 files changed, 652 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/lookup.c

diff --git a/fs/unionfs/lookup.c b/fs/unionfs/lookup.c
new file mode 100644
index 000..b9ee072
--- /dev/null
+++ b/fs/unionfs/lookup.c
@@ -0,0 +1,652 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+static int realloc_dentry_private_data(struct dentry *dentry);
+
+/* is the filename valid == !(whiteout for a file or opaque dir marker) */
+static int is_validname(const char *name)
+{
+   if (!strncmp(name, UNIONFS_WHPFX, UNIONFS_WHLEN))
+   return 0;
+   if (!strncmp(name, UNIONFS_DIR_OPAQUE_NAME,
+sizeof(UNIONFS_DIR_OPAQUE_NAME) - 1))
+   return 0;
+   return 1;
+}
+
+/* The rest of these are utility functions for lookup. */
+static noinline int is_opaque_dir(struct dentry *dentry, int bindex)
+{
+   int err = 0;
+   struct dentry *lower_dentry;
+   struct dentry *wh_lower_dentry;
+   struct inode *lower_inode;
+   struct sioq_args args;
+
+   lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+   lower_inode = lower_dentry->d_inode;
+
+   BUG_ON(!S_ISDIR(lower_inode->i_mode));
+
+   mutex_lock(_inode->i_mutex);
+
+   if (!permission(lower_inode, MAY_EXEC, NULL)) {
+   wh_lower_dentry =
+   lookup_one_len(UNIONFS_DIR_OPAQUE, lower_dentry,
+  sizeof(UNIONFS_DIR_OPAQUE) - 1);
+   } else {
+   args.is_opaque.dentry = lower_dentry;
+   run_sioq(__is_opaque_dir, );
+   wh_lower_dentry = args.ret;
+   }
+
+   mutex_unlock(_inode->i_mutex);
+
+   if (IS_ERR(wh_lower_dentry)) {
+   err = PTR_ERR(wh_lower_dentry);
+   goto out;
+   }
+
+   /* This is an opaque dir iff wh_lower_dentry is positive */
+   err = !!wh_lower_dentry->d_inode;
+
+   dput(wh_lower_dentry);
+out:
+   return err;
+}
+
+/*
+ * Main (and complex) driver function for Unionfs's lookup
+ *
+ * Returns: NULL (ok), ERR_PTR if an error occurred, or a non-null non-error
+ * PTR if d_splice returned a different dentry.
+ *
+ * If lookupmode is INTERPOSE_PARTIAL/REVAL/REVAL_NEG, the passed dentry's
+ * inode info must be locked.  If lookupmode is INTERPOSE_LOOKUP (i.e., a
+ * newly looked-up dentry), then unionfs_lookup_backend will return a locked
+ * dentry's info, which the caller must unlock.
+ */
+struct dentry *unionfs_lookup_backend(struct dentry *dentry,
+ struct nameidata *nd, int lookupmode)
+{
+   int err = 0;
+   struct dentry *lower_dentry = NULL;
+   struct dentry *wh_lower_dentry = NULL;
+   struct dentry *lower_dir_dentry = NULL;
+   struct dentry *parent_dentry = NULL;
+   struct dentry *d_interposed = NULL;
+   int bindex, bstart = -1, bend, bopaque;
+   int dentry_count = 0;   /* Number of positive dentries. */
+   int first_dentry_offset = -1; /* -1 is uninitialized */
+   struct dentry *first_dentry = NULL;
+   struct dentry *first_lower_dentry = NULL;
+   struct vfsmount *first_lower_mnt = NULL;
+   int opaque;
+   char *whname = NULL;
+   const char *name;
+   int namelen;
+
+   /*
+* We should already have a lock on this dentry in the case of a
+* partial lookup, or a revalidation. Otherwise it is returned from
+* new_dentry_private_data already locked.
+*/
+   if (lookupmode == INTERPOSE_PARTIAL || lookupmode == INTERPOSE_REVAL ||
+   lookupmode == INTERPOSE_REVAL_NEG)
+   verify_locked(dentry);
+   else/* this could only be INTERPOSE_LOOKUP */
+   BUG_ON(UNIONFS_D(dentry) != NULL);
+
+   switch (lookupmode) {
+   case INTERPOSE_PARTIAL:
+   break;
+   case INTERPOSE_LOOKUP:
+   err = new_dentry_private_data(dentry, UNIONFS_DMUTEX_CHILD);
+   if (unlikely(err))
+   goto out;
+   break;
+   default:
+

[PATCH 06/29] Unionfs: main header file

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/union.h |  602 
 1 files changed, 602 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/union.h

diff --git a/fs/unionfs/union.h b/fs/unionfs/union.h
new file mode 100644
index 000..d324f83
--- /dev/null
+++ b/fs/unionfs/union.h
@@ -0,0 +1,602 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _UNION_H_
+#define _UNION_H_
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+
+/* the file system name */
+#define UNIONFS_NAME "unionfs"
+
+/* unionfs root inode number */
+#define UNIONFS_ROOT_INO 1
+
+/* number of times we try to get a unique temporary file name */
+#define GET_TMPNAM_MAX_RETRY   5
+
+/* maximum number of branches we support, to avoid memory blowup */
+#define UNIONFS_MAX_BRANCHES   128
+
+/* minimum time (seconds) required for time-based cache-coherency */
+#define UNIONFS_MIN_CC_TIME3
+
+/* Operations vectors defined in specific files. */
+extern struct file_operations unionfs_main_fops;
+extern struct file_operations unionfs_dir_fops;
+extern struct inode_operations unionfs_main_iops;
+extern struct inode_operations unionfs_dir_iops;
+extern struct inode_operations unionfs_symlink_iops;
+extern struct super_operations unionfs_sops;
+extern struct dentry_operations unionfs_dops;
+extern struct address_space_operations unionfs_aops;
+
+/* How long should an entry be allowed to persist */
+#define RDCACHE_JIFFIES(5*HZ)
+
+/* file private data. */
+struct unionfs_file_info {
+   int bstart;
+   int bend;
+   atomic_t generation;
+
+   struct unionfs_dir_state *rdstate;
+   struct file **lower_files;
+   int *saved_branch_ids; /* IDs of branches when file was opened */
+};
+
+/* unionfs inode data in memory */
+struct unionfs_inode_info {
+   int bstart;
+   int bend;
+   atomic_t generation;
+   int stale;
+   /* Stuff for readdir over NFS. */
+   spinlock_t rdlock;
+   struct list_head readdircache;
+   int rdcount;
+   int hashsize;
+   int cookie;
+
+   /* The lower inodes */
+   struct inode **lower_inodes;
+
+   struct inode vfs_inode;
+};
+
+/* unionfs dentry data in memory */
+struct unionfs_dentry_info {
+   /*
+* The semaphore is used to lock the dentry as soon as we get into a
+* unionfs function from the VFS.  Our lock ordering is that children
+* go before their parents.
+*/
+   struct mutex lock;
+   int bstart;
+   int bend;
+   int bopaque;
+   int bcount;
+   atomic_t generation;
+   struct path *lower_paths;
+};
+
+/* These are the pointers to our various objects. */
+struct unionfs_data {
+   struct super_block *sb;
+   atomic_t open_files;/* number of open files on branch */
+   int branchperms;
+   int branch_id;  /* unique branch ID at re/mount time */
+};
+
+/* unionfs super-block data in memory */
+struct unionfs_sb_info {
+   int bend;
+
+   atomic_t generation;
+
+   /*
+* This rwsem is used to make sure that a branch management
+* operation...
+*   1) will not begin before all currently in-flight operations
+*  complete.
+*   2) any new operations do not execute until the currently
+*  running branch management operation completes.
+*
+* The write_lock_owner records the PID of the task which grabbed
+* the rw_sem for writing.  If the same task also tries to grab the
+* read lock, we allow it.  This prevents a self-deadlock when
+* branch-management is used on a pivot_root'ed union, because we
+* have to ->lookup paths which belong to the same union.
+*/
+   struct rw_semaphore rwsem;
+   pid_t write_lock_owner; /* PID of rw_sem owner (write lock) */
+   int high_branch_id; /* last unique branch ID given */
+   struct unionfs_data *data;
+};
+
+/*
+ * structure for making the linked list of

[PATCH 21/29] Unionfs: extended attributes operations

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/xattr.c |  153 
 1 files changed, 153 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/xattr.c

diff --git a/fs/unionfs/xattr.c b/fs/unionfs/xattr.c
new file mode 100644
index 000..8001c65
--- /dev/null
+++ b/fs/unionfs/xattr.c
@@ -0,0 +1,153 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+/* This is lifted from fs/xattr.c */
+void *unionfs_xattr_alloc(size_t size, size_t limit)
+{
+   void *ptr;
+
+   if (size > limit)
+   return ERR_PTR(-E2BIG);
+
+   if (!size)  /* size request, no buffer is needed */
+   return NULL;
+
+   ptr = kmalloc(size, GFP_KERNEL);
+   if (unlikely(!ptr))
+   return ERR_PTR(-ENOMEM);
+   return ptr;
+}
+
+/*
+ * BKL held by caller.
+ * dentry->d_inode->i_mutex locked
+ */
+ssize_t unionfs_getxattr(struct dentry *dentry, const char *name, void *value,
+size_t size)
+{
+   struct dentry *lower_dentry = NULL;
+   int err = -EOPNOTSUPP;
+
+   unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+
+   if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
+   err = -ESTALE;
+   goto out;
+   }
+
+   lower_dentry = unionfs_lower_dentry(dentry);
+
+   err = vfs_getxattr(lower_dentry, (char *) name, value, size);
+
+out:
+   unionfs_check_dentry(dentry);
+   unionfs_unlock_dentry(dentry);
+   unionfs_read_unlock(dentry->d_sb);
+   return err;
+}
+
+/*
+ * BKL held by caller.
+ * dentry->d_inode->i_mutex locked
+ */
+int unionfs_setxattr(struct dentry *dentry, const char *name,
+const void *value, size_t size, int flags)
+{
+   struct dentry *lower_dentry = NULL;
+   int err = -EOPNOTSUPP;
+
+   unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+
+   if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
+   err = -ESTALE;
+   goto out;
+   }
+
+   lower_dentry = unionfs_lower_dentry(dentry);
+
+   err = vfs_setxattr(lower_dentry, (char *) name, (void *) value,
+  size, flags);
+
+out:
+   unionfs_check_dentry(dentry);
+   unionfs_unlock_dentry(dentry);
+   unionfs_read_unlock(dentry->d_sb);
+   return err;
+}
+
+/*
+ * BKL held by caller.
+ * dentry->d_inode->i_mutex locked
+ */
+int unionfs_removexattr(struct dentry *dentry, const char *name)
+{
+   struct dentry *lower_dentry = NULL;
+   int err = -EOPNOTSUPP;
+
+   unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+
+   if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
+   err = -ESTALE;
+   goto out;
+   }
+
+   lower_dentry = unionfs_lower_dentry(dentry);
+
+   err = vfs_removexattr(lower_dentry, (char *) name);
+
+out:
+   unionfs_check_dentry(dentry);
+   unionfs_unlock_dentry(dentry);
+   unionfs_read_unlock(dentry->d_sb);
+   return err;
+}
+
+/*
+ * BKL held by caller.
+ * dentry->d_inode->i_mutex locked
+ */
+ssize_t unionfs_listxattr(struct dentry *dentry, char *list, size_t size)
+{
+   struct dentry *lower_dentry = NULL;
+   int err = -EOPNOTSUPP;
+   char *encoded_list = NULL;
+
+   unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+
+   if (unlikely(!__unionfs_d_revalidate_chain(dentry, NULL, false))) {
+   err = -ESTALE;
+   goto out;
+   }
+
+   lower_dentry = unionfs_lower_dentry(dentry);
+
+   encoded_list = list;
+   err = vfs_listxattr(lower_dentry, encoded_list, size);
+
+out:
+   unionfs_check_dentry(dentry);
+   unionfs_unlock_dentry(dentry);
+   unionfs_read_unlock(dentry->d_sb);
+   return err;
+}
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message 

[PATCH 07/29] Unionfs: common file copyup/revalidation operations

[Erez Zadok]

Includes open, ioctl, and flush operations.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/commonfops.c |  835 +++
 1 files changed, 835 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/commonfops.c

diff --git a/fs/unionfs/commonfops.c b/fs/unionfs/commonfops.c
new file mode 100644
index 000..f37192f
--- /dev/null
+++ b/fs/unionfs/commonfops.c
@@ -0,0 +1,835 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+/*
+ * 1) Copyup the file
+ * 2) Rename the file to '.unionfs' - obviously
+ * stolen from NFS's silly rename
+ */
+static int copyup_deleted_file(struct file *file, struct dentry *dentry,
+  int bstart, int bindex)
+{
+   static unsigned int counter;
+   const int i_inosize = sizeof(dentry->d_inode->i_ino) * 2;
+   const int countersize = sizeof(counter) * 2;
+   const int nlen = sizeof(".unionfs") + i_inosize + countersize - 1;
+   char name[nlen + 1];
+   int err;
+   struct dentry *tmp_dentry = NULL;
+   struct dentry *lower_dentry;
+   struct dentry *lower_dir_dentry = NULL;
+
+   lower_dentry = unionfs_lower_dentry_idx(dentry, bstart);
+
+   sprintf(name, ".unionfs%*.*lx",
+   i_inosize, i_inosize, lower_dentry->d_inode->i_ino);
+
+   /*
+* Loop, looking for an unused temp name to copyup to.
+*
+* It's somewhat silly that we look for a free temp tmp name in the
+* source branch (bstart) instead of the dest branch (bindex), where
+* the final name will be created.  We _will_ catch it if somehow
+* the name exists in the dest branch, but it'd be nice to catch it
+* sooner than later.
+*/
+retry:
+   tmp_dentry = NULL;
+   do {
+   char *suffix = name + nlen - countersize;
+
+   dput(tmp_dentry);
+   counter++;
+   sprintf(suffix, "%*.*x", countersize, countersize, counter);
+
+   pr_debug("unionfs: trying to rename %s to %s\n",
+dentry->d_name.name, name);
+
+   tmp_dentry = lookup_one_len(name, lower_dentry->d_parent,
+   nlen);
+   if (IS_ERR(tmp_dentry)) {
+   err = PTR_ERR(tmp_dentry);
+   goto out;
+   }
+   } while (tmp_dentry->d_inode != NULL);  /* need negative dentry */
+   dput(tmp_dentry);
+
+   err = copyup_named_file(dentry->d_parent->d_inode, file, name, bstart,
+   bindex,
+   i_size_read(file->f_path.dentry->d_inode));
+   if (err) {
+   if (unlikely(err == -EEXIST))
+   goto retry;
+   goto out;
+   }
+
+   /* bring it to the same state as an unlinked file */
+   lower_dentry = unionfs_lower_dentry_idx(dentry, dbstart(dentry));
+   if (!unionfs_lower_inode_idx(dentry->d_inode, bindex)) {
+   atomic_inc(_dentry->d_inode->i_count);
+   unionfs_set_lower_inode_idx(dentry->d_inode, bindex,
+   lower_dentry->d_inode);
+   }
+   lower_dir_dentry = lock_parent(lower_dentry);
+   err = vfs_unlink(lower_dir_dentry->d_inode, lower_dentry);
+   unlock_dir(lower_dir_dentry);
+
+out:
+   if (!err)
+   unionfs_check_dentry(dentry);
+   return err;
+}
+
+/*
+ * put all references held by upper struct file and free lower file pointer
+ * array
+ */
+static void cleanup_file(struct file *file)
+{
+   int bindex, bstart, bend;
+   struct file **lower_files;
+   struct file *lower_file;
+   struct super_block *sb = file->f_path.dentry->d_sb;
+
+   lower_files = UNIONFS_F(file)->lower_files;
+   bstart = fbstart(file);
+   bend = fbend(file);
+
+   for (bindex = bstart; bindex <= bend; bindex++) {
+   int i;  /* holds (possibly) updated branch index */
+   int old_bid;
+
+   lower_file = unionfs_lower_file_idx(file, bindex);
+   if (!lower_file)
+

[PATCH 10/29] Unionfs: dentry revalidation

[Erez Zadok]

Includes d_release methods and cache-coherency support for dentries.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/dentry.c |  548 +++
 1 files changed, 548 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/dentry.c

diff --git a/fs/unionfs/dentry.c b/fs/unionfs/dentry.c
new file mode 100644
index 000..cd15243
--- /dev/null
+++ b/fs/unionfs/dentry.c
@@ -0,0 +1,548 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+/*
+ * Revalidate a single dentry.
+ * Assume that dentry's info node is locked.
+ * Assume that parent(s) are all valid already, but
+ * the child may not yet be valid.
+ * Returns true if valid, false otherwise.
+ */
+static bool __unionfs_d_revalidate_one(struct dentry *dentry,
+  struct nameidata *nd)
+{
+   bool valid = true;  /* default is valid */
+   struct dentry *lower_dentry;
+   int bindex, bstart, bend;
+   int sbgen, dgen;
+   int positive = 0;
+   int interpose_flag;
+   struct nameidata lowernd; /* TODO: be gentler to the stack */
+
+   if (nd)
+   memcpy(, nd, sizeof(struct nameidata));
+   else
+   memset(, 0, sizeof(struct nameidata));
+
+   verify_locked(dentry);
+   verify_locked(dentry->d_parent);
+
+   /* if the dentry is unhashed, do NOT revalidate */
+   if (d_deleted(dentry))
+   goto out;
+
+   BUG_ON(dbstart(dentry) == -1);
+   if (dentry->d_inode)
+   positive = 1;
+   dgen = atomic_read(_D(dentry)->generation);
+   sbgen = atomic_read(_SB(dentry->d_sb)->generation);
+   /*
+* If we are working on an unconnected dentry, then there is no
+* revalidation to be done, because this file does not exist within
+* the namespace, and Unionfs operates on the namespace, not data.
+*/
+   if (unlikely(sbgen != dgen)) {
+   struct dentry *result;
+   int pdgen;
+
+   /* The root entry should always be valid */
+   BUG_ON(IS_ROOT(dentry));
+
+   /* We can't work correctly if our parent isn't valid. */
+   pdgen = atomic_read(_D(dentry->d_parent)->generation);
+   BUG_ON(pdgen != sbgen); /* should never happen here */
+
+   /* Free the pointers for our inodes and this dentry. */
+   bstart = dbstart(dentry);
+   bend = dbend(dentry);
+   if (bstart >= 0) {
+   struct dentry *lower_dentry;
+   for (bindex = bstart; bindex <= bend; bindex++) {
+   lower_dentry =
+   unionfs_lower_dentry_idx(dentry,
+bindex);
+   dput(lower_dentry);
+   }
+   }
+   set_dbstart(dentry, -1);
+   set_dbend(dentry, -1);
+
+   interpose_flag = INTERPOSE_REVAL_NEG;
+   if (positive) {
+   interpose_flag = INTERPOSE_REVAL;
+
+   bstart = ibstart(dentry->d_inode);
+   bend = ibend(dentry->d_inode);
+   if (bstart >= 0) {
+   struct inode *lower_inode;
+   for (bindex = bstart; bindex <= bend;
+bindex++) {
+   lower_inode =
+   unionfs_lower_inode_idx(
+   dentry->d_inode,
+   bindex);
+   iput(lower_inode);
+   }
+   }
+   kfree(UNIONFS_I(dentry->d_inode)->lower_inodes);
+   UNIONFS_I(dentry->d_inode)->lower_inodes = NULL;
+   ibstart(dentry->d_inode) = -1;
+   ibend(dentry->d_inode) = -1;
+   }
+
+   result = unionfs_lookup_back

[PATCH 13/29] Unionfs: directory reading file operations

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/dirfops.c |  290 ++
 1 files changed, 290 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/dirfops.c

diff --git a/fs/unionfs/dirfops.c b/fs/unionfs/dirfops.c
new file mode 100644
index 000..a613862
--- /dev/null
+++ b/fs/unionfs/dirfops.c
@@ -0,0 +1,290 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+/* Make sure our rdstate is playing by the rules. */
+static void verify_rdstate_offset(struct unionfs_dir_state *rdstate)
+{
+   BUG_ON(rdstate->offset >= DIREOF);
+   BUG_ON(rdstate->cookie >= MAXRDCOOKIE);
+}
+
+struct unionfs_getdents_callback {
+   struct unionfs_dir_state *rdstate;
+   void *dirent;
+   int entries_written;
+   int filldir_called;
+   int filldir_error;
+   filldir_t filldir;
+   struct super_block *sb;
+};
+
+/* based on generic filldir in fs/readir.c */
+static int unionfs_filldir(void *dirent, const char *name, int namelen,
+  loff_t offset, u64 ino, unsigned int d_type)
+{
+   struct unionfs_getdents_callback *buf = dirent;
+   struct filldir_node *found = NULL;
+   int err = 0;
+   int is_wh_entry = 0;
+
+   buf->filldir_called++;
+
+   if ((namelen > UNIONFS_WHLEN) &&
+   !strncmp(name, UNIONFS_WHPFX, UNIONFS_WHLEN)) {
+   name += UNIONFS_WHLEN;
+   namelen -= UNIONFS_WHLEN;
+   is_wh_entry = 1;
+   }
+
+   found = find_filldir_node(buf->rdstate, name, namelen, is_wh_entry);
+
+   if (found) {
+   /*
+* If we had non-whiteout entry in dir cache, then mark it
+* as a whiteout and but leave it in the dir cache.
+*/
+   if (is_wh_entry && !found->whiteout)
+   found->whiteout = is_wh_entry;
+   goto out;
+   }
+
+   /* if 'name' isn't a whiteout, filldir it. */
+   if (!is_wh_entry) {
+   off_t pos = rdstate2offset(buf->rdstate);
+   u64 unionfs_ino = ino;
+
+   err = buf->filldir(buf->dirent, name, namelen, pos,
+  unionfs_ino, d_type);
+   buf->rdstate->offset++;
+   verify_rdstate_offset(buf->rdstate);
+   }
+   /*
+* If we did fill it, stuff it in our hash, otherwise return an
+* error.
+*/
+   if (err) {
+   buf->filldir_error = err;
+   goto out;
+   }
+   buf->entries_written++;
+   err = add_filldir_node(buf->rdstate, name, namelen,
+  buf->rdstate->bindex, is_wh_entry);
+   if (err)
+   buf->filldir_error = err;
+
+out:
+   return err;
+}
+
+static int unionfs_readdir(struct file *file, void *dirent, filldir_t filldir)
+{
+   int err = 0;
+   struct file *lower_file = NULL;
+   struct inode *inode = NULL;
+   struct unionfs_getdents_callback buf;
+   struct unionfs_dir_state *uds;
+   int bend;
+   loff_t offset;
+
+   unionfs_read_lock(file->f_path.dentry->d_sb, UNIONFS_SMUTEX_PARENT);
+
+   err = unionfs_file_revalidate(file, false);
+   if (unlikely(err))
+   goto out;
+
+   inode = file->f_path.dentry->d_inode;
+
+   uds = UNIONFS_F(file)->rdstate;
+   if (!uds) {
+   if (file->f_pos == DIREOF) {
+   goto out;
+   } else if (file->f_pos > 0) {
+   uds = find_rdstate(inode, file->f_pos);
+   if (unlikely(!uds)) {
+   err = -ESTALE;
+   goto out;
+   }
+   UNIONFS_F(file)->rdstate = uds;
+   } else {
+   init_rdstate(file);
+   uds = UNIONFS_F(file)->rdstate;
+   }
+   }
+   bend = fbend(file);
+
+   while (uds->bindex <= bend) {
+   lower_file = unionfs_lower_file_idx(file, uds->bindex);
+ 

[PATCH 26/29] Unionfs: common header file for user-land utilities and kernel

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 include/linux/union_fs.h |   24 
 1 files changed, 24 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/union_fs.h

diff --git a/include/linux/union_fs.h b/include/linux/union_fs.h
new file mode 100644
index 000..9d601d2
--- /dev/null
+++ b/include/linux/union_fs.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _LINUX_UNION_FS_H
+#define _LINUX_UNION_FS_H
+
+#define UNIONFS_VERSION  "2.2-mm"
+
+/*
+ * DEFINITIONS FOR USER AND KERNEL CODE:
+ */
+# define UNIONFS_IOCTL_INCGEN  _IOR(0x15, 11, int)
+# define UNIONFS_IOCTL_QUERYFILE   _IOR(0x15, 15, int)
+
+#endif /* _LINUX_UNIONFS_H */
+
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 24/29] Unionfs: debugging infrastructure

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/debug.c |  533 
 1 files changed, 533 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/debug.c

diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
new file mode 100644
index 000..d154c32
--- /dev/null
+++ b/fs/unionfs/debug.c
@@ -0,0 +1,533 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+/*
+ * Helper debugging functions for maintainers (and for users to report back
+ * useful information back to maintainers)
+ */
+
+/* it's always useful to know what part of the code called us */
+#define PRINT_CALLER(fname, fxn, line) \
+   do {\
+   if (!printed_caller) {  \
+   pr_debug("PC:%s:%s:%d\n", (fname), (fxn), (line)); \
+   printed_caller = 1; \
+   }   \
+   } while (0)
+
+/*
+ * __unionfs_check_{inode,dentry,file} perform exhaustive sanity checking on
+ * the fan-out of various Unionfs objects.  We check that no lower objects
+ * exist  outside the start/end branch range; that all objects within are
+ * non-NULL (with some allowed exceptions); that for every lower file
+ * there's a lower dentry+inode; that the start/end ranges match for all
+ * corresponding lower objects; that open files/symlinks have only one lower
+ * objects, but directories can have several; and more.
+ */
+void __unionfs_check_inode(const struct inode *inode,
+  const char *fname, const char *fxn, int line)
+{
+   int bindex;
+   int istart, iend;
+   struct inode *lower_inode;
+   struct super_block *sb;
+   int printed_caller = 0;
+   void *poison_ptr;
+
+   /* for inodes now */
+   BUG_ON(!inode);
+   sb = inode->i_sb;
+   istart = ibstart(inode);
+   iend = ibend(inode);
+   /* don't check inode if no lower branches */
+   if (istart < 0 && iend < 0)
+   return;
+   if (unlikely(istart > iend)) {
+   PRINT_CALLER(fname, fxn, line);
+   pr_debug(" Ci0: inode=%p istart/end=%d:%d\n",
+inode, istart, iend);
+   }
+   if (unlikely((istart == -1 && iend != -1) ||
+(istart != -1 && iend == -1))) {
+   PRINT_CALLER(fname, fxn, line);
+   pr_debug(" Ci1: inode=%p istart/end=%d:%d\n",
+inode, istart, iend);
+   }
+   if (!S_ISDIR(inode->i_mode)) {
+   if (unlikely(iend != istart)) {
+   PRINT_CALLER(fname, fxn, line);
+   pr_debug(" Ci2: inode=%p istart=%d iend=%d\n",
+inode, istart, iend);
+   }
+   }
+
+   for (bindex = sbstart(sb); bindex < sbmax(sb); bindex++) {
+   if (unlikely(!UNIONFS_I(inode))) {
+   PRINT_CALLER(fname, fxn, line);
+   pr_debug(" Ci3: no inode_info %p\n", inode);
+   return;
+   }
+   if (unlikely(!UNIONFS_I(inode)->lower_inodes)) {
+   PRINT_CALLER(fname, fxn, line);
+   pr_debug(" Ci4: no lower_inodes %p\n", inode);
+   return;
+   }
+   lower_inode = unionfs_lower_inode_idx(inode, bindex);
+   if (lower_inode) {
+   memset(_ptr, POISON_INUSE, sizeof(void *));
+   if (unlikely(bindex < istart || bindex > iend)) {
+   PRINT_CALLER(fname, fxn, line);
+   pr_debug(" Ci5: inode/linode=%p:%p bindex=%d "
+"istart/end=%d:%d\n", inode,
+lower_inode, bindex, istart, iend);
+   } else if (unlikely(lower_inode == poison_ptr)) {
+   /* freed inode! */
+   PRINT_CALLER(fname, fxn, line);
+   pr_debug(" Ci6: inode/linode=%p:%p bindex=%d "
+"istart/end=%d:%d\n", inode,
+lower_inode, bindex, istart, iend);
+  

[PATCH 12/29] Unionfs: rename method and helpers

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/rename.c |  531 +++
 1 files changed, 531 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/rename.c

diff --git a/fs/unionfs/rename.c b/fs/unionfs/rename.c
new file mode 100644
index 000..9306a2b
--- /dev/null
+++ b/fs/unionfs/rename.c
@@ -0,0 +1,531 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+static int __unionfs_rename(struct inode *old_dir, struct dentry *old_dentry,
+   struct inode *new_dir, struct dentry *new_dentry,
+   int bindex, struct dentry **wh_old)
+{
+   int err = 0;
+   struct dentry *lower_old_dentry;
+   struct dentry *lower_new_dentry;
+   struct dentry *lower_old_dir_dentry;
+   struct dentry *lower_new_dir_dentry;
+   struct dentry *lower_wh_dentry;
+   struct dentry *lower_wh_dir_dentry;
+   char *wh_name = NULL;
+
+   lower_new_dentry = unionfs_lower_dentry_idx(new_dentry, bindex);
+   lower_old_dentry = unionfs_lower_dentry_idx(old_dentry, bindex);
+
+   if (!lower_new_dentry) {
+   lower_new_dentry =
+   create_parents(new_dentry->d_parent->d_inode,
+  new_dentry, new_dentry->d_name.name,
+  bindex);
+   if (IS_ERR(lower_new_dentry)) {
+   err = PTR_ERR(lower_new_dentry);
+   if (IS_COPYUP_ERR(err))
+   goto out;
+   printk(KERN_ERR "unionfs: error creating directory "
+  "tree for rename, bindex=%d err=%d\n",
+  bindex, err);
+   goto out;
+   }
+   }
+
+   wh_name = alloc_whname(new_dentry->d_name.name,
+  new_dentry->d_name.len);
+   if (unlikely(IS_ERR(wh_name))) {
+   err = PTR_ERR(wh_name);
+   goto out;
+   }
+
+   lower_wh_dentry = lookup_one_len(wh_name, lower_new_dentry->d_parent,
+new_dentry->d_name.len +
+UNIONFS_WHLEN);
+   if (IS_ERR(lower_wh_dentry)) {
+   err = PTR_ERR(lower_wh_dentry);
+   goto out;
+   }
+
+   if (lower_wh_dentry->d_inode) {
+   /* get rid of the whiteout that is existing */
+   if (lower_new_dentry->d_inode) {
+   printk(KERN_ERR "unionfs: both a whiteout and a "
+  "dentry exist when doing a rename!\n");
+   err = -EIO;
+
+   dput(lower_wh_dentry);
+   goto out;
+   }
+
+   lower_wh_dir_dentry = lock_parent_wh(lower_wh_dentry);
+   err = is_robranch_super(old_dentry->d_sb, bindex);
+   if (!err)
+   err = vfs_unlink(lower_wh_dir_dentry->d_inode,
+lower_wh_dentry);
+
+   dput(lower_wh_dentry);
+   unlock_dir(lower_wh_dir_dentry);
+   if (err)
+   goto out;
+   } else {
+   dput(lower_wh_dentry);
+   }
+
+   err = is_robranch_super(old_dentry->d_sb, bindex);
+   if (err)
+   goto out;
+
+   dget(lower_old_dentry);
+   lower_old_dir_dentry = dget_parent(lower_old_dentry);
+   lower_new_dir_dentry = dget_parent(lower_new_dentry);
+
+   /*
+* ready to whiteout for old_dentry. caller will create the actual
+* whiteout, and must dput(*wh_old)
+*/
+   if (wh_old) {
+   char *whname;
+   whname = alloc_whname(old_dentry->d_name.name,
+ old_dentry->d_name.len);
+   err = PTR_ERR(whname);
+   if (unlikely(IS_ERR(whname)))
+   goto out_dput;
+   *wh_old = lookup_one_len(whname, lower_old_dir_dentry,
+old_dentry->d_name.len +
+   

[PATCH 19/29] Unionfs: mount-time and stacking-interposition functions

[Erez Zadok]

Includes read_super and module-linkage routines.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/main.c |  794 +
 1 files changed, 794 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/main.c

diff --git a/fs/unionfs/main.c b/fs/unionfs/main.c
new file mode 100644
index 000..23c18f7
--- /dev/null
+++ b/fs/unionfs/main.c
@@ -0,0 +1,794 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+#include 
+#include 
+
+static void unionfs_fill_inode(struct dentry *dentry,
+  struct inode *inode)
+{
+   struct inode *lower_inode;
+   struct dentry *lower_dentry;
+   int bindex, bstart, bend;
+
+   bstart = dbstart(dentry);
+   bend = dbend(dentry);
+
+   for (bindex = bstart; bindex <= bend; bindex++) {
+   lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+   if (!lower_dentry) {
+   unionfs_set_lower_inode_idx(inode, bindex, NULL);
+   continue;
+   }
+
+   /* Initialize the lower inode to the new lower inode. */
+   if (!lower_dentry->d_inode)
+   continue;
+
+   unionfs_set_lower_inode_idx(inode, bindex,
+   igrab(lower_dentry->d_inode));
+   }
+
+   ibstart(inode) = dbstart(dentry);
+   ibend(inode) = dbend(dentry);
+
+   /* Use attributes from the first branch. */
+   lower_inode = unionfs_lower_inode(inode);
+
+   /* Use different set of inode ops for symlinks & directories */
+   if (S_ISLNK(lower_inode->i_mode))
+   inode->i_op = _symlink_iops;
+   else if (S_ISDIR(lower_inode->i_mode))
+   inode->i_op = _dir_iops;
+
+   /* Use different set of file ops for directories */
+   if (S_ISDIR(lower_inode->i_mode))
+   inode->i_fop = _dir_fops;
+
+   /* properly initialize special inodes */
+   if (S_ISBLK(lower_inode->i_mode) || S_ISCHR(lower_inode->i_mode) ||
+   S_ISFIFO(lower_inode->i_mode) || S_ISSOCK(lower_inode->i_mode))
+   init_special_inode(inode, lower_inode->i_mode,
+  lower_inode->i_rdev);
+
+   /* all well, copy inode attributes */
+   unionfs_copy_attr_all(inode, lower_inode);
+   fsstack_copy_inode_size(inode, lower_inode);
+}
+
+/*
+ * Connect a unionfs inode dentry/inode with several lower ones.  This is
+ * the classic stackable file system "vnode interposition" action.
+ *
+ * @sb: unionfs's super_block
+ */
+struct dentry *unionfs_interpose(struct dentry *dentry, struct super_block *sb,
+int flag)
+{
+   int err = 0;
+   struct inode *inode;
+   int is_negative_dentry = 1;
+   int bindex, bstart, bend;
+   int need_fill_inode = 1;
+   struct dentry *spliced = NULL;
+
+   verify_locked(dentry);
+
+   bstart = dbstart(dentry);
+   bend = dbend(dentry);
+
+   /* Make sure that we didn't get a negative dentry. */
+   for (bindex = bstart; bindex <= bend; bindex++) {
+   if (unionfs_lower_dentry_idx(dentry, bindex) &&
+   unionfs_lower_dentry_idx(dentry, bindex)->d_inode) {
+   is_negative_dentry = 0;
+   break;
+   }
+   }
+   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
+*/
+
+   /*
+* On revalidate we've already got our own inode and just need
+* to fix it up.
+*/
+   if (flag == INTERPOSE_REVAL) {
+   inode = dentry->d_inode;
+   UNIONFS_I(inode)->bstart = -1;
+   UNIONFS_I(inode)->bend = -1;
+   atomic_set(_I(inode)->generation,
+  atomic_read(_SB(sb)->generation));
+
+   UNIONFS_I(inode)->lower_inodes =
+   kcalloc(sbmax(sb), sizeof(struct inode *), GFP_KERNEL);
+   if (unlik

[PATCH 28/29] VFS: export release_open_intent symbol

[Erez Zadok]

Needed to release the resources of the lower nameidata structures that we
create and pass to lower file systems (e.g., when calling vfs_create).

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/namei.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 3b993db..14f9861 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -389,6 +389,7 @@ void release_open_intent(struct nameidata *nd)
else
fput(nd->intent.open.file);
 }
+EXPORT_SYMBOL(release_open_intent);
 
 static inline struct dentry *
 do_revalidate(struct dentry *dentry, struct nameidata *nd)
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 14/29] Unionfs: readdir helper functions

[Erez Zadok]

Includes whiteout handling for directories.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/dirhelper.c |  267 
 1 files changed, 267 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/dirhelper.c

diff --git a/fs/unionfs/dirhelper.c b/fs/unionfs/dirhelper.c
new file mode 100644
index 000..4b73bb6
--- /dev/null
+++ b/fs/unionfs/dirhelper.c
@@ -0,0 +1,267 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+/*
+ * Delete all of the whiteouts in a given directory for rmdir.
+ *
+ * lower directory inode should be locked
+ */
+int do_delete_whiteouts(struct dentry *dentry, int bindex,
+   struct unionfs_dir_state *namelist)
+{
+   int err = 0;
+   struct dentry *lower_dir_dentry = NULL;
+   struct dentry *lower_dentry;
+   char *name = NULL, *p;
+   struct inode *lower_dir;
+   int i;
+   struct list_head *pos;
+   struct filldir_node *cursor;
+
+   /* Find out lower parent dentry */
+   lower_dir_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+   BUG_ON(!S_ISDIR(lower_dir_dentry->d_inode->i_mode));
+   lower_dir = lower_dir_dentry->d_inode;
+   BUG_ON(!S_ISDIR(lower_dir->i_mode));
+
+   err = -ENOMEM;
+   name = __getname();
+   if (unlikely(!name))
+   goto out;
+   strcpy(name, UNIONFS_WHPFX);
+   p = name + UNIONFS_WHLEN;
+
+   err = 0;
+   for (i = 0; !err && i < namelist->size; i++) {
+   list_for_each(pos, >list[i]) {
+   cursor =
+   list_entry(pos, struct filldir_node,
+  file_list);
+   /* Only operate on whiteouts in this branch. */
+   if (cursor->bindex != bindex)
+   continue;
+   if (!cursor->whiteout)
+   continue;
+
+   strcpy(p, cursor->name);
+   lower_dentry =
+   lookup_one_len(name, lower_dir_dentry,
+  cursor->namelen +
+  UNIONFS_WHLEN);
+   if (IS_ERR(lower_dentry)) {
+   err = PTR_ERR(lower_dentry);
+   break;
+   }
+   if (lower_dentry->d_inode)
+   err = vfs_unlink(lower_dir, lower_dentry);
+   dput(lower_dentry);
+   if (err)
+   break;
+   }
+   }
+
+   __putname(name);
+
+   /* After all of the removals, we should copy the attributes once. */
+   fsstack_copy_attr_times(dentry->d_inode, lower_dir_dentry->d_inode);
+
+out:
+   return err;
+}
+
+/* delete whiteouts in a dir (for rmdir operation) using sioq if necessary */
+int delete_whiteouts(struct dentry *dentry, int bindex,
+struct unionfs_dir_state *namelist)
+{
+   int err;
+   struct super_block *sb;
+   struct dentry *lower_dir_dentry;
+   struct inode *lower_dir;
+   struct sioq_args args;
+
+   sb = dentry->d_sb;
+
+   BUG_ON(!S_ISDIR(dentry->d_inode->i_mode));
+   BUG_ON(bindex < dbstart(dentry));
+   BUG_ON(bindex > dbend(dentry));
+   err = is_robranch_super(sb, bindex);
+   if (err)
+   goto out;
+
+   lower_dir_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+   BUG_ON(!S_ISDIR(lower_dir_dentry->d_inode->i_mode));
+   lower_dir = lower_dir_dentry->d_inode;
+   BUG_ON(!S_ISDIR(lower_dir->i_mode));
+
+   if (!permission(lower_dir, MAY_WRITE | MAY_EXEC, NULL)) {
+   err = do_delete_whiteouts(dentry, bindex, namelist);
+   } else {
+   args.deletewh.namelist = namelist;
+   args.deletewh.dentry = dentry;
+   args.deletewh.bindex = bindex;
+   run_sioq(__delete_whiteouts, );
+   err = args.err;
+   }
+
+out:
+   return err;
+}
+
+#define RD_NONE 0
+#define RD

[PATCH 25/29] Unionfs file system magic number

[Erez Zadok]

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 include/linux/magic.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/magic.h b/include/linux/magic.h
index 1fa0c2c..67043ed 100644
--- a/include/linux/magic.h
+++ b/include/linux/magic.h
@@ -35,6 +35,8 @@
 #define REISER2FS_SUPER_MAGIC_STRING   "ReIsEr2Fs"
 #define REISER2FS_JR_SUPER_MAGIC_STRING"ReIsEr3Fs"
 
+#define UNIONFS_SUPER_MAGIC 0xf15f083d
+
 #define SMB_SUPER_MAGIC0x517B
 #define USBDEVICE_SUPER_MAGIC  0x9fa2
 #define CGROUP_SUPER_MAGIC 0x27e0eb
-- 
1.5.2.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 16/29] Unionfs: inode operations

[Erez Zadok]

Includes create, lookup, link, symlink, mkdir, mknod, readlink, follow_link,
put_link, permission, and setattr.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/inode.c | 1174 
 1 files changed, 1174 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/inode.c

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
new file mode 100644
index 000..e15ddb9
--- /dev/null
+++ b/fs/unionfs/inode.c
@@ -0,0 +1,1174 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+static int unionfs_create(struct inode *parent, struct dentry *dentry,
+ int mode, struct nameidata *nd)
+{
+   int err = 0;
+   struct dentry *lower_dentry = NULL;
+   struct dentry *wh_dentry = NULL;
+   struct dentry *lower_parent_dentry = NULL;
+   char *name = NULL;
+   int valid = 0;
+   struct nameidata lower_nd;
+
+   unionfs_read_lock(dentry->d_sb, UNIONFS_SMUTEX_CHILD);
+   unionfs_lock_dentry(dentry->d_parent, UNIONFS_DMUTEX_PARENT);
+   valid = __unionfs_d_revalidate_chain(dentry->d_parent, nd, false);
+   unionfs_unlock_dentry(dentry->d_parent);
+   if (unlikely(!valid)) {
+   err = -ESTALE;  /* same as what real_lookup does */
+   goto out;
+   }
+   unionfs_lock_dentry(dentry, UNIONFS_DMUTEX_CHILD);
+
+   valid = __unionfs_d_revalidate_chain(dentry, nd, false);
+   /*
+* It's only a bug if this dentry was not negative and couldn't be
+* revalidated (shouldn't happen).
+*/
+   BUG_ON(!valid && dentry->d_inode);
+
+   /*
+* We shouldn't create things in a read-only branch; this check is a
+* bit redundant as we don't allow branch 0 to be read-only at the
+* moment
+*/
+   err = is_robranch_super(dentry->d_sb, 0);
+   if (err) {
+   err = -EROFS;
+   goto out;
+   }
+
+   /*
+* We _always_ create on branch 0
+*/
+   lower_dentry = unionfs_lower_dentry_idx(dentry, 0);
+   if (lower_dentry) {
+   /*
+* check if whiteout exists in this branch, i.e. lookup .wh.foo
+* first.
+*/
+   name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
+   if (unlikely(IS_ERR(name))) {
+   err = PTR_ERR(name);
+   goto out;
+   }
+
+   wh_dentry = lookup_one_len(name, lower_dentry->d_parent,
+  dentry->d_name.len + UNIONFS_WHLEN);
+   if (IS_ERR(wh_dentry)) {
+   err = PTR_ERR(wh_dentry);
+   wh_dentry = NULL;
+   goto out;
+   }
+
+   if (wh_dentry->d_inode) {
+   /*
+* .wh.foo has been found, so let's unlink it
+*/
+   struct dentry *lower_dir_dentry;
+
+   lower_dir_dentry = lock_parent_wh(wh_dentry);
+   /* see Documentation/filesystems/unionfs/issues.txt */
+   lockdep_off();
+   err = vfs_unlink(lower_dir_dentry->d_inode, wh_dentry);
+   lockdep_on();
+   unlock_dir(lower_dir_dentry);
+
+   /*
+* Whiteouts are special files and should be deleted
+* no matter what (as if they never existed), in
+* order to allow this create operation to succeed.
+* This is especially important in sticky
+* directories: a whiteout may have been created by
+* one user, but the newly created file may be
+* created by another user.  Therefore, in order to
+* maintain Unix semantics, if the vfs_unlink above
+* ailed, then we have to try to directly unlink the
+* whiteout.  Note: in the ODF version of unionfs,
+* whiteout are handled much more cleanly.

[PATCH 18/29] Unionfs: address-space operations

[Erez Zadok]

Includes writepage, writepages, readpage, prepare_write, and commit_write.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/mmap.c |  343 +
 1 files changed, 343 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/mmap.c

diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
new file mode 100644
index 000..ad770ac
--- /dev/null
+++ b/fs/unionfs/mmap.c
@@ -0,0 +1,343 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2006  Shaya Potter
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
+{
+   int err = -EIO;
+   struct inode *inode;
+   struct inode *lower_inode;
+   struct page *lower_page;
+   struct address_space *lower_mapping; /* lower inode mapping */
+   gfp_t mask;
+
+   BUG_ON(!PageUptodate(page));
+   inode = page->mapping->host;
+   /* if no lower inode, nothing to do */
+   if (!inode || !UNIONFS_I(inode) || UNIONFS_I(inode)->lower_inodes) {
+   err = 0;
+   goto out;
+   }
+   lower_inode = unionfs_lower_inode(inode);
+   lower_mapping = lower_inode->i_mapping;
+
+   /*
+* find lower page (returns a locked page)
+*
+* We turn off __GFP_FS while we look for or create a new lower
+* page.  This prevents a recursion into the file system code, which
+* under memory pressure conditions could lead to a deadlock.  This
+* is similar to how the loop driver behaves (see loop_set_fd in
+* drivers/block/loop.c).  If we can't find the lower page, we
+* redirty our page and return "success" so that the VM will call us
+* again in the (hopefully near) future.
+*/
+   mask = mapping_gfp_mask(lower_mapping) & ~(__GFP_FS);
+   lower_page = find_or_create_page(lower_mapping, page->index, mask);
+   if (!lower_page) {
+   err = 0;
+   set_page_dirty(page);
+   goto out;
+   }
+
+   /* copy page data from our upper page to the lower page */
+   copy_highpage(lower_page, page);
+   flush_dcache_page(lower_page);
+   SetPageUptodate(lower_page);
+   set_page_dirty(lower_page);
+
+   /*
+* Call lower writepage (expects locked page).  However, if we are
+* called with wbc->for_reclaim, then the VFS/VM just wants to
+* reclaim our page.  Therefore, we don't need to call the lower
+* ->writepage: just copy our data to the lower page (already done
+* above), then mark the lower page dirty and unlock it, and return
+* success.
+*/
+   if (wbc->for_reclaim) {
+   unlock_page(lower_page);
+   goto out_release;
+   }
+
+   BUG_ON(!lower_mapping->a_ops->writepage);
+   wait_on_page_writeback(lower_page); /* prevent multiple writers */
+   clear_page_dirty_for_io(lower_page); /* emulate VFS behavior */
+   err = lower_mapping->a_ops->writepage(lower_page, wbc);
+   if (err < 0)
+   goto out_release;
+
+   /*
+* Lower file systems such as ramfs and tmpfs, may return
+* AOP_WRITEPAGE_ACTIVATE so that the VM won't try to (pointlessly)
+* write the page again for a while.  But those lower file systems
+* also set the page dirty bit back again.  Since we successfully
+* copied our page data to the lower page, then the VM will come
+* back to the lower page (directly) and try to flush it.  So we can
+* save the VM the hassle of coming back to our page and trying to
+* flush too.  Therefore, we don't re-dirty our own page, and we
+* never return AOP_WRITEPAGE_ACTIVATE back to the VM (we consider
+* this a success).
+*
+* We also unlock the lower page if the lower ->writepage returned
+* AOP_WRITEPAGE_ACTIVATE.  (This "anomalous" behaviour may be
+* addressed in future shmem/VM code.)
+*/
+   if (err == AOP_WRITEPAGE_ACTIVATE) {
+   err = 0;
+   unlock_page(lower_page);
+   }
+
+   /* all is well */
+
+   /* lower mtimes h

[PATCH 23/29] Unionfs: miscellaneous helper routines

[Erez Zadok]

Mostly related to whiteouts.

Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/unionfs/subr.c |  242 +
 1 files changed, 242 insertions(+), 0 deletions(-)
 create mode 100644 fs/unionfs/subr.c

diff --git a/fs/unionfs/subr.c b/fs/unionfs/subr.c
new file mode 100644
index 000..0a0fce9
--- /dev/null
+++ b/fs/unionfs/subr.c
@@ -0,0 +1,242 @@
+/*
+ * Copyright (c) 2003-2007 Erez Zadok
+ * Copyright (c) 2003-2006 Charles P. Wright
+ * Copyright (c) 2005-2007 Josef 'Jeff' Sipek
+ * Copyright (c) 2005-2006 Junjiro Okajima
+ * Copyright (c) 2005  Arun M. Krishnakumar
+ * Copyright (c) 2004-2006 David P. Quigley
+ * Copyright (c) 2003-2004 Mohammad Nayyer Zubair
+ * Copyright (c) 2003  Puja Gupta
+ * Copyright (c) 2003  Harikesavan Krishnan
+ * Copyright (c) 2003-2007 Stony Brook University
+ * Copyright (c) 2003-2007 The Research Foundation of SUNY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "union.h"
+
+/*
+ * Pass an unionfs dentry and an index.  It will try to create a whiteout
+ * for the filename in dentry, and will try in branch 'index'.  On error,
+ * it will proceed to a branch to the left.
+ */
+int create_whiteout(struct dentry *dentry, int start)
+{
+   int bstart, bend, bindex;
+   struct dentry *lower_dir_dentry;
+   struct dentry *lower_dentry;
+   struct dentry *lower_wh_dentry;
+   struct nameidata nd;
+   char *name = NULL;
+   int err = -EINVAL;
+
+   verify_locked(dentry);
+
+   bstart = dbstart(dentry);
+   bend = dbend(dentry);
+
+   /* create dentry's whiteout equivalent */
+   name = alloc_whname(dentry->d_name.name, dentry->d_name.len);
+   if (unlikely(IS_ERR(name))) {
+   err = PTR_ERR(name);
+   goto out;
+   }
+
+   for (bindex = start; bindex >= 0; bindex--) {
+   lower_dentry = unionfs_lower_dentry_idx(dentry, bindex);
+
+   if (!lower_dentry) {
+   /*
+* if lower dentry is not present, create the
+* entire lower dentry directory structure and go
+* ahead.  Since we want to just create whiteout, we
+* only want the parent dentry, and hence get rid of
+* this dentry.
+*/
+   lower_dentry = create_parents(dentry->d_inode,
+ dentry,
+ dentry->d_name.name,
+ bindex);
+   if (!lower_dentry || IS_ERR(lower_dentry)) {
+   int ret = PTR_ERR(lower_dentry);
+   if (!IS_COPYUP_ERR(ret))
+   printk(KERN_ERR
+  "unionfs: create_parents for "
+  "whiteout failed: bindex=%d "
+  "err=%d\n", bindex, ret);
+   continue;
+   }
+   }
+
+   lower_wh_dentry =
+   lookup_one_len(name, lower_dentry->d_parent,
+  dentry->d_name.len + UNIONFS_WHLEN);
+   if (IS_ERR(lower_wh_dentry))
+   continue;
+
+   /*
+* The whiteout already exists. This used to be impossible,
+* but now is possible because of opaqueness.
+*/
+   if (lower_wh_dentry->d_inode) {
+   dput(lower_wh_dentry);
+   err = 0;
+   goto out;
+   }
+
+   err = init_lower_nd(, LOOKUP_CREATE);
+   if (unlikely(err < 0))
+   goto out;
+   lower_dir_dentry = lock_parent_wh(lower_wh_dentry);
+   err = is_robranch_super(dentry->d_sb, bindex);
+   if (!err)
+   err = vfs_create(lower_dir_dentry->d_inode,
+lower_wh_dentry,
+~current->fs->umask & S_IRWXUGO,
+);
+   unlock_dir(lower_dir_dentry);
+   dput(lower_wh_dentry);
+   release_lower_nd(, err);
+
+   if (!err || !IS_COPYUP_ERR(err))
+   break;
+   }
+
+   /* set dbopaque so that lookup will not proceed after this branch */
+   if (!err)
+   set_dbopaque(dentry, bindex);
+
+out:
+   kfree(na