vfs intent lookup patch

RFC In review Peter Braam 2 participants 3 messages Oct 14, 2002 – Oct 16, 2002 VFS

lore

Share & tools

lore

Peter Braam

author

2002-10-14 22:53 UTC #0 lore raw

RFC 1/1 vfs intent lookup patch

Hi Al,

Attached is a 2.4 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.

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.

We would like to submit something like this for 2.5, for use with Lustre and InterMezzo.

The 2.5 patch is similar, but in 2.5 the struct intent is conveniently embedded in the struct nameidata making things quite a bit simpler. Intent_release is done as part of releasing the nameidata and still requires a new dentry method. We will send you this patch in a few days time.

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.

Best wishes,

- Peter -

include/linux/dcache.h +31 -0

@@ -6,6 +6,34 @@
 #include <asm/atomic.h>
 #include <linux/mount.h>
 
+#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)
+
+struct lookup_intent {
+	int it_op;
+	int it_mode;
+	int it_disposition;
+	int it_status;
+	struct iattr *it_iattr;
+	__u64 it_lock_handle[2];
+	int it_lock_mode;
+	void *it_data;
+};
+
 /*
  * linux/include/linux/dcache.h
  *

@@ -79,6 +107,7 @@
 	unsigned long d_time;		/* used by d_revalidate */
 	struct dentry_operations  *d_op;
 	struct super_block * d_sb;	/* The root of the dentry tree */
+	struct lookup_intent *d_it;
 	unsigned long d_vfs_flags;
 	void * d_fsdata;		/* fs-specific data */
 	void * d_extra_attributes;	/* TUX-specific data */

@@ -92,6 +121,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 +9 -1

@@ -572,6 +572,7 @@
 
 	/* needed for tty driver, and maybe others */
 	void			*private_data;
+	struct lookup_intent    *f_intent;
 
 	/* preallocated helper kiobuf to speedup O_DIRECT */
 	struct kiobuf		*f_iobuf;

@@ -829,7 +830,9 @@
 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 *);
+int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
+		struct inode *new_dir, struct dentry *new_dentry,
+		struct lookup_intent *it);
 
 /*
  * File types

@@ -890,6 +893,7 @@
 struct inode_operations {
 	int (*create) (struct inode *,struct dentry *,int);
 	struct dentry * (*lookup) (struct inode *,struct dentry *);
+	struct dentry * (*lookup2) (struct inode *,struct dentry *, 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 *);

@@ -1036,6 +1040,7 @@
 extern struct vfsmount *kern_mount(struct file_system_type *);
 extern int may_umount(struct vfsmount *);
 extern long do_mount(char *, char *, char *, unsigned long, void *);
+struct vfsmount *do_kern_mount(const char *type, int flags, char *name, void *data);
 extern void umount_tree(struct vfsmount *);
 
 #define kern_umount mntput

@@ -1370,6 +1375,7 @@
 extern loff_t default_llseek(struct file *file, loff_t offset, int origin);
 
 extern int FASTCALL(__user_walk(const char *, unsigned, struct nameidata *));
+extern int FASTCALL(__user_walk_it(const char *, unsigned, struct nameidata *, struct lookup_intent *it));
 extern int FASTCALL(path_init(const char *, unsigned, struct nameidata *));
 extern int FASTCALL(path_walk(const char *, struct nameidata *));
 extern int FASTCALL(path_lookup(const char *, unsigned, struct nameidata *));

@@ -1381,6 +1387,8 @@
 extern struct dentry * lookup_hash(struct qstr *, struct dentry *);
 #define user_path_walk(name,nd)	 __user_walk(name, LOOKUP_FOLLOW|LOOKUP_POSITIVE, nd)
 #define user_path_walk_link(name,nd) __user_walk(name, LOOKUP_POSITIVE, nd)
+#define user_path_walk_it(name,nd,it)  __user_walk_it(name, LOOKUP_FOLLOW|LOOKUP_POSITIVE, nd, it)
+#define user_path_walk_link_it(name,nd,it) __user_walk_it(name, LOOKUP_POSITIVE, nd, it)
 
 extern void iput(struct inode *);
 extern void force_delete(struct inode *);

fs/dcache.c +1 -0

@@ -645,6 +645,7 @@
 	dentry->d_fsdata = NULL;
 	dentry->d_extra_attributes = NULL;
 	dentry->d_mounted = 0;
+	dentry->d_it = NULL;
 	INIT_LIST_HEAD(&dentry->d_hash);
 	INIT_LIST_HEAD(&dentry->d_lru);
 	INIT_LIST_HEAD(&dentry->d_subdirs);

fs/namei.c +149 ~~-33~~

521 lines not on this page · raw message

fs/open.c +53 ~~-16~~

228 lines not on this page · raw message

fs/stat.c +7 -2

@@ -13,6 +13,7 @@
 
 #include <asm/uaccess.h>
 
+extern void intent_release(struct dentry *de, struct lookup_intent *it);
 /*
  * Revalidate the inode. This is required for proper NFS attribute caching.
  */

