Re: [RFC][PATCH 7/14] Union-mount mounting

2007-05-15 Thread Bharata B Rao
On Tue, May 15, 2007 at 09:29:39AM +0200, Jan Engelhardt wrote:
> 
> On May 14 2007 15:11, Bharata B Rao wrote:
> >
> >TODO: bind and move mounts aren't yet supported with union mounts.
> 
> Are the semantics already set?

Not yet.

> 
> >@@ -294,6 +294,10 @@ static struct vfsmount *clone_mnt(struct
> > if (!mnt)
> > goto alloc_failed;
> > 
> >+/*
> >+ * As of now, cloning of union mounted mnt isn't permitted.
> >+ */
> >+BUG_ON(mnt->mnt_flags & MNT_UNION);
> 
> One, please avoid BUG_ONs. Now I am not sure if clone_mnt is called
> as part of kthread creation when CLONE_FS is it. If so, get rid of
> this one real fast. Also see chunk "@@ -1031,.. @@ do_loopback"
> below.

Looks like not CLONE_FS but CLONE_NEWNS ends up calling clone_mnt.

> 
>   if(mnt->mnt_flags & MNT_UNION)
>   goto return_einval;
> 
> or something.

Will do this for now, but eventually we need to get this working sanely anyway.

> 
> >+#ifdef CONFIG_UNION_MOUNT
> >+struct union_info *uinfo = NULL;
> >+#endif
> > 
> > retval = security_sb_umount(mnt, flags);
> > if (retval)
> >@@ -685,6 +696,14 @@ static int do_umount(struct vfsmount *mn
> > }
> > 
> > down_write(&namespace_sem);
> >+#ifdef CONFIG_UNION_MOUNT
> >+/*
> >+ * Grab a reference to the union_info which gets detached
> >+ * from the dentries in release_mounts().
> >+ */
> >+if (mnt->mnt_flags & MNT_UNION)
> >+uinfo = union_lock_and_get(mnt->mnt_root);
> >+#endif
> > spin_lock(&vfsmount_lock);
> > event++;
> > 
> >@@ -699,6 +718,15 @@ static int do_umount(struct vfsmount *mn
> > security_sb_umount_busy(mnt);
> > up_write(&namespace_sem);
> > release_mounts(&umount_list);
> >+#ifdef CONFIG_UNION_MOUNT
> >+if (uinfo) {
> >+if (atomic_read(&uinfo->u_count) == 1)
> >+/* We are the last user of this union_info */
> >+union_release(uinfo);
> >+else
> >+union_put_and_unlock(uinfo);
> >+}
> >+#endif
> > return retval;
> > }
> > 
> 
> Is it feasible to do with with some less #if/#endif magic?:
> 

Will try. We need union_info here which is available only with
CONFIG_UNION_MOUNT.

> >@@ -1031,6 +1070,15 @@ static int do_loopback(struct nameidata 
> > if (err)
> > return err;
> > 
> >+/*
> >+ * bind mounting to or from union mounts is not supported
> >+ */
> >+err = -EINVAL;
> >+if (nd->mnt->mnt_flags & MNT_UNION)
> >+goto out_unlocked;
> >+if (old_nd.mnt->mnt_flags & MNT_UNION)
> >+goto out_unlocked;
> >+
> 
> Do the same in clone_mnt.
> 
> > down_write(&namespace_sem);
> > err = -EINVAL;
> > if (IS_MNT_UNBINDABLE(old_nd.mnt))
> >@@ -1064,6 +1112,7 @@ static int do_loopback(struct nameidata 
> > 
> > out:
> > up_write(&namespace_sem);
> >+out_unlocked:
> > path_release(&old_nd);
> > return err;
> > }
> 
> >+++ b/include/linux/fs.h
> >@@ -1984,6 +1984,9 @@ static inline ino_t parent_ino(struct de
> > /* kernel/fork.c */
> > extern int unshare_files(void);
> > 
> >+/* fs/union.c */
> >+#include 
> >+
> > /* Transaction based IO helpers */
> > 
> > /*
> 
> This raises a big eyebrow. If linux/fs.h can compile without the
> inclusion of linux/union.h, do not put linux/union.h in fs.h.
> 

Ok, better to include union.h in the appropriate .c file which needs it.

> >+#ifdef CONFIG_UNION_MOUNT
> >+
> >+#include 
> >+
> >+/* namespace stuff used at mount time */
> >+extern void attach_mnt_union(struct vfsmount *, struct nameidata *);
> >+extern void detach_mnt_union(struct vfsmount *, struct path *);
> 
> You do not need that #include I suppose. Just predeclare the structs.
> 
>   struct path;
>   struct vfsmount;
>   extern void ...
> 
> Saves us the "compiler slurps in so many .h files" problem.

Sure. And thanks for the review.

Regards,
Bharata.
-
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: [RFC][PATCH 7/14] Union-mount mounting

2007-05-15 Thread Jan Engelhardt

On May 14 2007 15:11, Bharata B Rao wrote:
>
>TODO: bind and move mounts aren't yet supported with union mounts.

Are the semantics already set?

>@@ -294,6 +294,10 @@ static struct vfsmount *clone_mnt(struct
>   if (!mnt)
>   goto alloc_failed;
> 
>+  /*
>+   * As of now, cloning of union mounted mnt isn't permitted.
>+   */
>+  BUG_ON(mnt->mnt_flags & MNT_UNION);

One, please avoid BUG_ONs. Now I am not sure if clone_mnt is called
as part of kthread creation when CLONE_FS is it. If so, get rid of
this one real fast. Also see chunk "@@ -1031,.. @@ do_loopback"
below.

if(mnt->mnt_flags & MNT_UNION)
goto return_einval;

or something.

>+#ifdef CONFIG_UNION_MOUNT
>+  struct union_info *uinfo = NULL;
>+#endif
> 
>   retval = security_sb_umount(mnt, flags);
>   if (retval)
>@@ -685,6 +696,14 @@ static int do_umount(struct vfsmount *mn
>   }
> 
>   down_write(&namespace_sem);
>+#ifdef CONFIG_UNION_MOUNT
>+  /*
>+   * Grab a reference to the union_info which gets detached
>+   * from the dentries in release_mounts().
>+   */
>+  if (mnt->mnt_flags & MNT_UNION)
>+  uinfo = union_lock_and_get(mnt->mnt_root);
>+#endif
>   spin_lock(&vfsmount_lock);
>   event++;
> 
>@@ -699,6 +718,15 @@ static int do_umount(struct vfsmount *mn
>   security_sb_umount_busy(mnt);
>   up_write(&namespace_sem);
>   release_mounts(&umount_list);
>+#ifdef CONFIG_UNION_MOUNT
>+  if (uinfo) {
>+  if (atomic_read(&uinfo->u_count) == 1)
>+  /* We are the last user of this union_info */
>+  union_release(uinfo);
>+  else
>+  union_put_and_unlock(uinfo);
>+  }
>+#endif
>   return retval;
> }
> 

Is it feasible to do with with some less #if/#endif magic?:

>@@ -1031,6 +1070,15 @@ static int do_loopback(struct nameidata 
>   if (err)
>   return err;
> 
>+  /*
>+   * bind mounting to or from union mounts is not supported
>+   */
>+  err = -EINVAL;
>+  if (nd->mnt->mnt_flags & MNT_UNION)
>+  goto out_unlocked;
>+  if (old_nd.mnt->mnt_flags & MNT_UNION)
>+  goto out_unlocked;
>+

Do the same in clone_mnt.

>   down_write(&namespace_sem);
>   err = -EINVAL;
>   if (IS_MNT_UNBINDABLE(old_nd.mnt))
>@@ -1064,6 +1112,7 @@ static int do_loopback(struct nameidata 
> 
> out:
>   up_write(&namespace_sem);
>+out_unlocked:
>   path_release(&old_nd);
>   return err;
> }

>+++ b/include/linux/fs.h
>@@ -1984,6 +1984,9 @@ static inline ino_t parent_ino(struct de
> /* kernel/fork.c */
> extern int unshare_files(void);
> 
>+/* fs/union.c */
>+#include 
>+
> /* Transaction based IO helpers */
> 
> /*

This raises a big eyebrow. If linux/fs.h can compile without the
inclusion of linux/union.h, do not put linux/union.h in fs.h.

>+#ifdef CONFIG_UNION_MOUNT
>+
>+#include 
>+
>+/* namespace stuff used at mount time */
>+extern void attach_mnt_union(struct vfsmount *, struct nameidata *);
>+extern void detach_mnt_union(struct vfsmount *, struct path *);

You do not need that #include I suppose. Just predeclare the structs.

struct path;
struct vfsmount;
extern void ...

Saves us the "compiler slurps in so many .h files" problem.


Jan
-- 
-
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/


[RFC][PATCH 7/14] Union-mount mounting

2007-05-14 Thread Bharata B Rao
From: Jan Blunck <[EMAIL PROTECTED]>
Subject: Union-mount mounting

Adds union mount support to mount() and umount() system calls.
Sets up the union stack during mount and destroys it during unmount.

TODO: bind and move mounts aren't yet supported with union mounts.

Signed-off-by: Jan Blunck <[EMAIL PROTECTED]>
Signed-off-by: Bharata B Rao <[EMAIL PROTECTED]>
---
 fs/namespace.c|   90 ++
 fs/union.c|   71 +++
 include/linux/fs.h|3 +
 include/linux/union.h |   33 ++
 4 files changed, 190 insertions(+), 7 deletions(-)

--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -169,7 +169,7 @@ void mnt_set_mountpoint(struct vfsmount 
struct vfsmount *child_mnt)
 {
child_mnt->mnt_parent = mntget(mnt);
-   child_mnt->mnt_mountpoint = dget(dentry);
+   child_mnt->mnt_mountpoint = __dget(dentry);
dentry->d_mounted++;
 }
 
@@ -294,6 +294,10 @@ static struct vfsmount *clone_mnt(struct
if (!mnt)
goto alloc_failed;
 
+   /*
+* As of now, cloning of union mounted mnt isn't permitted.
+*/
+   BUG_ON(mnt->mnt_flags & MNT_UNION);
mnt->mnt_flags = old->mnt_flags;
atomic_inc(&sb->s_active);
mnt->mnt_sb = sb;
@@ -579,16 +583,20 @@ void release_mounts(struct list_head *he
mnt = list_first_entry(head, struct vfsmount, mnt_hash);
list_del_init(&mnt->mnt_hash);
if (mnt->mnt_parent != mnt) {
-   struct dentry *dentry;
-   struct vfsmount *m;
+   struct path old_nd;
spin_lock(&vfsmount_lock);
-   dentry = mnt->mnt_mountpoint;
-   m = mnt->mnt_parent;
+   old_nd.dentry = mnt->mnt_mountpoint;
+   old_nd.mnt = mnt->mnt_parent;
mnt->mnt_mountpoint = mnt->mnt_root;
mnt->mnt_parent = mnt;
+   detach_mnt_union(mnt, &old_nd);
spin_unlock(&vfsmount_lock);
-   dput(dentry);
-   mntput(m);
+   if (mnt->mnt_flags & MNT_UNION) {
+   UM_DEBUG("shrink the mountpoint's dcache\n");
+   shrink_dcache_sb(old_nd.dentry->d_sb);
+   }
+   __dput(old_nd.dentry);
+   mntput(old_nd.mnt);
}
mntput(mnt);
}
@@ -621,6 +629,9 @@ static int do_umount(struct vfsmount *mn
struct super_block *sb = mnt->mnt_sb;
int retval;
LIST_HEAD(umount_list);
+#ifdef CONFIG_UNION_MOUNT
+   struct union_info *uinfo = NULL;
+#endif
 
retval = security_sb_umount(mnt, flags);
if (retval)
@@ -685,6 +696,14 @@ static int do_umount(struct vfsmount *mn
}
 
down_write(&namespace_sem);
+#ifdef CONFIG_UNION_MOUNT
+   /*
+* Grab a reference to the union_info which gets detached
+* from the dentries in release_mounts().
+*/
+   if (mnt->mnt_flags & MNT_UNION)
+   uinfo = union_lock_and_get(mnt->mnt_root);
+#endif
spin_lock(&vfsmount_lock);
event++;
 
@@ -699,6 +718,15 @@ static int do_umount(struct vfsmount *mn
security_sb_umount_busy(mnt);
up_write(&namespace_sem);
release_mounts(&umount_list);
+#ifdef CONFIG_UNION_MOUNT
+   if (uinfo) {
+   if (atomic_read(&uinfo->u_count) == 1)
+   /* We are the last user of this union_info */
+   union_release(uinfo);
+   else
+   union_put_and_unlock(uinfo);
+   }
+#endif
return retval;
 }
 
@@ -941,6 +969,9 @@ static int attach_recursive_mnt(struct v
set_mnt_shared(p);
}
 
+   if (source_mnt->mnt_flags & MNT_UNION)
+   union_alloc_dentry(nd->dentry);
+
spin_lock(&vfsmount_lock);
if (parent_nd) {
detach_mnt(source_mnt, parent_nd);
@@ -948,6 +979,7 @@ static int attach_recursive_mnt(struct v
touch_mnt_namespace(current->nsproxy->mnt_ns);
} else {
mnt_set_mountpoint(dest_mnt, dest_dentry, source_mnt);
+   attach_mnt_union(source_mnt, nd);
commit_tree(source_mnt);
}
 
@@ -956,6 +988,7 @@ static int attach_recursive_mnt(struct v
commit_tree(child);
}
spin_unlock(&vfsmount_lock);
+   union_unlock(nd->dentry);
return 0;
 }
 
@@ -1003,6 +1036,12 @@ static int do_change_type(struct nameida
if (nd->dentry != nd->mnt->mnt_root)
return -EINVAL;
 
+   /*
+* Don't change the type of union mounts
+*/
+   if (nd->mnt->mnt_flags & MNT