Kernel patch to better support bind-mounts

Kernel patch to better support bind-mounts

[Lukas Kolbe]

Hi!

Ian, you made a patch for our problem here late laste year that didn't
quite work for all of our cases. I don't really understand the code in
the kernel module, but logic tells me that this patch should be safe.
Plus, it's solving our problems and is straightforward. Your patch was:

diff -up linux-2.6.21/fs/autofs4/root.c.lookup_access-intent linux-2.6.21/fs/autofs4/root.c
--- linux-2.6.21/fs/autofs4/root.c.lookup_access-intent 2007-10-11 16:32:07.000000000 +0800
+++ linux-2.6.21/fs/autofs4/root.c      2007-10-11 16:34:59.000000000 +0800
@@ -20,6 +20,8 @@
 #include <linux/smp_lock.h>
 #include "autofs_i.h"
 
+#define DIRECT_TRIGGER_FLAGS (LOOKUP_CONTINUE|LOOKUP_DIRECTORY|LOOKUP_ACCESS)
+
 static int autofs4_dir_symlink(struct inode *,struct dentry *,const char *);
 static int autofs4_dir_unlink(struct inode *,struct dentry *);
 static int autofs4_dir_rmdir(struct inode *,struct dentry *);
@@ -336,7 +338,7 @@ static void *autofs4_follow_link(struct 
                nd->flags);
 
        /* If it's our master or we shouldn't trigger a mount we're done */
-       lookup_type = nd->flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY);
+       lookup_type = nd->flags & DIRECT_TRIGGER_FLAGS;
        if (oz_mode || !lookup_type)
                goto done;


This helps for a map like this:

eclipse-3.1 \
  /bin          -rw,nosuid,grpid          fileserver:/volumes/vol1/eclipse-3.1/bin.linx86
eclipse \
  /bin          -rw,nosuid,grpid          :/vol/eclipse-3.1/bin

Accessing /vol/eclipse/bin now works flawlessly. But given this map:

jdk-1.5.13 -rw,nosuid,grpid fileserver:/export/stud/vol/jdk-1.5.13/linx86
jdk-1.5    -rw,nosuid,grpid :/vol/jdk-1.5.13

accessing /vol/jdk-1.5 doesn't work always (ie only when /vol/jdk-1.5.13
was accessed first). I changed the patch to the one below, and this
problem is now also fixed.

Ian, can you comment on the sanity of this one? If it's OK, I'd like to
see it pushed upstream. We're building our own kernel anyway now (had
known large-scale linux was so damn difficult I bet I wouldn't have
started in the first plate), but it would be nice to see upstream.


-- 
Lukas


diff -ru linux-2.6-2.6.23-a/fs/autofs4/root.c linux-2.6-2.6.23/fs/autofs4/root.c
--- linux-2.6-2.6.23-a/fs/autofs4/root.c        2007-11-30 16:20:33.000000000 +0100
+++ linux-2.6-2.6.23/fs/autofs4/root.c  2007-11-30 16:22:24.000000000 +0100
@@ -19,6 +19,9 @@
 #include <linux/time.h>
 #include "autofs_i.h"

+#define DIRECT_TRIGGER_FLAGS (LOOKUP_CONTINUE|LOOKUP_DIRECTORY|LOOKUP_ACCESS)
+
+
 static int autofs4_dir_symlink(struct inode *,struct dentry *,const char *);
 static int autofs4_dir_unlink(struct inode *,struct dentry *);
 static int autofs4_dir_rmdir(struct inode *,struct dentry *);
@@ -291,7 +294,7 @@
                        return status;
                }
        /* Trigger mount for path component or follow link */
-       } else if (flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY) ||
+       } else if (flags & DIRECT_TRIGGER_FLAGS ||
                        current->link_count) {
                DPRINTK("waiting for mount name=%.*s",
                        dentry->d_name.len, dentry->d_name.name);
@@ -335,7 +338,7 @@
                nd->flags);

        /* If it's our master or we shouldn't trigger a mount we're done */