@@ -104,10 +105,12 @@
 {
 	struct nameidata nd;
 	int error;
+	struct lookup_intent it = { .it_op = IT_GETATTR };
 
-	error = user_path_walk(name, &nd);
+	error = user_path_walk_it(name, &nd, &it);
 	if (!error) {
 		error = do_getattr(nd.mnt, nd.dentry, stat);
+		intent_release(nd.dentry, &it);
 		path_release(&nd);
 	}
 	return error;

@@ -117,10 +120,12 @@
 {
 	struct nameidata nd;
 	int error;
+	struct lookup_intent it = { .it_op = IT_GETATTR };
 
-	error = user_path_walk_link(name, &nd);
+	error = user_path_walk_link_it(name, &nd, &it);
 	if (!error) {
 		error = do_getattr(nd.mnt, nd.dentry, stat);
+		intent_release(nd.dentry, &it);
 		path_release(&nd);
 	}
 	return error;

William A.(Andy) Adamson

2002-10-15 15:58 UTC #1 lore raw

hi peter

i've looked over your patch WRT NFSv4 open semantics. for regular files, the NFSv4 open combines lookup, open, and create. this means that we need both the mode and the open flags to satisfy the OPEN operation, which we would implement in the nfsv4 version of lookup2. as i follow the code path in your patch:

filp_open -> open_namei_it which receives both the mode and the open flags. but then the open flags are lost in the call to path_lookup_it, which translates the open flags into lookup_flags. so, if i'm reading the code correctly, NFSv4 would require another field in the intent struct to hold the open flags.

-->Andy

Quoted 39 lines

Hi Al,
Attached is a 2.4 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.
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.
We would like to submit something like this for 2.5, for use with Lustre and InterMezzo.
The 2.5 patch is similar, but in 2.5 the struct intent is conveniently embedded in the struct nameidata making things quite a bit simpler. Intent_release is done as part of releasing the nameidata and still requires a new dentry method. We will send you this patch in a few days time.
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.
Best wishes,
- Peter -

Peter Braam

author

2002-10-16 21:30 UTC #2 lore raw

Ah yes, I thought that might be useful. Similarly if you wanted to do setattr with an intent you'd need a pointer to iattr's to do so.

- Peter -

Quoted 8 lines · William A.(Andy) Adamson

hi peter
i've looked over your patch WRT NFSv4 open semantics. for regular files, the NFSv4 open combines lookup, open, and create. this means that we need both the mode and the open flags to satisfy the OPEN operation, which we would implement in the nfsv4 version of lookup2. as i follow the code path in your patch:

Quoted 48 lines

filp_open -> open_namei_it which receives both the mode and the open flags. but then the open flags are lost in the call to path_lookup_it, which translates the open flags into lookup_flags. so, if i'm reading the code correctly, NFSv4 would require another field in the intent struct to hold the open flags.
-->Andy
Quoted 39 lines
Hi Al,
Attached is a 2.4 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.
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.
We would like to submit something like this for 2.5, for use with Lustre and InterMezzo.
The 2.5 patch is similar, but in 2.5 the struct intent is conveniently embedded in the struct nameidata making things quite a bit simpler. Intent_release is done as part of releasing the nameidata and still requires a new dentry method. We will send you this patch in a few days time.
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.
Best wishes,
- Peter -

- Peter -