Skip to contentSkip to conversation

2.5 vfs intent lookup patch

PATCH/RFC Peter Braam 2 participants 2 messages Oct 25, 2002

Subsystems (5)
Networking VFS NFS server exec and binfmt +1 more subsystems
lore
Share & tools
lore

Peter Braam

author
#0 lore raw

Hi Al,

Attached is a 2.5 patch for lookup intents, which we use with Lustre. This patch adds a lookup2, revalidate2 and intent_release method. None of those are required if the kernel changes the parameters globally, but I wanted to keep things simple.

We got some feedback last week and incorporated that.

The purpose of the patch is to give the lookup call sufficient information about the "intent" of the lookup. Intents can be tags like "a create/open is coming". With the intent patch, distributed O_EXCL problems, from which NFS suffers too, are easily addressed. For Lustre, which runs on 1,000+ node clusters, reducing file system transactions to a single RPC from ->lookup is the core benefit. For InterMezzo the intent opens the opportunity to refresh the cache before the ->create/mkdir etc methods lock anything, which is very beneficial. This is much along the lines of what we discussed over the last year and a bit.

Another way to look at it is that it enables one to re-order the execution slightly:

(client, namei) --> (server, namei)
(client, vfs_xxx) --> (server, vfs_xxx)

changes into:

(client, namei) --> (server, namei ; vfs_xxx)
(client, vfs_xxx)

I think this functionality does require VFS changes, but quite clearly it does not affect file systems that pay no attention to the intent.

We hope that something like this can be incorporated in 2.5, for use with Lustre and InterMezzo.

However, I suspect that after you review this you might come up with an equivalent but better way of doing this. Particularly interesting to us is how the d_it field we added to the dentry can be protected between the return from lookup and the use of that field in ->create/mkdir etc. We do that with a semaphore (which we placed under d_fsdata); alternatives are passing the nameidata or intent to every inode operation that requires the intent.

Do you have a good idea how to do this better?

Best wishes,

- Peter -

fs/driverfs/inode.c +1 -1
diff -Nru a/fs/driverfs/inode.c b/fs/driverfs/inode.c
@@ -521,7 +521,7 @@
 	qstr.name = name;
 	qstr.len = strlen(name);
 	qstr.hash = full_name_hash(name,qstr.len);
-	return lookup_hash(&qstr,parent);
+	return lookup_hash(&qstr,parent, NULL);
 }
 
 /**
fs/exec.c +7 -2
diff -Nru a/fs/exec.c b/fs/exec.c
@@ -113,6 +113,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_OPEN, 0, NULL);
 	error = user_path_walk(library, &nd);
 	if (error)
 		goto out;

@@ -421,8 +422,12 @@
 struct file *open_exec(const char *name)
 {
 	struct nameidata nd;
-	int err = path_lookup(name, LOOKUP_FOLLOW, &nd);
-	struct file *file = ERR_PTR(err);
+	int err;
+	struct file *file;
+
+        intent_init(&nd, IT_OPEN, 0, NULL);
+        err = path_lookup(name, LOOKUP_FOLLOW, &nd);
+        file = ERR_PTR(err);
 
 	if (!err) {
 		struct inode *inode = nd.dentry->d_inode;
fs/namei.c +62 -20

275 lines not on this page · raw message

fs/namespace.c +8 -0
diff -Nru a/fs/namespace.c b/fs/namespace.c
@@ -1,3 +1,5 @@
+
+
 /*
  *  linux/fs/namespace.c
  *

@@ -370,6 +372,7 @@
 	struct nameidata nd;
 	int retval;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	retval = __user_walk(name, LOOKUP_FOLLOW, &nd);
 	if (retval)
 		goto out;

@@ -503,6 +506,7 @@
 		return err;
 	if (!old_name || !*old_name)
 		return -EINVAL;
+        intent_init(&old_nd, IT_GETATTR, 0, NULL);
 	err = path_lookup(old_name, LOOKUP_FOLLOW, &old_nd);
 	if (err)
 		return err;

@@ -571,6 +575,8 @@
 		return -EPERM;
 	if (!old_name || !*old_name)
 		return -EINVAL;
+
+        intent_init(&old_nd, IT_GETATTR, 0, NULL);
 	err = path_lookup(old_name, LOOKUP_FOLLOW, &old_nd);
 	if (err)
 		return err;

@@ -928,6 +934,7 @@
 
 	lock_kernel();
 
+        intent_init(&new_nd, IT_GETATTR, 0, NULL);
 	error = __user_walk(new_root, LOOKUP_FOLLOW|LOOKUP_DIRECTORY, &new_nd);
 	if (error)
 		goto out0;

@@ -935,6 +942,7 @@
 	if (!check_mnt(new_nd.mnt))
 		goto out1;
 
+        intent_init(&old_nd, IT_GETATTR, 0, NULL);
 	error = __user_walk(put_old, LOOKUP_FOLLOW|LOOKUP_DIRECTORY, &old_nd);
 	if (error)
 		goto out1;
fs/nfsd/vfs.c +1 -1
diff -Nru a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
@@ -1282,7 +1282,7 @@
 			err = nfserr_perm;
 	} else
 #endif
-	err = vfs_rename(fdir, odentry, tdir, ndentry);
+	err = vfs_rename(fdir, odentry, tdir, ndentry, NULL);
 	if (!err && EX_ISSYNC(tfhp->fh_export)) {
 		nfsd_sync_dir(tdentry);
 		nfsd_sync_dir(fdentry);
fs/open.c +11 -0
diff -Nru a/fs/open.c b/fs/open.c
@@ -46,6 +46,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_STATFS, 0, buf);
 	error = user_path_walk(path, &nd);
 	if (!error) {
 		struct statfs tmp;

@@ -102,6 +103,7 @@
 	if (length < 0)	/* sorry, but loff_t says... */
 		goto out;
 