-       lookup_type = nd->flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY);
+       lookup_type = nd->flags & DIRECT_TRIGGER_FLAGS;
        if (oz_mode || !lookup_type)
                goto done;

Re: Kernel patch to better support bind-mounts

[Ian Kent]

On Tue, 2008-01-08 at 11:38 +0100, Lukas Kolbe wrote:
> Hi!
> 
> Ian, you made a patch for our problem here late laste year that didn't
> quite work for all of our cases. I don't really understand the code in
> the kernel module, but logic tells me that this patch should be safe.
> Plus, it's solving our problems and is straightforward. Your patch was:

Sure, but you'll need to give me a while to re-familiarize myself with
the history and think about it for a while.

The only real issue here is the question of whether we will trigger
mounts when they shouldn't be. Which can lead to mount storms in
unexpected ways, but then I seem to remember that this change was needed
due to changes in the way the kernel does it's lookups.

> 
> diff -up linux-2.6.21/fs/autofs4/root.c.lookup_access-intent linux-2.6.21/fs/autofs4/root.c
> --- linux-2.6.21/fs/autofs4/root.c.lookup_access-intent 2007-10-11 16:32:07.000000000 +0800
> +++ linux-2.6.21/fs/autofs4/root.c      2007-10-11 16:34:59.000000000 +0800
> @@ -20,6 +20,8 @@
>  #include <linux/smp_lock.h>
>  #include "autofs_i.h"
>  
> +#define DIRECT_TRIGGER_FLAGS (LOOKUP_CONTINUE|LOOKUP_DIRECTORY|LOOKUP_ACCESS)
> +
>  static int autofs4_dir_symlink(struct inode *,struct dentry *,const char *);
>  static int autofs4_dir_unlink(struct inode *,struct dentry *);
>  static int autofs4_dir_rmdir(struct inode *,struct dentry *);
> @@ -336,7 +338,7 @@ static void *autofs4_follow_link(struct 
>                 nd->flags);
>  
>         /* If it's our master or we shouldn't trigger a mount we're done */
> -       lookup_type = nd->flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY);
> +       lookup_type = nd->flags & DIRECT_TRIGGER_FLAGS;
>         if (oz_mode || !lookup_type)
>                 goto done;
> 
> 
> This helps for a map like this:
> 
> eclipse-3.1 \
>   /bin          -rw,nosuid,grpid          fileserver:/volumes/vol1/eclipse-3.1/bin.linx86
> eclipse \
>   /bin          -rw,nosuid,grpid          :/vol/eclipse-3.1/bin

Mounts for these multiple offsets are triggered in the same way as
direct mounts, they are triggered when autofs4_follow_link is called by
the VFS.

> 
> Accessing /vol/eclipse/bin now works flawlessly. But given this map:
> 
> jdk-1.5.13 -rw,nosuid,grpid fileserver:/export/stud/vol/jdk-1.5.13/linx86
> jdk-1.5    -rw,nosuid,grpid :/vol/jdk-1.5.13

But these are indirect mounts and they are triggered by calling either
autofs4_lookup or autofs4_revalidate and then indirectly
try_to_fill_dentry, which is where you added the change I think.

What kernel version is this patch against?

Ian

Re: Kernel patch to better support bind-mounts

[Lukas Kolbe]

Hi Ian!

> Sure, but you'll need to give me a while to re-familiarize myself with
> the history and think about it for a while.

Of course. This patch was sitting here since before christmas and I only
now came around actually talking about it, so no stress with that :)

