lkml.org 
[lkml]   [2010]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 1/6] Add a dentry op to handle automounting rather than abusing follow_link() [ver #2]
    From
    Date
    On Fri, 2010-07-23 at 16:09 +0100, David Howells wrote:
    > Here's an updated patch that:
    >
    > (1) Fixes a bug in error handling (needs to use path_put_conditional not
    > path_put).
    >
    > (2) Absorbs autofs4's decisions about whether to automount or not. This
    > means that colour-ls won't cause automounts unless -L is supplied or it
    > doesn't get a DT_DIR flag from getdents(). It also means that autofs4
    > can dispense with this logic should it choose to use d_automount().

    I think my statements about this were a little incorrect.

    In the current kernel the check isn't imposed for ->lookup() but only
    ->d_revalidate() (the deosn't already exist vs the already exists
    cases). Since ->d_lookup() (currently) leaves the dentry negative until
    ->mkdir() that could be used in the check. But then this may be starting
    to get a little too autofs specific, maybe we should re-consider passing
    the flags, I don't know.

    In any case I'll have a go at using this as is, and see what happens.

    >
    > (3) Moves all the automounter logic out of __follow_mount() into
    > follow_automount().
    >
    > (4) Stops pathwalk at the automount point and returns that point in the fs
    > tree if it decides not to automount rather than reporting ELOOP (see its
    > use of EXDEV for this).
    >
    > David
    >
    > ---
    > From: David Howells <dhowells@redhat.com>
    > Subject: [PATCH] Add a dentry op to handle automounting rather than abusing follow_link()
    >
    > Add a dentry op (d_automount) to handle automounting directories rather than
    > abusing the follow_link() inode operation. The operation is keyed off a new
    > inode flag (S_AUTOMOUNT).
    >
    > This makes it easier to add an AT_ flag to suppress terminal segment automount
    > during pathwalk. It should also remove the need for the kludge code in the
    > pathwalk algorithm to handle directories with follow_link() semantics.
    >
    > I've only changed __follow_mount() to handle automount points, but it might be
    > necessary to change follow_mount() too. The latter is only used from
    > follow_dotdot(), but any automounts on ".." should be pinned whilst we're using
    > a child of it.
    >
    > I've also extracted the mount/don't-mount logic from autofs4 and included it
    > here. It makes the mount go ahead anyway if someone calls open() or creat(),
    > tries to traverse the directory, tries to chdir/chroot/etc. into the directory,
    > or sticks a '/' on the end of the pathname. If they do a stat(), however,
    > they'll only trigger the automount if they didn't also say O_NOFOLLOW.
    >
    > Signed-off-by: David Howells <dhowells@redhat.com>
    > ---
    >
    > Documentation/filesystems/Locking | 2 +
    > Documentation/filesystems/vfs.txt | 13 +++++
    > fs/namei.c | 96 ++++++++++++++++++++++++++++++-------
    > include/linux/dcache.h | 5 ++
    > include/linux/fs.h | 2 +
    > 5 files changed, 100 insertions(+), 18 deletions(-)
    >
    >
    > diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
    > index 96d4293..ccbfa98 100644
    > --- a/Documentation/filesystems/Locking
    > +++ b/Documentation/filesystems/Locking
    > @@ -16,6 +16,7 @@ prototypes:
    > void (*d_release)(struct dentry *);
    > void (*d_iput)(struct dentry *, struct inode *);
    > char *(*d_dname)((struct dentry *dentry, char *buffer, int buflen);
    > + struct vfsmount *(*d_automount)(struct path *path);
    >
    > locking rules:
    > none have BKL
    > @@ -27,6 +28,7 @@ d_delete: yes no yes no
    > d_release: no no no yes
    > d_iput: no no no yes
    > d_dname: no no no no
    > +d_automount: no no no yes
    >
    > --------------------------- inode_operations ---------------------------
    > prototypes:
    > diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
    > index 94677e7..31a9e8f 100644
    > --- a/Documentation/filesystems/vfs.txt
    > +++ b/Documentation/filesystems/vfs.txt
    > @@ -851,6 +851,7 @@ struct dentry_operations {
    > void (*d_release)(struct dentry *);
    > void (*d_iput)(struct dentry *, struct inode *);
    > char *(*d_dname)(struct dentry *, char *, int);
    > + struct vfsmount *(*d_automount)(struct path *);
    > };
    >
    > d_revalidate: called when the VFS needs to revalidate a dentry. This
    > @@ -885,6 +886,18 @@ struct dentry_operations {
    > at the end of the buffer, and returns a pointer to the first char.
    > dynamic_dname() helper function is provided to take care of this.
    >
    > + d_automount: called when an automount dentry is to be traversed (optional).
    > + This should create a new VFS mount record, mount it on the directory
    > + and return the record to the caller. The caller is supplied with a
    > + path parameter giving the automount directory to describe the automount
    > + target and the parent VFS mount record to provide inheritable mount
    > + parameters. NULL should be returned if someone else managed to make
    > + the automount first. If the automount failed, then an error code
    > + should be returned.
    > +
    > + This function is only used if S_AUTOMOUNT is set on the inode to which
    > + the dentry refers.
    > +
    > Example :
    >
    > static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
    > diff --git a/fs/namei.c b/fs/namei.c
    > index 868d0cb..6509ca5 100644
    > --- a/fs/namei.c
    > +++ b/fs/namei.c
    > @@ -617,24 +617,82 @@ int follow_up(struct path *path)
    > return 1;
    > }
    >
    > +/*
    > + * Perform an automount
    > + * - return -EXDEV to tell __follow_mount() to stop and return the path we were
    > + * called with.
    > + */
    > +static int follow_automount(struct path *path, unsigned flags, int res)
    > +{
    > + struct vfsmount *mnt;
    > +
    > + if (!path->dentry->d_op || !path->dentry->d_op->d_automount)
    > + return -EREMOTE;
    > +
    > + /* We want to mount if someone is trying to open/create a file of any
    > + * type under the mountpoint, wants to traverse through the mountpoint
    > + * or wants to open the mounted directory.
    > + *
    > + * We don't want to mount if someone's just doing a stat and they've
    > + * set AT_SYMLINK_NOFOLLOW - unless they're stat'ing a directory and
    > + * appended a '/' to the name.
    > + */
    > + if (!(flags & LOOKUP_FOLLOW) &&
    > + !(flags & (LOOKUP_CONTINUE | LOOKUP_DIRECTORY |
    > + LOOKUP_OPEN | LOOKUP_CREATE)))
    > + return -EXDEV;
    > +
    > + current->total_link_count++;
    > + if (current->total_link_count >= 40)
    > + return -ELOOP;
    > +
    > + mnt = path->dentry->d_op->d_automount(path);
    > + if (IS_ERR(mnt))
    > + return PTR_ERR(mnt);
    > + if (!mnt) /* mount collision */
    > + return 0;
    > +
    > + if (mnt->mnt_sb == path->mnt->mnt_sb &&
    > + mnt->mnt_root == path->dentry) {
    > + mntput(mnt);
    > + return -ELOOP;
    > + }
    > +
    > + dput(path->dentry);
    > + if (res)
    > + mntput(path->mnt);
    > + path->mnt = mnt;
    > + path->dentry = dget(mnt->mnt_root);
    > + return 0;
    > +}
    > +
    > /* no need for dcache_lock, as serialization is taken care in
    > * namespace.c
    > */
    > -static int __follow_mount(struct path *path)
    > +static int __follow_mount(struct path *path, unsigned flags)
    > {
    > - int res = 0;
    > - while (d_mountpoint(path->dentry)) {
    > - struct vfsmount *mounted = lookup_mnt(path);
    > - if (!mounted)
    > + struct vfsmount *mounted;
    > + int ret, res = 0;
    > + for (;;) {
    > + while (d_mountpoint(path->dentry)) {
    > + mounted = lookup_mnt(path);
    > + if (!mounted)
    > + break;
    > + dput(path->dentry);
    > + if (res)
    > + mntput(path->mnt);
    > + path->mnt = mounted;
    > + path->dentry = dget(mounted->mnt_root);
    > + res = 1;
    > + }
    > + if (!d_automount_point(path->dentry))
    > break;
    > - dput(path->dentry);
    > - if (res)
    > - mntput(path->mnt);
    > - path->mnt = mounted;
    > - path->dentry = dget(mounted->mnt_root);
    > + ret = follow_automount(path, flags, res);
    > + if (ret < 0)
    > + return ret == -EXDEV ? 0 : ret;
    > res = 1;
    > }
    > - return res;
    > + return 0;
    > }
    >
    > static void follow_mount(struct path *path)
    > @@ -702,6 +760,8 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
    > struct vfsmount *mnt = nd->path.mnt;
    > struct dentry *dentry, *parent;
    > struct inode *dir;
    > + int ret;
    > +
    > /*
    > * See if the low-level filesystem might want
    > * to use its own hash..
    > @@ -720,8 +780,10 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
    > done:
    > path->mnt = mnt;
    > path->dentry = dentry;
    > - __follow_mount(path);
    > - return 0;
    > + ret = __follow_mount(path, nd->flags);
    > + if (unlikely(ret < 0))
    > + path_put_conditional(path, nd);
    > + return ret;
    >
    > need_lookup:
    > parent = nd->path.dentry;
    > @@ -1721,11 +1783,9 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
    > if (open_flag & O_EXCL)
    > goto exit_dput;
    >
    > - if (__follow_mount(path)) {
    > - error = -ELOOP;
    > - if (open_flag & O_NOFOLLOW)
    > - goto exit_dput;
    > - }
    > + error = __follow_mount(path, nd->flags);
    > + if (error < 0)
    > + goto exit_dput;
    >
    > error = -ENOENT;
    > if (!path->dentry->d_inode)
    > diff --git a/include/linux/dcache.h b/include/linux/dcache.h
    > index eebb617..5380bff 100644
    > --- a/include/linux/dcache.h
    > +++ b/include/linux/dcache.h
    > @@ -139,6 +139,7 @@ struct dentry_operations {
    > void (*d_release)(struct dentry *);
    > void (*d_iput)(struct dentry *, struct inode *);
    > char *(*d_dname)(struct dentry *, char *, int);
    > + struct vfsmount *(*d_automount)(struct path *);
    > };
    >
    > /* the dentry parameter passed to d_hash and d_compare is the parent
    > @@ -157,6 +158,7 @@ d_compare: no yes yes no
    > d_delete: no yes no no
    > d_release: no no no yes
    > d_iput: no no no yes
    > +d_automount: no no no yes
    > */
    >
    > /* d_flags entries */
    > @@ -389,6 +391,9 @@ static inline int d_mountpoint(struct dentry *dentry)
    > return dentry->d_mounted;
    > }
    >
    > +#define d_automount_point(dentry) \
    > + (dentry->d_inode && IS_AUTOMOUNT(dentry->d_inode))
    > +
    > extern struct vfsmount *lookup_mnt(struct path *);
    > extern struct dentry *lookup_create(struct nameidata *nd, int is_dir);
    >
    > diff --git a/include/linux/fs.h b/include/linux/fs.h
    > index 68ca1b0..a83fc81 100644
    > --- a/include/linux/fs.h
    > +++ b/include/linux/fs.h
    > @@ -235,6 +235,7 @@ struct inodes_stat_t {
    > #define S_NOCMTIME 128 /* Do not update file c/mtime */
    > #define S_SWAPFILE 256 /* Do not truncate: swapon got its bmaps */
    > #define S_PRIVATE 512 /* Inode is fs-internal */
    > +#define S_AUTOMOUNT 1024 /* Automount/referral quasi-directory */
    >
    > /*
    > * Note that nosuid etc flags are inode-specific: setting some file-system
    > @@ -269,6 +270,7 @@ struct inodes_stat_t {
    > #define IS_NOCMTIME(inode) ((inode)->i_flags & S_NOCMTIME)
    > #define IS_SWAPFILE(inode) ((inode)->i_flags & S_SWAPFILE)
    > #define IS_PRIVATE(inode) ((inode)->i_flags & S_PRIVATE)
    > +#define IS_AUTOMOUNT(inode) ((inode)->i_flags & S_AUTOMOUNT)
    >
    > /* the read-only stuff doesn't really belong here, but any other place is
    > probably as bad and I don't want to create yet another include file. */




    \
     
     \ /
      Last update: 2010-07-24 06:13    [W:0.047 / U:0.604 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site