+        intent_init(&nd, IT_SETATTR, 0, NULL);
 	error = user_path_walk(path, &nd);
 	if (error)
 		goto out;

@@ -239,6 +241,7 @@
 	struct inode * inode;
 	struct iattr newattrs;
 
+        intent_init(&nd, IT_SETATTR, 0, NULL);
 	error = user_path_walk(filename, &nd);
 	if (error)
 		goto out;

@@ -285,6 +288,7 @@
 	struct inode * inode;
 	struct iattr newattrs;
 
+        intent_init(&nd, IT_SETATTR, 0, NULL);
 	error = user_path_walk(filename, &nd);
 
 	if (error)

@@ -354,6 +358,7 @@
 	else
 		current->cap_effective = current->cap_permitted;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	res = user_path_walk(filename, &nd);
 	if (!res) {
 		res = permission(nd.dentry->d_inode, mode);

@@ -376,6 +381,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	error = __user_walk(filename, LOOKUP_FOLLOW|LOOKUP_DIRECTORY, &nd);
 	if (error)
 		goto out;

@@ -427,6 +433,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	error = __user_walk(filename, LOOKUP_FOLLOW | LOOKUP_DIRECTORY | LOOKUP_NOALT, &nd);
 	if (error)
 		goto out;

@@ -490,6 +497,7 @@
 	int error;
 	struct iattr newattrs;
 
+        intent_init(&nd, IT_SETATTR, 0, NULL);
 	error = user_path_walk(filename, &nd);
 	if (error)
 		goto out;

@@ -557,6 +565,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_SETATTR, 0, NULL);
 	error = user_path_walk(filename, &nd);
 	if (!error) {
 		error = chown_common(nd.dentry, user, group);

@@ -570,6 +579,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_SETATTR, 0, NULL);
 	error = user_path_walk_link(filename, &nd);
 	if (!error) {
 		error = chown_common(nd.dentry, user, group);

@@ -617,6 +627,7 @@
 	if (namei_flags & O_TRUNC)
 		namei_flags |= 2;
 
+        intent_init(&nd, IT_OPEN, 0, NULL);
 	error = open_namei(filename, namei_flags, mode, &nd);
 	if (!error)
 		return dentry_open(nd.dentry, nd.mnt, flags);
fs/quota.c +1 -0
diff -Nru a/fs/quota.c b/fs/quota.c
@@ -110,6 +110,7 @@
 	struct block_device *bdev;
 	struct super_block *sb;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	ret = user_path_walk(path, &nd);
 	if (ret)
 		goto out;
fs/stat.c +3 -0
diff -Nru a/fs/stat.c b/fs/stat.c
@@ -62,6 +62,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_GETATTR, 0, stat);
 	error = user_path_walk(name, &nd);
 	if (!error) {
 		error = vfs_getattr(nd.mnt, nd.dentry, stat);

@@ -75,6 +76,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_GETATTR, 0, stat);
 	error = user_path_walk_link(name, &nd);
 	if (!error) {
 		error = vfs_getattr(nd.mnt, nd.dentry, stat);

@@ -232,6 +234,7 @@
 	if (bufsiz <= 0)
 		return -EINVAL;
 
+        intent_init(&nd, IT_READLINK, 0, NULL);
 	error = user_path_walk_link(path, &nd);
 	if (!error) {
 		struct inode * inode = nd.dentry->d_inode;
fs/super.c +1 -0
diff -Nru a/fs/super.c b/fs/super.c
@@ -476,6 +476,7 @@
 	/* What device it is? */
 	if (!dev_name || !*dev_name)
 		return ERR_PTR(-EINVAL);
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	error = path_lookup(dev_name, LOOKUP_FOLLOW, &nd);
 	if (error)
 		return ERR_PTR(error);
fs/xattr.c +8 -0
diff -Nru a/fs/xattr.c b/fs/xattr.c
@@ -105,6 +105,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_SETATTR, 0, NULL);
 	error = user_path_walk(path, &nd);
 	if (error)
 		return error;

@@ -119,6 +120,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_SETATTR, 0, NULL);
 	error = user_path_walk_link(path, &nd);
 	if (error)
 		return error;