> > eclipse-3.1 \
> >   /bin          -rw,nosuid,grpid          fileserver:/volumes/vol1/eclipse-3.1/bin.linx86
> > eclipse \
> >   /bin          -rw,nosuid,grpid          :/vol/eclipse-3.1/bin
> 
> Mounts for these multiple offsets are triggered in the same way as
> direct mounts, they are triggered when autofs4_follow_link is called by
> the VFS.
> 
> > 
> > Accessing /vol/eclipse/bin now works flawlessly. But given this map:
> > 
> > jdk-1.5.13 -rw,nosuid,grpid fileserver:/export/stud/vol/jdk-1.5.13/linx86
> > jdk-1.5    -rw,nosuid,grpid :/vol/jdk-1.5.13
> 
> But these are indirect mounts and they are triggered by calling either
> autofs4_lookup or autofs4_revalidate and then indirectly
> try_to_fill_dentry, which is where you added the change I think.

Ok, thanks, then I understand a bit better what's actually happening
there.

> What kernel version is this patch against?

I tested it (IIRC) against 2.6.18 (from debian) and 2.6.23 (from
kernel.org git) - autofs-code doesn't seem to have changed much.

> Ian


-- 
Lukas

Re: Kernel patch to better support bind-mounts

[Ian Kent]

On Tue, 2008-01-08 at 15:41 +0100, Lukas Kolbe wrote:
> > What kernel version is this patch against?
> 
> I tested it (IIRC) against 2.6.18 (from debian) and 2.6.23 (from
> kernel.org git) - autofs-code doesn't seem to have changed much.

True, just a couple of bug fixes, I'll look again at the kernel bits the
patch relates to as well.

Ian

Re: Kernel patch to better support bind-mounts

[Lukas Kolbe]

Hi Ian!

Ping? :)

We use this patch now for several servers and clients (debian etch with
2.6.18, ubuntu feisty with 2.6.20 and some test systems with 2.6.23),
and have seen no regression so far, only working automounts :)
I'd love to see this change upstream, so that I don't have to worry
about constantly patching my own kernel anymore :)

-- 
Lukas


diff -ru linux-2.6-2.6.23-a/fs/autofs4/root.c linux-2.6-2.6.23/fs/autofs4/root.c
--- linux-2.6-2.6.23-a/fs/autofs4/root.c        2007-11-30 16:20:33.000000000 +0100
+++ linux-2.6-2.6.23/fs/autofs4/root.c  2007-11-30 16:22:24.000000000 +0100
@@ -19,6 +19,9 @@
 #include <linux/time.h>
 #include "autofs_i.h"

+#define DIRECT_TRIGGER_FLAGS (LOOKUP_CONTINUE|LOOKUP_DIRECTORY|LOOKUP_ACCESS)
+
+
 static int autofs4_dir_symlink(struct inode *,struct dentry *,const char *);
 static int autofs4_dir_unlink(struct inode *,struct dentry *);
 static int autofs4_dir_rmdir(struct inode *,struct dentry *);
@@ -291,7 +294,7 @@
                        return status;
                }
        /* Trigger mount for path component or follow link */
-       } else if (flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY) ||
+       } else if (flags & DIRECT_TRIGGER_FLAGS ||
                        current->link_count) {
                DPRINTK("waiting for mount name=%.*s",
                        dentry->d_name.len, dentry->d_name.name);
@@ -335,7 +338,7 @@
                nd->flags);

        /* If it's our master or we shouldn't trigger a mount we're done */
-       lookup_type = nd->flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY);
+       lookup_type = nd->flags & DIRECT_TRIGGER_FLAGS;
        if (oz_mode || !lookup_type)
                goto done;

Re: Kernel patch to better support bind-mounts

[Peter Staubach]

Lukas Kolbe wrote:
> Hi Ian!
>
> Ping? :)
>
> We use this patch now for several servers and clients (debian etch with
> 2.6.18, ubuntu feisty with 2.6.20 and some test systems with 2.6.23),
> and have seen no regression so far, only working automounts :)
> I'd love to see this change upstream, so that I don't have to worry
> about constantly patching my own kernel anymore :)

Why does adding mount on access(2) make things work better?  What works 
better?

    Thanx...

       ps

Re: Kernel patch to better support bind-mounts

[Ian Kent]

