Add a dentry op to handle automounting rather than abusing follow_link
David Howells
maintainerRFC PATCH 1/1 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.
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.
AFS is made to use this facility so that it can be tested. Other filesystems abusing the follow_mount() inode operation will also need to be modified.
---
Documentation/filesystems/Locking | 2 + Documentation/filesystems/vfs.txt | 13 +++++ fs/afs/dir.c | 12 ++++- fs/afs/internal.h | 1 fs/afs/mntpt.c | 45 +++++------------ fs/namei.c | 96 +++++++++++++++++++++++++------------ include/linux/dcache.h | 7 +++ 7 files changed, 112 insertions(+), 64 deletions(-)
Documentation/filesystems/Locking +2 -0
@@ -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:
Documentation/filesystems/vfs.txt +13 -0
@@ -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. + + The presence of a non-NULL d_automount for a dentry is taken to + indicate that the directory is an automount point. + Example : static char *pipefs_dname(struct dentry *dent, char *buffer, int buflen)
fs/afs/dir.c +11 -1
@@ -67,6 +67,13 @@ static const struct dentry_operations afs_fs_dentry_operations = { .d_release = afs_d_release, }; +static const struct dentry_operations afs_mntpt_dentry_operations = { + .d_revalidate = afs_d_revalidate, + .d_delete = afs_d_delete, + .d_release = afs_d_release, + .d_automount = afs_mntpt_d_automount, +}; +#define AFS_DIR_HASHTBL_SIZE 128#define AFS_DIR_DIRENT_SIZE 32#define AFS_DIRENT_PER_BLOCK 64
@@ -539,7 +546,10 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,return ERR_CAST (inode); } - dentry->d_op = &afs_fs_dentry_operations; +if (test_bit (AFS_VNODE_MOUNTPOINT , &AFS_FS_I (inode)->flags)) + dentry->d_op = &afs_mntpt_dentry_operations; +else + dentry->d_op = &afs_fs_dentry_operations;d_add (dentry, inode); _leave(" = 0 { vn=%u u=%u } -> { ino=%lu v=%u }",
fs/afs/internal.h +1 -0
@@ -583,6 +583,7 @@ extern int afs_abort_to_error(u32);extern const struct inode_operations afs_mntpt_inode_operations;extern const struct file_operations afs_mntpt_file_operations; +extern struct vfsmount *afs_mntpt_d_automount (struct path *);extern int afs_mntpt_check_symlink (struct afs_vnode *,struct key *);extern void afs_mntpt_kill_timer (void );
fs/afs/mntpt.c +14 -31
80 lines not on this page · raw message
fs/namei.c +64 -32
145 lines not on this page · raw message
include/linux/dcache.h +7 -0
@@ -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 nod_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,11 @@ static inline int d_mountpoint(struct dentry *dentry)return dentry->d_mounted; } +static inline bool d_automount_point (struct dentry *dentry) +{ +return !!dentry->d_op && dentry->d_op->d_automount; +} +extern struct vfsmount *lookup_mnt (struct path *);extern struct dentry *lookup_create (struct nameidata *nd,int is_dir);
David Howells
maintainerRFC PATCH 1/1 xstat: Add an AT_NO_AUTOMOUNT flag to suppress terminal automount
[Based on the patch to add an automount dentry op]
Add an AT_NO_AUTOMOUNT flag to suppress terminal automounting of directories with follow_link semantics. This can be used by fstatat()/xstat() users to permit the gathering of attributes on an automount point and also prevent mass-automounting of a directory of automount points by ls.
---
fs/namei.c | 15 ++++++++++----- fs/stat.c | 4 +++- include/linux/fcntl.h | 1 + include/linux/namei.h | 2 ++ 4 files changed, 16 insertions(+), 6 deletions(-)
fs/namei.c +10 -5
@@ -648,7 +648,8 @@ static int follow_automount(struct path *path, int res)/* no need for dcache_lock, as serialization is taken care in * namespace.c */ -static int __follow_mount (struct path *path,unsigned nofollow) +static int __follow_mount (struct path *path,unsigned nofollow, +struct nameidata *nd) {struct vfsmount *mounted;int ret, res =0 ;
@@ -668,8 +669,12 @@ static int __follow_mount(struct path *path, unsigned nofollow) }if (!d_automount_point (path->dentry))break ; -if (nofollow) -return -ELOOP ; +if (!(nd->flags &LOOKUP_CONTINUE )) { +if (nofollow) +return -ELOOP ; +if (nd->flags &LOOKUP_NO_AUTOMOUNT ) +break ; + } ret =follow_automount (path, res);if (ret <0 )return ret;
@@ -763,7 +768,7 @@ static int do_lookup(struct nameidata *nd, struct qstr *name, done: path->mnt = mnt; path->dentry = dentry; - ret =__follow_mount (path,0 ); + ret =__follow_mount (path,0 , nd);if (unlikely (ret <0 ))path_put (path);return ret;
@@ -1756,7 +1761,7 @@ static struct file *do_last(struct nameidata *nd, struct path *path,if (open_flag &O_EXCL ) goto exit_dput; - error =__follow_mount (path, open_flag &O_NOFOLLOW ); + error =__follow_mount (path, open_flag &O_NOFOLLOW , nd);if (error <0 ) goto exit_dput;
fs/stat.c +3 -1
@@ -180,11 +180,13 @@ int vfs_xstat(int dfd, const char __user *filename, int flags,struct path path;int error, lookup_flags; -if (flags & ~(AT_SYMLINK_NOFOLLOW |KSTAT_QUERY_FLAGS )) +if (flags & ~(AT_SYMLINK_NOFOLLOW |AT_NO_AUTOMOUNT | +KSTAT_QUERY_FLAGS ))return -EINVAL ; stat->query_flags = flags &KSTAT_QUERY_FLAGS ; lookup_flags = (flags &AT_SYMLINK_NOFOLLOW ) ?0 :LOOKUP_FOLLOW ; + lookup_flags |= (flags &AT_NO_AUTOMOUNT ) ?LOOKUP_NO_AUTOMOUNT :0 ; error =user_path_at (dfd, filename, lookup_flags, &path);if (!error) {
include/linux/fcntl.h +1 -0
@@ -46,6 +46,7 @@unlinking file. */ #define AT_SYMLINK_FOLLOW 0x400/* Follow symbolic links. */ #define AT_FORCE_ATTR_SYNC 0x800/* Force the attributes to be sync'd with the server */ +#define AT_NO_AUTOMOUNT 0x1000/* Suppress terminal automount traversal */ #ifdef __KERNEL__
include/linux/namei.h +2 -0
@@ -43,12 +43,14 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};* - internal "there are more path components" flag * - locked when lookup done with dcache_lock held * - dentry cache is untrusted; force a real lookup +* - suppress terminal automount */ #define LOOKUP_FOLLOW 1#define LOOKUP_DIRECTORY 2#define LOOKUP_CONTINUE 4#define LOOKUP_PARENT 16#define LOOKUP_REVAL 64 +#define LOOKUP_NO_AUTOMOUNT 128/* * Intent data */
This hasn't had much comments yet.
Quoted 11 lines · David Howells
Add a dentry op (
d_automount) to handle automounting directories rather than abusing thefollow_link()inode operation.I've only changed
__follow_mount()to handle automount points, but it might be necessary to changefollow_mount()too. The latter is only used fromfollow_dotdot(), but any automounts on ".." should be pinned whilst we're using a child of it.AFS is made to use this facility so that it can be tested. Other filesystems abusing the
follow_mount()inode operation will also need to be modified.
How about having a .follow_mount op, and using that instead of default follow_mount in case mounted is incremented?
Also I would prefer the patch to add this call keep basically the same API as follow_mount, so if you are going to change that to return an error and do the NOFOLLOW handling in there, then could you do that first, as a more trivial patch?
Then your addition of the d_op should not touch outside *follow_mount.
David Howells
maintainerQuoted 3 lines · Nick Piggin
Quoted 3 lines
AFS is made to use this facility so that it can be tested. Other filesystems abusing the
follow_mount()inode operation will also need to be modified.
I meant follow_link() here of course... Too many followy things:-)
Quoted 2 lines
How about having a .follow_mount op, and using that instead of default
follow_mountin case mounted is incremented?
But what if d_mounted is not incremented, though? That's usually the point you'd want to call the automount code. Why would you want to call into the filesystem just to skip over possibly mounted dentries? A dentry may have an elevated d_mount on it, but nothing mounted at that {vfsmount,dentry} point I suppose, but still jumping into the filesystem just so it can skip an already mounted point would seem a waste of time.
Quoted 1 line
Also I would prefer the patch to add this call
Meaning i_op->follow_mount()?
Quoted 3 lines
keep basically the same API as
follow_mount, so if you are going to change that to return an error and do the NOFOLLOW handling in there, then could you do that first, as a more trivial patch?
Ummm... I'm not sure I follow you. I changed __follow_mount() not follow_mount(). I don't think changing the latter is necessary.
Quoted 1 line
Then your addition of the
d_opshould not touch outside*follow_mount.
But calling i_op->follow_mount() would, so what does this gain you? And why not touch the inside of __follow_mount()?
Are you suggesting doing i_op->follow_mount() instead of or as well as d_op->d_automount()? I'm not entirely sure.
David
Quoted 6 lines · David Howells
Quoted 3 lines · Nick Piggin
Quoted 3 lines
AFS is made to use this facility so that it can be tested. Other filesystems abusing the
follow_mount()inode operation will also need to be modified.
Quoted 6 lines
I meant
follow_link()here of course... Too many followy things:-)Quoted 2 lines
How about having a .follow_mount op, and using that instead of default
follow_mountin case mounted is incremented?But what if
d_mountedis not incremented, though?
Nothing?
Quoted 2 lines
That's usually the point
you'd want to call the automount code.
I think you have it the wrong way around. If you wanted to call the automount code, you would have incremented d_mounted.
Quoted 5 lines
Why would you want to call into the
filesystem just to skip over possibly mounted dentries? A dentry may have an elevatedd_mounton it, but nothing mounted at that {vfsmount,dentry} point I suppose, but still jumping into the filesystem just so it can skip an already mounted point would seem a waste of time.
Those that don't care wouldn't set ->follow_mount though. Following a mount is a fairly heavy operation already, it does take a global lock (before vfs scalability patches, anyway).
I like the flexibility of doing one's own ->follow_mount, although Al might object to allowing filesystems to follow mounts in ways that are not published to the core namespace structures.
Quoted 3 lines
Quoted 1 line
Also I would prefer the patch to add this call
Meaning
i_op->follow_mount()?
Either one, just make the follow_mount/__follow_mount API changes in one patch, and add the callback in another.
Quoted 6 lines
Quoted 3 lines
keep basically the same API as
follow_mount, so if you are going to change that to return an error and do the NOFOLLOW handling in there, then could you do that first, as a more trivial patch?Ummm... I'm not sure I follow you. I changed
__follow_mount()notfollow_mount(). I don't think changing the latter is necessary.
I meant __follow_mount.
Quoted 7 lines
Quoted 1 line
Then your addition of the
d_opshould not touch outside*follow_mount.But calling
i_op->follow_mount()would, so what does this gain you? And why not touch the inside of__follow_mount()?Are you suggesting doing
i_op->follow_mount()instead of or as well asd_op->d_automount()? I'm not entirely sure.
Two suggestions. Firstly a d_op->d_follow_mount() (does following a mount even make sense at the inode level?)
Secondly, just simply to split the patch so you change the __follow_mount API in namespace first.
David Howells
maintainerQuoted 2 lines · Nick Piggin
I think you have it the wrong way around. If you wanted to call the automount code, you would have incremented
d_mounted.
Why? d_mounted indicates how many things are mounted on a dentry, doesn't it? Before the automounter is invoked there isn't anything mounted there.
Quoted 3 lines
Those that don't care wouldn't set ->follow_mount though. Following a mount is a fairly heavy operation already, it does take a global lock (before vfs scalability patches, anyway).
I wonder if we could do it with a lock on vfsmount instead and use mnt_mounts to find it.
Quoted 3 lines
I like the flexibility of doing one's own ->follow_mount, although Al might object to allowing filesystems to follow mounts in ways that are not published to the core namespace structures.
But why would you want to delegate mountpoint traversal to the filesystem?
David
David Howells
maintainerQuoted 1 line · David Howells
But why would you want to delegate mountpoint traversal to the filesystem?
s/mountpoint traversal/mounted filesystem iteration/
David