@@ -185,6 +187,7 @@
 	struct nameidata nd;
 	ssize_t error;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	error = user_path_walk(path, &nd);
 	if (error)
 		return error;

@@ -199,6 +202,7 @@
 	struct nameidata nd;
 	ssize_t error;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	error = user_path_walk_link(path, &nd);
 	if (error)
 		return error;

@@ -258,6 +262,7 @@
 	struct nameidata nd;
 	ssize_t error;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	error = user_path_walk(path, &nd);
 	if (error)
 		return error;

@@ -272,6 +277,7 @@
 	struct nameidata nd;
 	ssize_t error;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	error = user_path_walk_link(path, &nd);
 	if (error)
 		return error;

@@ -328,6 +334,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	error = user_path_walk(path, &nd);
 	if (error)
 		return error;

@@ -342,6 +349,7 @@
 	struct nameidata nd;
 	int error;
 
+        intent_init(&nd, IT_GETATTR, 0, NULL);
 	error = user_path_walk_link(path, &nd);
 	if (error)
 		return error;
include/linux/dcache.h +24 -0
diff -Nru a/include/linux/dcache.h b/include/linux/dcache.h
@@ -9,6 +9,25 @@
 #include <linux/spinlock.h>
 #include <asm/page.h>			/* for BUG() */
 
+#define IT_OPEN  (1)
+#define IT_CREAT  (1<<1)
+#define IT_MKDIR  (1<<2)
+#define IT_LINK  (1<<3)
+#define IT_LINK2  (1<<4)
+#define IT_SYMLINK  (1<<5)
+#define IT_UNLINK  (1<<6)
+#define IT_RMDIR  (1<<7)
+#define IT_RENAME  (1<<8)
+#define IT_RENAME2  (1<<9)
+#define IT_READDIR  (1<<10)
+#define IT_GETATTR  (1<<11)
+#define IT_SETATTR  (1<<12)
+#define IT_READLINK  (1<<13)
+#define IT_MKNOD  (1<<14)
+#define IT_LOOKUP  (1<<15)
+#define IT_STATFS  (1<<16)
+
+
 /*
  * linux/include/linux/dcache.h
  *

@@ -30,6 +49,8 @@
 	unsigned int hash;
 };
 
+#include <linux/namei.h>
+
 struct dentry_stat_t {
 	int nr_dentry;
 	int nr_unused;

@@ -77,6 +98,7 @@
 	struct list_head d_subdirs;	/* our children */
 	struct list_head d_alias;	/* inode alias list */
 	int d_mounted;
+        struct lookup_intent *d_it;
 	struct qstr d_name;
 	unsigned long d_time;		/* used by d_revalidate */
 	struct dentry_operations  *d_op;

@@ -93,6 +115,8 @@
 	int (*d_delete)(struct dentry *);
 	void (*d_release)(struct dentry *);
 	void (*d_iput)(struct dentry *, struct inode *);