On Tue, 2008-03-04 at 08:25 -0500, Peter Staubach wrote:
> Lukas Kolbe wrote:
> > Hi Ian!
> >
> > Ping? :)
> >
> > We use this patch now for several servers and clients (debian etch with
> > 2.6.18, ubuntu feisty with 2.6.20 and some test systems with 2.6.23),
> > and have seen no regression so far, only working automounts :)
> > I'd love to see this change upstream, so that I don't have to worry
> > about constantly patching my own kernel anymore :)
> 
> Why does adding mount on access(2) make things work better?  What works 
> better?

Lucas has recursive maps.
For these mounts to work, we need to ensure that there aren't dependent
mounts in the mount point path itself before proceeding. We've used the
access system call, made just prior to performing the mount, to make
this happen. In this case, however, the lookup flags in the kernel where
not triggering the mount.

Ian

Re: Kernel patch to better support bind-mounts

[Peter Staubach]

Ian Kent wrote:
> On Tue, 2008-03-04 at 08:25 -0500, Peter Staubach wrote:
>   
>> Lukas Kolbe wrote:
>>     
>>> Hi Ian!
>>>
>>> Ping? :)
>>>
>>> We use this patch now for several servers and clients (debian etch with
>>> 2.6.18, ubuntu feisty with 2.6.20 and some test systems with 2.6.23),
>>> and have seen no regression so far, only working automounts :)
>>> I'd love to see this change upstream, so that I don't have to worry
>>> about constantly patching my own kernel anymore :)
>>>       
>> Why does adding mount on access(2) make things work better?  What works 
>> better?
>>     
>
> Lucas has recursive maps.
> For these mounts to work, we need to ensure that there aren't dependent
> mounts in the mount point path itself before proceeding. We've used the
> access system call, made just prior to performing the mount, to make
> this happen. In this case, however, the lookup flags in the kernel where
> not triggering the mount.

And this does not trigger mounts by other applications, such as ls, too?

    Thanx...

       ps

Re: Kernel patch to better support bind-mounts

[Lukas Kolbe]

Hi Peter!

>> Lucas has recursive maps.
> > For these mounts to work, we need to ensure that there aren't dependent
> > mounts in the mount point path itself before proceeding. We've used the
> > access system call, made just prior to performing the mount, to make
> > this happen. In this case, however, the lookup flags in the kernel where
> > not triggering the mount.
> 
> And this does not trigger mounts by other applications, such as ls, too?

Nope. If I only 'ls -la' my /vol/ or /homes, nothing gets mounted at
all.

>     Thanx...
> 
>        ps

-- 
Lukas

Re: Kernel patch to better support bind-mounts

[Peter Staubach]

Lukas Kolbe wrote:
> Hi Peter!
>
>   
>>> Lucas has recursive maps.
>>> For these mounts to work, we need to ensure that there aren't dependent
>>> mounts in the mount point path itself before proceeding. We've used the
>>> access system call, made just prior to performing the mount, to make
>>> this happen. In this case, however, the lookup flags in the kernel where
>>> not triggering the mount.
>>>       
>> And this does not trigger mounts by other applications, such as ls, too?
>>     
>
> Nope. If I only 'ls -la' my /vol/ or /homes, nothing gets mounted at
> all.
>
>   

Cool.  Thanx!

       ps

>>     Thanx...
>>
>>        ps
>>     
>
>   

Re: Kernel patch to better support bind-mounts

[Ian Kent]

On Tue, 2008-03-04 at 13:47 +0100, Lukas Kolbe wrote:
> Hi Ian!
> 
> Ping? :)
> 
> We use this patch now for several servers and clients (debian etch with
> 2.6.18, ubuntu feisty with 2.6.20 and some test systems with 2.6.23),
> and have seen no regression so far, only working automounts :)
> I'd love to see this change upstream, so that I don't have to worry
> about constantly patching my own kernel anymore :)

Sorry, I'll get to it.
Ian