+	int (*d_revalidate2)(struct dentry *, int, struct lookup_intent *);
+	void (*d_intent_release)(struct  dentry *, struct lookup_intent *);
 };
 
 /* the dentry parameter passed to d_hash and d_compare is the parent
include/linux/fs.h +6 -1
diff -Nru a/include/linux/fs.h b/include/linux/fs.h
@@ -1,3 +1,5 @@
+
+
 #ifndef _LINUX_FS_H
 #define _LINUX_FS_H
 

@@ -704,7 +706,7 @@
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
 extern int vfs_rmdir(struct inode *, struct dentry *);
 extern int vfs_unlink(struct inode *, struct dentry *);
-extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);
+extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *, struct lookup_intent *it);
 
 /*
  * File types

@@ -773,6 +775,8 @@
 struct inode_operations {
 	int (*create) (struct inode *,struct dentry *,int);
 	struct dentry * (*lookup) (struct inode *,struct dentry *);
+	struct dentry * (*lookup2) (struct inode *,struct dentry *, 
+                                    int flags, struct lookup_intent *);
 	int (*link) (struct dentry *,struct inode *,struct dentry *);
 	int (*unlink) (struct inode *,struct dentry *);
 	int (*symlink) (struct inode *,struct dentry *,const char *);

@@ -999,6 +1003,7 @@
 extern int unregister_filesystem(struct file_system_type *);
 extern struct vfsmount *kern_mount(struct file_system_type *);
 extern int may_umount(struct vfsmount *);
+struct vfsmount *do_kern_mount(const char *type, int flags, char *name, void *data);
 extern long do_mount(char *, char *, char *, unsigned long, void *);
 
 #define kern_umount mntput
include/linux/namei.h +23 -1
diff -Nru a/include/linux/namei.h b/include/linux/namei.h
@@ -1,3 +1,4 @@
+
 #ifndef _LINUX_NAMEI_H
 #define _LINUX_NAMEI_H
 

@@ -5,6 +6,18 @@
 
 struct vfsmount;
 
+/* for debugging to detect unitialized intents */
+#define INTENT_MAGIC 0x102E02
+
+struct lookup_intent {
+	int it_op;
+        int it_magic;
+	int it_mode;
+	void *it_data;
+        __u8 it_fsdata[80];
+};
+
+
 struct nameidata {
 	struct dentry	*dentry;
 	struct vfsmount *mnt;

@@ -13,8 +26,17 @@
 	int		last_type;
 	struct dentry	*old_dentry;
 	struct vfsmount	*old_mnt;
+        struct lookup_intent it;
 };
 
+static inline void intent_init(struct nameidata *nd, int op, 
+                               int mode, void *data)
+{
+        nd->it.it_op = op;
+        nd->it.it_mode = mode;
+        nd->it.it_data = data;
+        nd->it.it_magic = INTENT_MAGIC;
+}
 /*
  * Type of the last component on LOOKUP_PARENT
  */

@@ -46,7 +68,7 @@
 extern void path_release(struct nameidata *);
 
 extern struct dentry * lookup_one_len(const char *, struct dentry *, int);
-extern struct dentry * lookup_hash(struct qstr *, struct dentry *);
+extern struct dentry * lookup_hash(struct qstr *, struct dentry *, struct lookup_intent *);
 
 extern int follow_down(struct vfsmount **, struct dentry **);
 extern int follow_up(struct vfsmount **, struct dentry **);
kernel/ksyms.c +7 -0
diff -Nru a/kernel/ksyms.c b/kernel/ksyms.c
@@ -361,6 +361,13 @@
 EXPORT_SYMBOL(tty_get_baud_rate);
 EXPORT_SYMBOL(do_SAK);
 
+/* lustre */
+EXPORT_SYMBOL(panic_notifier_list);
+//EXPORT_SYMBOL(pagecache_lock_cacheline);
+EXPORT_SYMBOL(do_kern_mount);
+EXPORT_SYMBOL(exit_files);
+EXPORT_SYMBOL(kmem_cache_validate);
+
 /* filesystem registration */
 EXPORT_SYMBOL(register_filesystem);
 EXPORT_SYMBOL(unregister_filesystem);
net/unix/af_unix.c +1 -1
diff -Nru a/net/unix/af_unix.c b/net/unix/af_unix.c
@@ -715,7 +715,7 @@
 		/*
 		 * Do the final lookup.
 		 */
-		dentry = lookup_hash(&nd.last, nd.dentry);
+		dentry = lookup_hash(&nd.last, nd.dentry, NULL);
 		err = PTR_ERR(dentry);
 		if (IS_ERR(dentry))
 			goto out_mknod_unlock;
#1 lore raw
Quoted 4 lines · Peter Braam

Attached is a 2.5 patch for lookup intents, which we use with Lustre. This patch adds a lookup2, revalidate2 and intent_release method.

Do you have a good idea how to do this better?

At least use better names than `lookup2' &c.

[`lookup_with_intent'?]

-Miles

Signature · 2 lines
Somebody has to do something, and it's just incredibly pathetic that it
has to be us.  -- Jerry Garcia