[patch 0/9] file locking fixes + fuse nfs export support

[patch 0/9] file locking fixes + fuse nfs export support

[Miklos Szeredi]

Patches 1-4 are file locking API changes required for the rest of the
patches.

Patches 5-9 add nfs exporting support for fuse filesystems.

This is for 2.6.27.

Thanks,
Miklos

--

[patch 1/9] lockd: dont return EAGAIN from blocking lock request

[Miklos Szeredi]

[-- Attachment #1: lockd_fix.patch --]
[-- Type: text/plain, Size: 1042 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

EAGAIN does not make sense as a return value from a blocking lock
request, the lock should be retried by the kernel until either it is
successful or fails permanently.

Looking at the server side NLM_LCK_DENIED means a permanent error in
this case, so just turn this into an ENOLCK.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Trond Myklebust <trond.myklebust@fys.uio.no>
CC: "J. Bruce Fields" <bfields@fieldses.org>
---
 fs/lockd/clntproc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/fs/lockd/clntproc.c
===================================================================
--- linux-2.6.orig/fs/lockd/clntproc.c	2008-05-09 14:04:16.000000000 +0200
+++ linux-2.6/fs/lockd/clntproc.c	2008-05-09 14:04:46.000000000 +0200
@@ -808,7 +808,7 @@ nlm_stat_to_errno(__be32 status)
 	case NLM_LCK_GRANTED:
 		return 0;
 	case NLM_LCK_DENIED:
-		return -EAGAIN;
+		return -ENOLCK;
 	case NLM_LCK_DENIED_NOLOCKS:
 	case NLM_LCK_DENIED_GRACE_PERIOD:
 		return -ENOLCK;

--

[patch 2/9] locks: add special return value for asynchronous locks

[Miklos Szeredi]

[-- Attachment #1: locking_api_fix.patch --]
[-- Type: text/plain, Size: 7377 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Use a special error value FILE_LOCK_DEFERRED to mean that a locking
operation returned asynchronously.  This is returned by

  posix_lock_file() for sleeping locks to mean that the lock has been
  queued on the block list, and will be woken up when it might become
  available and needs to be retried (either fl_lmops->fl_notify() is
  called or fl_wait is woken up).

  f_op->lock() to mean either the above, or that the filesystem will
  call back with fl_lmops->fl_grant() when the result of the locking
  operation is known.  The filesystem can do this for sleeping as well
  as non-sleeping locks.

This is to make sure, that return values of -EAGAIN and -EINPROGRESS
by filesystems are not mistaken to mean an asynchronous locking.

This also makes error handling in fs/locks.c and lockd/svclock.c
slightly cleaner.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: Matthew Wilcox <matthew@wil.cx>
CC: "J. Bruce Fields" <bfields@fieldses.org>
CC: David Teigland <teigland@redhat.com>
---
 fs/dlm/plock.c     |    2 +-
 fs/lockd/svclock.c |   13 ++++---------
 fs/locks.c         |   28 ++++++++++++++--------------
 include/linux/fs.h |    6 ++++++
 4 files changed, 25 insertions(+), 24 deletions(-)

Index: linux-2.6/fs/locks.c
===================================================================
--- linux-2.6.orig/fs/locks.c	2008-05-09 14:04:16.000000000 +0200
+++ linux-2.6/fs/locks.c	2008-05-09 14:04:46.000000000 +0200
@@ -785,8 +785,10 @@ find_conflict:
 		if (!flock_locks_conflict(request, fl))
 			continue;
 		error = -EAGAIN;
-		if (request->fl_flags & FL_SLEEP)
-			locks_insert_block(fl, request);
+		if (!(request->fl_flags & FL_SLEEP))
+			goto out;
+		error = FILE_LOCK_DEFERRED;
+		locks_insert_block(fl, request);
 		goto out;
 	}
 	if (request->fl_flags & FL_ACCESS)
@@ -842,7 +844,7 @@ static int __posix_lock_file(struct inod
 			error = -EDEADLK;
 			if (posix_locks_deadlock(request, fl))
 				goto out;
-			error = -EAGAIN;
+			error = FILE_LOCK_DEFERRED;
 			locks_insert_block(fl, request);
 			goto out;
   		}
@@ -1041,7 +1043,7 @@ int posix_lock_file_wait(struct file *fi
 	might_sleep ();
 	for (;;) {
 		error = posix_lock_file(filp, fl, NULL);
-		if ((error != -EAGAIN) || !(fl->fl_flags & FL_SLEEP))
+		if (error != FILE_LOCK_DEFERRED)
 			break;
 		error = wait_event_interruptible(fl->fl_wait, !fl->fl_next);
 		if (!error)
@@ -1113,9 +1115,7 @@ int locks_mandatory_area(int read_write,
 
 	for (;;) {
 		error = __posix_lock_file(inode, &fl, NULL);
-		if (error != -EAGAIN)
-			break;
-		if (!(fl.fl_flags & FL_SLEEP))
+		if (error != FILE_LOCK_DEFERRED)
 			break;
 		error = wait_event_interruptible(fl.fl_wait, !fl.fl_next);
 		if (!error) {
@@ -1537,7 +1537,7 @@ int flock_lock_file_wait(struct file *fi
 	might_sleep();
 	for (;;) {
 		error = flock_lock_file(filp, fl);
-		if ((error != -EAGAIN) || !(fl->fl_flags & FL_SLEEP))
+		if (error != FILE_LOCK_DEFERRED)
 			break;
 		error = wait_event_interruptible(fl->fl_wait, !fl->fl_next);
 		if (!error)
@@ -1722,17 +1722,17 @@ out:
  * fl_grant is set. Callers expecting ->lock() to return asynchronously
  * will only use F_SETLK, not F_SETLKW; they will set FL_SLEEP if (and only if)
  * the request is for a blocking lock. When ->lock() does return asynchronously,
- * it must return -EINPROGRESS, and call ->fl_grant() when the lock
+ * it must return FILE_LOCK_DEFERRED, and call ->fl_grant() when the lock
  * request completes.
  * If the request is for non-blocking lock the file system should return
- * -EINPROGRESS then try to get the lock and call the callback routine with
- * the result. If the request timed out the callback routine will return a
+ * FILE_LOCK_DEFERRED then try to get the lock and call the callback routine
+ * with the result. If the request timed out the callback routine will return a
  * nonzero return code and the file system should release the lock. The file
  * system is also responsible to keep a corresponding posix lock when it
  * grants a lock so the VFS can find out which locks are locally held and do
  * the correct lock cleanup when required.
  * The underlying filesystem must not drop the kernel lock or call
- * ->fl_grant() before returning to the caller with a -EINPROGRESS
+ * ->fl_grant() before returning to the caller with a FILE_LOCK_DEFERRED
  * return code.
  */
 int vfs_lock_file(struct file *filp, unsigned int cmd, struct file_lock *fl, struct file_lock *conf)
@@ -1810,7 +1810,7 @@ again:
 	else {
 		for (;;) {
 			error = posix_lock_file(filp, file_lock, NULL);
-			if (error != -EAGAIN || cmd == F_SETLK)
+			if (error != FILE_LOCK_DEFERRED)
 				break;
 			error = wait_event_interruptible(file_lock->fl_wait,
 					!file_lock->fl_next);
@@ -1947,7 +1947,7 @@ again:
 	else {
 		for (;;) {
 			error = posix_lock_file(filp, file_lock, NULL);
-			if (error != -EAGAIN || cmd == F_SETLK64)
+			if (error != FILE_LOCK_DEFERRED)
 				break;
 			error = wait_event_interruptible(file_lock->fl_wait,
 					!file_lock->fl_next);
Index: linux-2.6/fs/lockd/svclock.c
===================================================================
--- linux-2.6.orig/fs/lockd/svclock.c	2008-05-09 14:04:16.000000000 +0200
+++ linux-2.6/fs/lockd/svclock.c	2008-05-09 14:04:46.000000000 +0200
@@ -423,8 +423,8 @@ nlmsvc_lock(struct svc_rqst *rqstp, stru
 			goto out;
 		case -EAGAIN:
 			ret = nlm_lck_denied;
-			break;
-		case -EINPROGRESS:
+			goto out;
+		case FILE_LOCK_DEFERRED:
 			if (wait)
 				break;
 			/* Filesystem lock operation is in progress
@@ -439,10 +439,6 @@ nlmsvc_lock(struct svc_rqst *rqstp, stru
 			goto out;
 	}
 
-	ret = nlm_lck_denied;
-	if (!wait)
-		goto out;
-
 	ret = nlm_lck_blocked;
 
 	/* Append to list of blocked */
@@ -520,7 +516,7 @@ nlmsvc_testlock(struct svc_rqst *rqstp, 
 	}
 
 	error = vfs_test_lock(file->f_file, &lock->fl);
-	if (error == -EINPROGRESS) {
+	if (error == FILE_LOCK_DEFERRED) {
 		ret = nlmsvc_defer_lock_rqst(rqstp, block);
 		goto out;
 	}
@@ -744,8 +740,7 @@ nlmsvc_grant_blocked(struct nlm_block *b
 	switch (error) {
 	case 0:
 		break;
-	case -EAGAIN:
-	case -EINPROGRESS:
+	case FILE_LOCK_DEFERRED:
 		dprintk("lockd: lock still blocked error %d\n", error);
 		nlmsvc_insert_block(block, NLM_NEVER);
 		nlmsvc_release_block(block);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-09 14:04:16.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-09 14:04:46.000000000 +0200
@@ -885,6 +885,12 @@ static inline int file_check_writeable(s
 #define FL_SLEEP	128	/* A blocking lock */
 
 /*
+ * Special return value from posix_lock_file() and vfs_lock_file() for
+ * asynchronous locking.
+ */
+#define FILE_LOCK_DEFERRED 1
+
+/*
  * The POSIX file lock owner is determined by
  * the "struct files_struct" in the thread group
  * (or NULL for no owner - BSD locks).
Index: linux-2.6/fs/dlm/plock.c
===================================================================
--- linux-2.6.orig/fs/dlm/plock.c	2008-05-09 14:04:16.000000000 +0200
+++ linux-2.6/fs/dlm/plock.c	2008-05-09 14:04:46.000000000 +0200
@@ -116,7 +116,7 @@ int dlm_posix_lock(dlm_lockspace_t *lock
 	if (xop->callback == NULL)
 		wait_event(recv_wq, (op->done != 0));
 	else {
-		rv = -EINPROGRESS;
+		rv = FILE_LOCK_DEFERRED;
 		goto out;
 	}
 

--

[patch 3/9] locks: cleanup code duplication

[Miklos Szeredi]

[-- Attachment #1: locks_cleanup.patch --]
[-- Type: text/plain, Size: 2681 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Extract common code into a function.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: "J. Bruce Fields" <bfields@fieldses.org>
CC: Matthew Wilcox <matthew@wil.cx>
---
 fs/locks.c |   71 ++++++++++++++++++++++++++-----------------------------------
 1 file changed, 31 insertions(+), 40 deletions(-)

Index: linux-2.6/fs/locks.c
===================================================================
--- linux-2.6.orig/fs/locks.c	2008-05-09 14:04:46.000000000 +0200
+++ linux-2.6/fs/locks.c	2008-05-09 14:04:47.000000000 +0200
@@ -1744,6 +1744,35 @@ int vfs_lock_file(struct file *filp, uns
 }
 EXPORT_SYMBOL_GPL(vfs_lock_file);
 
+static int do_lock_file_wait(struct file *filp, unsigned int cmd,
+			     struct file_lock *fl)
+{
+	int error;
+
+	error = security_file_lock(filp, fl->fl_type);
+	if (error)
+		return error;
+
+	if (filp->f_op && filp->f_op->lock != NULL)
+		error = filp->f_op->lock(filp, cmd, fl);
+	else {
+		for (;;) {
+			error = posix_lock_file(filp, fl, NULL);
+			if (error != FILE_LOCK_DEFERRED)
+				break;
+			error = wait_event_interruptible(fl->fl_wait,
+							 !fl->fl_next);
+			if (!error)
+				continue;
+
+			locks_delete_block(fl);
+			break;
+		}
+	}
+
+	return error;
+}
+
 /* Apply the lock described by l to an open file descriptor.
  * This implements both the F_SETLK and F_SETLKW commands of fcntl().
  */
@@ -1801,26 +1830,7 @@ again:
 		goto out;
 	}
 
-	error = security_file_lock(filp, file_lock->fl_type);
-	if (error)
-		goto out;
-
-	if (filp->f_op && filp->f_op->lock != NULL)
-		error = filp->f_op->lock(filp, cmd, file_lock);
-	else {
-		for (;;) {
-			error = posix_lock_file(filp, file_lock, NULL);
-			if (error != FILE_LOCK_DEFERRED)
-				break;
-			error = wait_event_interruptible(file_lock->fl_wait,
-					!file_lock->fl_next);
-			if (!error)
-				continue;
-
-			locks_delete_block(file_lock);
-			break;
-		}
-	}
+	error = do_lock_file_wait(filp, cmd, file_lock);
 
 	/*
 	 * Attempt to detect a close/fcntl race and recover by
@@ -1938,26 +1948,7 @@ again:
 		goto out;
 	}
 
-	error = security_file_lock(filp, file_lock->fl_type);
-	if (error)
-		goto out;
-
-	if (filp->f_op && filp->f_op->lock != NULL)
-		error = filp->f_op->lock(filp, cmd, file_lock);
-	else {
-		for (;;) {
-			error = posix_lock_file(filp, file_lock, NULL);
-			if (error != FILE_LOCK_DEFERRED)
-				break;
-			error = wait_event_interruptible(file_lock->fl_wait,
-					!file_lock->fl_next);
-			if (!error)
-				continue;
-
-			locks_delete_block(file_lock);
-			break;
-		}
-	}
+	error = do_lock_file_wait(filp, cmd, file_lock);
 
 	/*
 	 * Attempt to detect a close/fcntl race and recover by

--

[patch 4/9] locks: allow ->lock() to return FILE_LOCK_DEFERRED

[Miklos Szeredi]

[-- Attachment #1: locks_allow_async_lock_method.patch --]
[-- Type: text/plain, Size: 1544 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Allow filesystem's ->lock() method to call posix_lock_file() instead
of posix_lock_file_wait(), and return FILE_LOCK_DEFERRED.  This makes
it possible to implement a such a ->lock() function, that works with
the lock manager, which needs the call to be asynchronous.

Now the vfs_lock_file() helper can be used, so this is a cleanup as
well.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: "J. Bruce Fields" <bfields@fieldses.org>
CC: Matthew Wilcox <matthew@wil.cx>
---
 fs/locks.c |   23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

Index: linux-2.6/fs/locks.c
===================================================================
--- linux-2.6.orig/fs/locks.c	2008-05-09 14:04:47.000000000 +0200
+++ linux-2.6/fs/locks.c	2008-05-09 14:04:48.000000000 +0200
@@ -1753,21 +1753,16 @@ static int do_lock_file_wait(struct file
 	if (error)
 		return error;
 
-	if (filp->f_op && filp->f_op->lock != NULL)
-		error = filp->f_op->lock(filp, cmd, fl);
-	else {
-		for (;;) {
-			error = posix_lock_file(filp, fl, NULL);
-			if (error != FILE_LOCK_DEFERRED)
-				break;
-			error = wait_event_interruptible(fl->fl_wait,
-							 !fl->fl_next);
-			if (!error)
-				continue;
-
-			locks_delete_block(fl);
+	for (;;) {
+		error = vfs_lock_file(filp, cmd, fl, NULL);
+		if (error != FILE_LOCK_DEFERRED)
 			break;
-		}
+		error = wait_event_interruptible(fl->fl_wait, !fl->fl_next);
+		if (!error)
+			continue;
+
+		locks_delete_block(fl);
+		break;
 	}
 
 	return error;

--

[patch 5/9] fuse: prepare lookup for nfs export

[Miklos Szeredi]

[-- Attachment #1: fuse_nfs_export_prep.patch --]
[-- Type: text/plain, Size: 2288 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Use d_splice_alias() instead of d_add() in fuse lookup code, to allow
NFS exporting.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/dir.c |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Index: linux-2.6/fs/fuse/dir.c
===================================================================
--- linux-2.6.orig/fs/fuse/dir.c	2008-05-09 14:04:16.000000000 +0200
+++ linux-2.6/fs/fuse/dir.c	2008-05-09 14:04:49.000000000 +0200
@@ -239,18 +239,20 @@ int fuse_valid_type(int m)
  * Add a directory inode to a dentry, ensuring that no other dentry
  * refers to this inode.  Called with fc->inst_mutex.
  */
-static int fuse_d_add_directory(struct dentry *entry, struct inode *inode)
+static struct dentry *fuse_d_add_directory(struct dentry *entry,
+					   struct inode *inode)
 {
 	struct dentry *alias = d_find_alias(inode);
-	if (alias) {
+	if (alias && !(alias->d_flags & DCACHE_DISCONNECTED)) {
 		/* This tries to shrink the subtree below alias */
 		fuse_invalidate_entry(alias);
 		dput(alias);
 		if (!list_empty(&inode->i_dentry))
-			return -EBUSY;
+			return ERR_PTR(-EBUSY);
+	} else {
+		dput(alias);
 	}
-	d_add(entry, inode);
-	return 0;
+	return d_splice_alias(inode, entry);
 }
 
 static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
@@ -259,6 +261,7 @@ static struct dentry *fuse_lookup(struct
 	int err;
 	struct fuse_entry_out outarg;
 	struct inode *inode = NULL;
+	struct dentry *newent;
 	struct fuse_conn *fc = get_fuse_conn(dir);
 	struct fuse_req *req;
 	struct fuse_req *forget_req;
@@ -303,21 +306,22 @@ static struct dentry *fuse_lookup(struct
 
 	if (inode && S_ISDIR(inode->i_mode)) {
 		mutex_lock(&fc->inst_mutex);
-		err = fuse_d_add_directory(entry, inode);
+		newent = fuse_d_add_directory(entry, inode);
 		mutex_unlock(&fc->inst_mutex);
-		if (err) {
+		if (IS_ERR(newent)) {
 			iput(inode);
-			return ERR_PTR(err);
+			return newent;
 		}
 	} else
-		d_add(entry, inode);
+		newent = d_splice_alias(inode, entry);
 
+	entry = newent ? newent : entry;
 	entry->d_op = &fuse_dentry_operations;
 	if (!err)
 		fuse_change_entry_timeout(entry, &outarg);
 	else
 		fuse_invalidate_entry_cache(entry);
-	return NULL;
+	return newent;
 }
 
 /*

--

[patch 6/9] fuse: add export operations

[Miklos Szeredi]

[-- Attachment #1: fuse_nfs_export.patch --]
[-- Type: text/plain, Size: 6158 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Implement export_operations, to allow fuse filesystems to be exported
to NFS.  This feature has been in the out-of-tree fuse module, and is
widely used and tested.

It has not been originally merged into mainline, because doing the NFS
export in userspace was thought to be a cleaner and more efficient way
of doing it, than through the kernel.

While that is true, it would also have involved a lot of duplicated
effort at reimplementing NFS exporting (all the different versions of
the protocol).  This effort was unfortunately not undertaken by
anyone, so we are left with doing it the easy but less efficient way.

If this feature goes it, the out-of-tree fuse module can go away,
which would have several advantages:

  - not having to maintain two versions
  - less confusion for users
  - no bugs due to kernel API changes

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/dir.c    |    4 -
 fs/fuse/fuse_i.h |    4 +
 fs/fuse/inode.c  |  119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 125 insertions(+), 2 deletions(-)

Index: linux-2.6/fs/fuse/dir.c
===================================================================
--- linux-2.6.orig/fs/fuse/dir.c	2008-05-09 14:04:49.000000000 +0200
+++ linux-2.6/fs/fuse/dir.c	2008-05-09 14:04:50.000000000 +0200
@@ -97,7 +97,7 @@ void fuse_invalidate_attr(struct inode *
  * timeout is unknown (unlink, rmdir, rename and in some cases
  * lookup)
  */
-static void fuse_invalidate_entry_cache(struct dentry *entry)
+void fuse_invalidate_entry_cache(struct dentry *entry)
 {
 	fuse_dentry_settime(entry, 0);
 }
@@ -225,7 +225,7 @@ static int invalid_nodeid(u64 nodeid)
 	return !nodeid || nodeid == FUSE_ROOT_ID;
 }
 
-static struct dentry_operations fuse_dentry_operations = {
+struct dentry_operations fuse_dentry_operations = {
 	.d_revalidate	= fuse_dentry_revalidate,
 };
 
Index: linux-2.6/fs/fuse/fuse_i.h
===================================================================
--- linux-2.6.orig/fs/fuse/fuse_i.h	2008-05-09 14:04:45.000000000 +0200
+++ linux-2.6/fs/fuse/fuse_i.h	2008-05-09 14:04:50.000000000 +0200
@@ -464,6 +464,8 @@ static inline u64 get_node_id(struct ino
 /** Device operations */
 extern const struct file_operations fuse_dev_operations;
 
+extern struct dentry_operations fuse_dentry_operations;
+
 /**
  * Get a filled in inode
  */
@@ -604,6 +606,8 @@ void fuse_abort_conn(struct fuse_conn *f
  */
 void fuse_invalidate_attr(struct inode *inode);
 
+void fuse_invalidate_entry_cache(struct dentry *entry);
+
 /**
  * Acquire reference to fuse_conn
  */
Index: linux-2.6/fs/fuse/inode.c
===================================================================
--- linux-2.6.orig/fs/fuse/inode.c	2008-05-09 14:04:45.000000000 +0200
+++ linux-2.6/fs/fuse/inode.c	2008-05-09 14:04:50.000000000 +0200
@@ -18,6 +18,7 @@
 #include <linux/statfs.h>
 #include <linux/random.h>
 #include <linux/sched.h>
+#include <linux/exportfs.h>
 
 MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>");
 MODULE_DESCRIPTION("Filesystem in Userspace");
@@ -547,6 +548,123 @@ static struct inode *get_root_inode(stru
 	return fuse_iget(sb, 1, 0, &attr, 0, 0);
 }
 
+struct fuse_inode_handle
+{
+	u64 nodeid;
+	u32 generation;
+};
+
+static struct dentry *fuse_get_dentry(struct super_block *sb,
+				      struct fuse_inode_handle *handle)
+{
+	struct inode *inode;
+	struct dentry *entry;
+	int err = -ESTALE;
+
+	if (handle->nodeid == 0)
+		goto out_err;
+
+	inode = ilookup5(sb, handle->nodeid, fuse_inode_eq, &handle->nodeid);
+	if (!inode)
+		goto out_err;
+	err = -ESTALE;
+	if (inode->i_generation != handle->generation)
+		goto out_iput;
+
+	entry = d_alloc_anon(inode);
+	err = -ENOMEM;
+	if (!entry)
+		goto out_iput;
+
+	if (get_node_id(inode) != FUSE_ROOT_ID) {
+		entry->d_op = &fuse_dentry_operations;
+		fuse_invalidate_entry_cache(entry);
+	}
+
+	return entry;
+
+ out_iput:
+	iput(inode);
+ out_err:
+	return ERR_PTR(err);
+}
+
+static int fuse_encode_fh(struct dentry *dentry, u32 *fh, int *max_len,
+			   int connectable)
+{
+	struct inode *inode = dentry->d_inode;
+	int len = *max_len;
+	int type = 1;
+	u64 nodeid;
+	u32 generation;
+
+	if (len < 3 || (connectable && len < 6))
+		return  255;
+
+	nodeid = get_fuse_inode(inode)->nodeid;
+	generation = inode->i_generation;
+
+	len = 3;
+	fh[0] = (u32)(nodeid >> 32);
+	fh[1] = (u32)(nodeid & 0xffffffff);
+	fh[2] = generation;
+
+	if (connectable && !S_ISDIR(inode->i_mode)) {
+		struct inode *parent;
+
+		spin_lock(&dentry->d_lock);
+		parent = dentry->d_parent->d_inode;
+		nodeid = get_fuse_inode(parent)->nodeid;
+		generation = parent->i_generation;
+
+		fh[3] = (u32)(nodeid >> 32);
+		fh[4] = (u32)(nodeid & 0xffffffff);
+		fh[5] = generation;
+		spin_unlock(&dentry->d_lock);
+
+		len = 6;
+		type = 2;
+	}
+
+	*max_len = len;
+	return type;
+}
+
+static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
+		struct fid *fid, int fh_len, int fh_type)
+{
+	struct fuse_inode_handle handle;
+
+	if (fh_len < 3 || fh_type > 2)
+		return NULL;
+
+	handle.nodeid = (u64) fid->raw[0] << 32;
+	handle.nodeid |= (u64) fid->raw[1];
+	handle.generation = fid->raw[2];
+	return fuse_get_dentry(sb, &handle);
+}
+
+static struct dentry *fuse_fh_to_parent(struct super_block *sb,
+		struct fid *fid, int fh_len, int fh_type)
+{
+	struct fuse_inode_handle parent;
+
+	if (fh_type != 2 || fh_len < 6)
+		return NULL;
+
+	parent.nodeid = (u64) fid->raw[3] << 32;
+	parent.nodeid |= (u64) fid->raw[4];
+	parent.generation = fid->raw[5];
+	return fuse_get_dentry(sb, &parent);
+}
+
+
+static const struct export_operations fuse_export_operations = {
+	.fh_to_dentry	= fuse_fh_to_dentry,
+	.fh_to_parent	= fuse_fh_to_parent,
+	.encode_fh	= fuse_encode_fh,
+};
+
 static const struct super_operations fuse_super_operations = {
 	.alloc_inode    = fuse_alloc_inode,
 	.destroy_inode  = fuse_destroy_inode,
@@ -647,6 +765,7 @@ static int fuse_fill_super(struct super_
 	sb->s_magic = FUSE_SUPER_MAGIC;
 	sb->s_op = &fuse_super_operations;
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
+	sb->s_export_op = &fuse_export_operations;
 
 	file = fget(d.fd);
 	if (!file)

--

[patch 7/9] fuse: add fuse_lookup_name() helper

[Miklos Szeredi]

[-- Attachment #1: fuse_nfs_export_lookup_name.patch --]
[-- Type: text/plain, Size: 5405 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Add a new helper function which sends a LOOKUP request with the
supplied name.  This will be used by the next patch to send special
LOOKUP requests with "." and ".." as the name.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/dir.c |  117 ++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 77 insertions(+), 40 deletions(-)

Index: linux-2.6/fs/fuse/dir.c
===================================================================
--- linux-2.6.orig/fs/fuse/dir.c	2008-05-09 14:04:50.000000000 +0200
+++ linux-2.6/fs/fuse/dir.c	2008-05-09 14:04:51.000000000 +0200
@@ -112,18 +112,16 @@ static void fuse_invalidate_entry(struct
 	fuse_invalidate_entry_cache(entry);
 }
 
-static void fuse_lookup_init(struct fuse_req *req, struct inode *dir,
-			     struct dentry *entry,
+static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_req *req,
+			     u64 nodeid, struct qstr *name,
 			     struct fuse_entry_out *outarg)
 {
-	struct fuse_conn *fc = get_fuse_conn(dir);
-
 	memset(outarg, 0, sizeof(struct fuse_entry_out));
 	req->in.h.opcode = FUSE_LOOKUP;
-	req->in.h.nodeid = get_node_id(dir);
+	req->in.h.nodeid = nodeid;
 	req->in.numargs = 1;
-	req->in.args[0].size = entry->d_name.len + 1;
-	req->in.args[0].value = entry->d_name.name;
+	req->in.args[0].size = name->len + 1;
+	req->in.args[0].value = name->name;
 	req->out.numargs = 1;
 	if (fc->minor < 9)
 		req->out.args[0].size = FUSE_COMPAT_ENTRY_OUT_SIZE;
@@ -189,7 +187,8 @@ static int fuse_dentry_revalidate(struct
 		attr_version = fuse_get_attr_version(fc);
 
 		parent = dget_parent(entry);
-		fuse_lookup_init(req, parent->d_inode, entry, &outarg);
+		fuse_lookup_init(fc, req, get_node_id(parent->d_inode),
+				 &entry->d_name, &outarg);
 		request_send(fc, req);
 		dput(parent);
 		err = req->out.h.error;
@@ -255,73 +254,111 @@ static struct dentry *fuse_d_add_directo
 	return d_splice_alias(inode, entry);
 }
 
-static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
-				  struct nameidata *nd)
+int fuse_lookup_name(struct super_block *sb, u64 nodeid, struct qstr *name,
+		     struct fuse_entry_out *outarg, struct inode **inode)
 {
-	int err;
-	struct fuse_entry_out outarg;
-	struct inode *inode = NULL;
-	struct dentry *newent;
-	struct fuse_conn *fc = get_fuse_conn(dir);
+	struct fuse_conn *fc = get_fuse_conn_super(sb);
 	struct fuse_req *req;
 	struct fuse_req *forget_req;
 	u64 attr_version;
+	int err;
 
-	if (entry->d_name.len > FUSE_NAME_MAX)
-		return ERR_PTR(-ENAMETOOLONG);
+	*inode = NULL;
+	err = -ENAMETOOLONG;
+	if (name->len > FUSE_NAME_MAX)
+		goto out;
 
 	req = fuse_get_req(fc);
+	err = PTR_ERR(req);
 	if (IS_ERR(req))
-		return ERR_CAST(req);
+		goto out;
 
 	forget_req = fuse_get_req(fc);
+	err = PTR_ERR(forget_req);
 	if (IS_ERR(forget_req)) {
 		fuse_put_request(fc, req);
-		return ERR_CAST(forget_req);
+		goto out;
 	}
 
 	attr_version = fuse_get_attr_version(fc);
 
-	fuse_lookup_init(req, dir, entry, &outarg);
+	fuse_lookup_init(fc, req, nodeid, name, outarg);
 	request_send(fc, req);
 	err = req->out.h.error;
 	fuse_put_request(fc, req);
 	/* Zero nodeid is same as -ENOENT, but with valid timeout */
-	if (!err && outarg.nodeid &&
-	    (invalid_nodeid(outarg.nodeid) ||
-	     !fuse_valid_type(outarg.attr.mode)))
-		err = -EIO;
-	if (!err && outarg.nodeid) {
-		inode = fuse_iget(dir->i_sb, outarg.nodeid, outarg.generation,
-				  &outarg.attr, entry_attr_timeout(&outarg),
-				  attr_version);
-		if (!inode) {
-			fuse_send_forget(fc, forget_req, outarg.nodeid, 1);
-			return ERR_PTR(-ENOMEM);
-		}
+	if (err || !outarg->nodeid)
+		goto out_put_forget;
+
+	err = -EIO;
+	if (!outarg->nodeid)
+		goto out_put_forget;
+	if (!fuse_valid_type(outarg->attr.mode))
+		goto out_put_forget;
+
+	*inode = fuse_iget(sb, outarg->nodeid, outarg->generation,
+			   &outarg->attr, entry_attr_timeout(outarg),
+			   attr_version);
+	err = -ENOMEM;
+	if (!*inode) {
+		fuse_send_forget(fc, forget_req, outarg->nodeid, 1);
+		goto out;
 	}
+	err = 0;
+
+ out_put_forget:
 	fuse_put_request(fc, forget_req);
-	if (err && err != -ENOENT)
-		return ERR_PTR(err);
+ out:
+	return err;
+}
+
+static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
+				  struct nameidata *nd)
+{
+	int err;
+	struct fuse_entry_out outarg;
+	struct inode *inode;
+	struct dentry *newent;
+	struct fuse_conn *fc = get_fuse_conn(dir);
+	bool outarg_valid = true;
+
+	err = fuse_lookup_name(dir->i_sb, get_node_id(dir), &entry->d_name,
+			       &outarg, &inode);
+	if (err == -ENOENT) {
+		outarg_valid = false;
+		err = 0;
+	}
+	if (err)
+		goto out_err;
+
+	err = -EIO;
+	if (inode && get_node_id(inode) == FUSE_ROOT_ID)
+		goto out_iput;
 
 	if (inode && S_ISDIR(inode->i_mode)) {
 		mutex_lock(&fc->inst_mutex);
 		newent = fuse_d_add_directory(entry, inode);
 		mutex_unlock(&fc->inst_mutex);
-		if (IS_ERR(newent)) {
-			iput(inode);
-			return newent;
-		}
-	} else
+		err = PTR_ERR(newent);
+		if (IS_ERR(newent))
+			goto out_iput;
+	} else {
 		newent = d_splice_alias(inode, entry);
+	}
 
 	entry = newent ? newent : entry;
 	entry->d_op = &fuse_dentry_operations;
-	if (!err)
+	if (outarg_valid)
 		fuse_change_entry_timeout(entry, &outarg);
 	else
 		fuse_invalidate_entry_cache(entry);
+
 	return newent;
+
+ out_iput:
+	iput(inode);
+ out_err:
+	return ERR_PTR(err);
 }
 
 /*

--

[patch 8/9] fuse: nfs export special lookups

[Miklos Szeredi]

[-- Attachment #1: fuse_nfs_export_lookup.patch --]
[-- Type: text/plain, Size: 5546 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Implement the get_parent export operation by sending a LOOKUP request
with ".." as the name.

Implement looking up an inode by node ID after it has been evicted
from the cache.  This is done by seding a LOOKUP request with "." as
the name (for all file types, not just directories).

The filesystem can set the FUSE_EXPORT_SUPPORT flag in the INIT reply,
to indicate that it supports these special lookups.

Thanks to John Muir for the original implementation of this feature.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/fuse_i.h     |    6 ++++
 fs/fuse/inode.c      |   66 ++++++++++++++++++++++++++++++++++++++++++++++++---
 include/linux/fuse.h |    3 ++
 3 files changed, 72 insertions(+), 3 deletions(-)

Index: linux-2.6/fs/fuse/fuse_i.h
===================================================================
--- linux-2.6.orig/fs/fuse/fuse_i.h	2008-05-09 14:04:50.000000000 +0200
+++ linux-2.6/fs/fuse/fuse_i.h	2008-05-09 14:04:51.000000000 +0200
@@ -363,6 +363,9 @@ struct fuse_conn {
 	/** Do not send separate SETATTR request before open(O_TRUNC)  */
 	unsigned atomic_o_trunc : 1;
 
+	/** Filesystem supports NFS exporting.  Only set in INIT */
+	unsigned export_support : 1;
+
 	/*
 	 * The following bitfields are only for optimization purposes
 	 * and hence races in setting them will not cause malfunction
@@ -473,6 +476,9 @@ struct inode *fuse_iget(struct super_blo
 			int generation, struct fuse_attr *attr,
 			u64 attr_valid, u64 attr_version);
 
+int fuse_lookup_name(struct super_block *sb, u64 nodeid, struct qstr *name,
+		     struct fuse_entry_out *outarg, struct inode **inode);
+
 /**
  * Send FORGET command
  */
Index: linux-2.6/fs/fuse/inode.c
===================================================================
--- linux-2.6.orig/fs/fuse/inode.c	2008-05-09 14:04:50.000000000 +0200
+++ linux-2.6/fs/fuse/inode.c	2008-05-09 14:04:51.000000000 +0200
@@ -557,6 +557,7 @@ struct fuse_inode_handle
 static struct dentry *fuse_get_dentry(struct super_block *sb,
 				      struct fuse_inode_handle *handle)
 {
+	struct fuse_conn *fc = get_fuse_conn_super(sb);
 	struct inode *inode;
 	struct dentry *entry;
 	int err = -ESTALE;
@@ -565,8 +566,27 @@ static struct dentry *fuse_get_dentry(st
 		goto out_err;
 
 	inode = ilookup5(sb, handle->nodeid, fuse_inode_eq, &handle->nodeid);
-	if (!inode)
-		goto out_err;
+	if (!inode) {
+		struct fuse_entry_out outarg;
+		struct qstr name;
+
+		if (!fc->export_support)
+			goto out_err;
+
+		name.len = 1;
+		name.name = ".";
+		err = fuse_lookup_name(sb, handle->nodeid, &name, &outarg,
+				       &inode);
+		if (err && err != -ENOENT)
+			goto out_err;
+		if (err || !inode) {
+			err = -ESTALE;
+			goto out_err;
+		}
+		err = -EIO;
+		if (get_node_id(inode) != handle->nodeid)
+			goto out_iput;
+	}
 	err = -ESTALE;
 	if (inode->i_generation != handle->generation)
 		goto out_iput;
@@ -658,11 +678,46 @@ static struct dentry *fuse_fh_to_parent(
 	return fuse_get_dentry(sb, &parent);
 }
 
+static struct dentry *fuse_get_parent(struct dentry *child)
+{
+	struct inode *child_inode = child->d_inode;
+	struct fuse_conn *fc = get_fuse_conn(child_inode);
+	struct inode *inode;
+	struct dentry *parent;
+	struct fuse_entry_out outarg;
+	struct qstr name;
+	int err;
+
+	if (!fc->export_support)
+		return ERR_PTR(-ESTALE);
+
+	name.len = 2;
+	name.name = "..";
+	err = fuse_lookup_name(child_inode->i_sb, get_node_id(child_inode),
+			       &name, &outarg, &inode);
+	if (err && err != -ENOENT)
+		return ERR_PTR(err);
+	if (err || !inode)
+		return ERR_PTR(-ESTALE);
+
+	parent = d_alloc_anon(inode);
+	if (!parent) {
+		iput(inode);
+		return ERR_PTR(-ENOMEM);
+	}
+	if (get_node_id(inode) != FUSE_ROOT_ID) {
+		parent->d_op = &fuse_dentry_operations;
+		fuse_invalidate_entry_cache(parent);
+	}
+
+	return parent;
+}
 
 static const struct export_operations fuse_export_operations = {
 	.fh_to_dentry	= fuse_fh_to_dentry,
 	.fh_to_parent	= fuse_fh_to_parent,
 	.encode_fh	= fuse_encode_fh,
+	.get_parent	= fuse_get_parent,
 };
 
 static const struct super_operations fuse_super_operations = {
@@ -694,6 +749,11 @@ static void process_init_reply(struct fu
 				fc->no_lock = 1;
 			if (arg->flags & FUSE_ATOMIC_O_TRUNC)
 				fc->atomic_o_trunc = 1;
+			if (arg->minor >= 9) {
+				/* LOOKUP has dependency on proto version */
+				if (arg->flags & FUSE_EXPORT_SUPPORT)
+					fc->export_support = 1;
+			}
 			if (arg->flags & FUSE_BIG_WRITES)
 				fc->big_writes = 1;
 		} else {
@@ -720,7 +780,7 @@ static void fuse_send_init(struct fuse_c
 	arg->minor = FUSE_KERNEL_MINOR_VERSION;
 	arg->max_readahead = fc->bdi.ra_pages * PAGE_CACHE_SIZE;
 	arg->flags |= FUSE_ASYNC_READ | FUSE_POSIX_LOCKS | FUSE_ATOMIC_O_TRUNC |
-		FUSE_BIG_WRITES;
+		FUSE_EXPORT_SUPPORT | FUSE_BIG_WRITES;
 	req->in.h.opcode = FUSE_INIT;
 	req->in.numargs = 1;
 	req->in.args[0].size = sizeof(*arg);
Index: linux-2.6/include/linux/fuse.h
===================================================================
--- linux-2.6.orig/include/linux/fuse.h	2008-05-09 14:04:45.000000000 +0200
+++ linux-2.6/include/linux/fuse.h	2008-05-09 14:04:51.000000000 +0200
@@ -104,11 +104,14 @@ struct fuse_file_lock {
 
 /**
  * INIT request/reply flags
+ *
+ * FUSE_EXPORT_SUPPORT: filesystem handles lookups of "." and ".."
  */
 #define FUSE_ASYNC_READ		(1 << 0)
 #define FUSE_POSIX_LOCKS	(1 << 1)
 #define FUSE_FILE_OPS		(1 << 2)
 #define FUSE_ATOMIC_O_TRUNC	(1 << 3)
+#define FUSE_EXPORT_SUPPORT	(1 << 4)
 #define FUSE_BIG_WRITES		(1 << 5)
 
 /**

--

[patch 9/9] fuse: lockd support

[Miklos Szeredi]

[-- Attachment #1: fuse_async_lock.patch --]
[-- Type: text/plain, Size: 1613 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

If fuse filesystem doesn't define it's own lock operations, then allow
the lock manager to work with fuse.

Adding lockd support for remote locking is also possible, but more
difficult, because of the need handle lock cancellation.  It's also
rarely used, so leave it till later.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/file.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Index: linux-2.6/fs/fuse/file.c
===================================================================
--- linux-2.6.orig/fs/fuse/file.c	2008-05-09 14:04:45.000000000 +0200
+++ linux-2.6/fs/fuse/file.c	2008-05-09 14:04:52.000000000 +0200
@@ -1341,6 +1341,11 @@ static int fuse_setlk(struct file *file,
 	pid_t pid = fl->fl_type != F_UNLCK ? current->tgid : 0;
 	int err;
 
+	if (fl->fl_lmops && fl->fl_lmops->fl_grant) {
+		/* NLM needs asynchronous locks, which we don't support yet */
+		return -ENOLCK;
+	}
+
 	/* Unlock on close is handled by the flush method */
 	if (fl->fl_flags & FL_CLOSE)
 		return 0;
@@ -1365,7 +1370,9 @@ static int fuse_file_lock(struct file *f
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	int err;
 
-	if (cmd == F_GETLK) {
+	if (cmd == F_CANCELLK) {
+		err = 0;
+	} else if (cmd == F_GETLK) {
 		if (fc->no_lock) {
 			posix_test_lock(file, fl);
 			err = 0;
@@ -1373,7 +1380,7 @@ static int fuse_file_lock(struct file *f
 			err = fuse_getlk(file, fl);
 	} else {
 		if (fc->no_lock)
-			err = posix_lock_file_wait(file, fl);
+			err = posix_lock_file(file, fl, NULL);
 		else
 			err = fuse_setlk(file, fl, 0);
 	}

--

Re: [patch 0/9] file locking fixes + fuse nfs export support

[J. Bruce Fields]

A separate question--does the problem you reported with interrupted lock
requests still exist?

On Fri, May 09, 2008 at 02:41:07PM +0200, Miklos Szeredi wrote:
> Patches 1-4 are file locking API changes required for the rest of the
> patches.
> 
> Patches 5-9 add nfs exporting support for fuse filesystems.

Neat, thanks for working on this.

--b.

> This is for 2.6.27.

Re: [patch 0/9] file locking fixes + fuse nfs export support

[Miklos Szeredi]

> A separate question--does the problem you reported with interrupted lock
> requests still exist?

No, that bug seems to have gone away in 2.6.26-rc1.  Don't know how,
since the 2.6.25 + nfs.git was still bad.

Should I bisect it to see which patch solved it? ;)

Miklos

Re: [patch 0/9] file locking fixes + fuse nfs export support

[J. Bruce Fields]

On Fri, May 09, 2008 at 05:58:05PM +0200, Miklos Szeredi wrote:
> > A separate question--does the problem you reported with interrupted lock
> > requests still exist?
> 
> No, that bug seems to have gone away in 2.6.26-rc1.  Don't know how,
> since the 2.6.25 + nfs.git was still bad.

Yipes.

> Should I bisect it to see which patch solved it? ;)

That would be kinda amusing....

Well, and since I don't remember seeing anything that'd fix that I'd
like to make sure it isn't still lurking.

--b.

Re: [patch 1/9] lockd: dont return EAGAIN from blocking lock request

[J. Bruce Fields]

On Fri, May 09, 2008 at 02:41:08PM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> EAGAIN does not make sense as a return value from a blocking lock

But nlm_stat_to_errno() is called in the non-blocking case too, right?
What am I missing?

--b.

> request, the lock should be retried by the kernel until either it is
> successful or fails permanently.
> 
> Looking at the server side NLM_LCK_DENIED means a permanent error in
> this case, so just turn this into an ENOLCK.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
> CC: Trond Myklebust <trond.myklebust@fys.uio.no>
> CC: "J. Bruce Fields" <bfields@fieldses.org>
> ---
>  fs/lockd/clntproc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux-2.6/fs/lockd/clntproc.c
> ===================================================================
> --- linux-2.6.orig/fs/lockd/clntproc.c	2008-05-09 14:04:16.000000000 +0200
> +++ linux-2.6/fs/lockd/clntproc.c	2008-05-09 14:04:46.000000000 +0200
> @@ -808,7 +808,7 @@ nlm_stat_to_errno(__be32 status)
>  	case NLM_LCK_GRANTED:
>  		return 0;
>  	case NLM_LCK_DENIED:
> -		return -EAGAIN;
> +		return -ENOLCK;
>  	case NLM_LCK_DENIED_NOLOCKS:
>  	case NLM_LCK_DENIED_GRACE_PERIOD:
>  		return -ENOLCK;
> 
> --

Re: [patch 1/9] lockd: dont return EAGAIN from blocking lock request

[Miklos Szeredi]

> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > EAGAIN does not make sense as a return value from a blocking lock
> 
> But nlm_stat_to_errno() is called in the non-blocking case too, right?
> What am I missing?

Yes, you're right, I messed that up and didn't even test properly.

Will fix.

Thanks,
Miklos

Re: [patch 6/9] fuse: add export operations

[Christoph Hellwig]

> +struct fuse_inode_handle
> +{
> +	u64 nodeid;
> +	u32 generation;
> +};

This is the same filehandle type XFS uses for 64bit inode filesystems
where the parent is not encoded.  I'll post a patch soon to move that
64bit inode handling to common code.  I don't think it's a good idea
to delay your patch until that happens, but can you chose the fid
types to be the same as XFS does currently so that a conversion is
easily possible.  The fh_type should be 0x81 (fh type 1 order by 0x80
to imply it's 64bit inodes)

In case you want to look at that patch I'v uploaded the current
version at http://verein.lst.de/~hch/generic-64bit-inode-export

> +static struct dentry *fuse_get_dentry(struct super_block *sb,
> +				      struct fuse_inode_handle *handle)
> +{
> +	struct inode *inode;
> +	struct dentry *entry;
> +	int err = -ESTALE;
> +
> +	if (handle->nodeid == 0)
> +		goto out_err;
> +
> +	inode = ilookup5(sb, handle->nodeid, fuse_inode_eq, &handle->nodeid);
> +	if (!inode)
> +		goto out_err;

Only finding in-memory inodes for nfs export is quite dangerous.  I
think it would be a much better idea to support whatever upcalls are
needed to find the object if it's not in memory.  Without that the nfs
exporting support is hardly useable.

Re: [patch 6/9] fuse: add export operations

[Christoph Hellwig]

On Fri, May 09, 2008 at 03:40:30PM -0400, Christoph Hellwig wrote:
> This is the same filehandle type XFS uses for 64bit inode filesystems
> where the parent is not encoded.  I'll post a patch soon to move that
> 64bit inode handling to common code.  I don't think it's a good idea
> to delay your patch until that happens, but can you chose the fid
> types to be the same as XFS does currently so that a conversion is
> easily possible.  The fh_type should be 0x81 (fh type 1 order by 0x80
> to imply it's 64bit inodes)
> 
> In case you want to look at that patch I'v uploaded the current
> version at http://verein.lst.de/~hch/generic-64bit-inode-export

Actually that patch should be ready for submission, so if you want to
use it (and it should help cleaning things up a bit) feel free to add
it to the series.  I'll provide a proper description and sign-off in
that case.

> Only finding in-memory inodes for nfs export is quite dangerous.  I
> think it would be a much better idea to support whatever upcalls are
> needed to find the object if it's not in memory.  Without that the nfs
> exporting support is hardly useable.

Just noticed this is fixed in a later patch, sorry.

But it would be much better to first introduce the infrastructure and
then use it from the beginning.

Re: [patch 6/9] fuse: add export operations

[Erez Zadok]

In message <20080509194030.GA29118@infradead.org>, Christoph Hellwig writes:
> > +struct fuse_inode_handle
> > +{
> > +	u64 nodeid;
> > +	u32 generation;
> > +};
> 
> This is the same filehandle type XFS uses for 64bit inode filesystems
> where the parent is not encoded.  I'll post a patch soon to move that
> 64bit inode handling to common code.  I don't think it's a good idea
> to delay your patch until that happens, but can you chose the fid
> types to be the same as XFS does currently so that a conversion is
> easily possible.  The fh_type should be 0x81 (fh type 1 order by 0x80
> to imply it's 64bit inodes)
> 
> In case you want to look at that patch I'v uploaded the current
> version at http://verein.lst.de/~hch/generic-64bit-inode-export
> 
> > +static struct dentry *fuse_get_dentry(struct super_block *sb,
> > +				      struct fuse_inode_handle *handle)
> > +{
> > +	struct inode *inode;
> > +	struct dentry *entry;
> > +	int err = -ESTALE;
> > +
> > +	if (handle->nodeid == 0)
> > +		goto out_err;
> > +
> > +	inode = ilookup5(sb, handle->nodeid, fuse_inode_eq, &handle->nodeid);
> > +	if (!inode)
> > +		goto out_err;
> 
> Only finding in-memory inodes for nfs export is quite dangerous.  I
> think it would be a much better idea to support whatever upcalls are
> needed to find the object if it's not in memory.  Without that the nfs
> exporting support is hardly useable.

In the ODF version of unionfs, we store the inode numbers persistently in a
small /odf partition, exactly for that reason.  It'd be wonderful if there
was kernel support for persistent inode numbers that could be used by
various virtual filesystems.

Erez.

Re: [patch 6/9] fuse: add export operations

[Miklos Szeredi]

> > Only finding in-memory inodes for nfs export is quite dangerous.  I
> > think it would be a much better idea to support whatever upcalls are
> > needed to find the object if it's not in memory.  Without that the nfs
> > exporting support is hardly useable.
> 
> In the ODF version of unionfs, we store the inode numbers persistently in a
> small /odf partition, exactly for that reason.

What do inode numbers index on that partition?  A path?  How do you
garbage collect unused inode numbers?

>  It'd be wonderful if there
> was kernel support for persistent inode numbers that could be used by
> various virtual filesystems.

Well, fuse for one doesn't need it because it's typically a function
that is best pushed to userspace.

Miklos

Re: [patch 6/9] fuse: add export operations

[Miklos Szeredi]

> On Fri, May 09, 2008 at 03:40:30PM -0400, Christoph Hellwig wrote:
> > This is the same filehandle type XFS uses for 64bit inode filesystems
> > where the parent is not encoded.  I'll post a patch soon to move that
> > 64bit inode handling to common code.  I don't think it's a good idea
> > to delay your patch until that happens, but can you chose the fid
> > types to be the same as XFS does currently so that a conversion is
> > easily possible.  The fh_type should be 0x81 (fh type 1 order by 0x80
> > to imply it's 64bit inodes)
> > 
> > In case you want to look at that patch I'v uploaded the current
> > version at http://verein.lst.de/~hch/generic-64bit-inode-export
> 
> Actually that patch should be ready for submission, so if you want to
> use it (and it should help cleaning things up a bit) feel free to add
> it to the series.  I'll provide a proper description and sign-off in
> that case.

Yeah, it would help.

One suggestion I have for that patch: leave export_encode_fh() as is
(or just rename got generic_encode_fh()), and introduce a new
generic_encode64_fh().  One less magic boolean parameter to worry
about, and there's very little shared code between those two anyway.

Miklos

Re: [patch 6/9] fuse: add export operations

[Christoph Hellwig]

On Tue, May 13, 2008 at 10:34:12AM +0200, Miklos Szeredi wrote:
> One suggestion I have for that patch: leave export_encode_fh() as is
> (or just rename got generic_encode_fh()), and introduce a new
> generic_encode64_fh().  One less magic boolean parameter to worry
> about, and there's very little shared code between those two anyway.

Yeah, probably makes sense to split them.

Re: [patch 0/9] file locking fixes + fuse nfs export support

[Miklos Szeredi]

> > > A separate question--does the problem you reported with interrupted lock
> > > requests still exist?
> > 
> > No, that bug seems to have gone away in 2.6.26-rc1.  Don't know how,
> > since the 2.6.25 + nfs.git was still bad.
> 
> Yipes.
> 
> > Should I bisect it to see which patch solved it? ;)
> 
> That would be kinda amusing....
> 
> Well, and since I don't remember seeing anything that'd fix that I'd
> like to make sure it isn't still lurking.

Well, it was in the nfs-2.6 tree after all (commit 4d7c402).  I just
mucked up testing the last time, probably because I didn't truly
believe in this fix.

Miklos

Re: [patch 0/9] file locking fixes + fuse nfs export support

[J. Bruce Fields]

On Tue, May 13, 2008 at 11:05:28PM +0200, Miklos Szeredi wrote:
> > > > A separate question--does the problem you reported with interrupted lock
> > > > requests still exist?
> > > 
> > > No, that bug seems to have gone away in 2.6.26-rc1.  Don't know how,
> > > since the 2.6.25 + nfs.git was still bad.
> > 
> > Yipes.
> > 
> > > Should I bisect it to see which patch solved it? ;)
> > 
> > That would be kinda amusing....
> > 
> > Well, and since I don't remember seeing anything that'd fix that I'd
> > like to make sure it isn't still lurking.
> 
> Well, it was in the nfs-2.6 tree after all (commit 4d7c402).  I just

I think you must mean c4d7c402b788b73dc24f1e54a57f89d3dc5eb7bc, "NFS:
Remove the buggy lock-if-signalled case from do_setlk()"?

> mucked up testing the last time, probably because I didn't truly
> believe in this fix.

OK, that makes much more sense!  Thanks for the confirmation.

--b.

Re: [patch 0/9] file locking fixes + fuse nfs export support

[Miklos Szeredi]

> > Well, it was in the nfs-2.6 tree after all (commit 4d7c402).  I just
> 
> I think you must mean c4d7c402b788b73dc24f1e54a57f89d3dc5eb7bc, "NFS:
> Remove the buggy lock-if-signalled case from do_setlk()"?

Yeah.

Miklos

Re: [patch 6/9] fuse: add export operations

[Miklos Szeredi]

> > This is the same filehandle type XFS uses for 64bit inode filesystems
> > where the parent is not encoded.  I'll post a patch soon to move that
> > 64bit inode handling to common code.  I don't think it's a good idea
> > to delay your patch until that happens, but can you chose the fid
> > types to be the same as XFS does currently so that a conversion is
> > easily possible.  The fh_type should be 0x81 (fh type 1 order by 0x80
> > to imply it's 64bit inodes)
> > 
> > In case you want to look at that patch I'v uploaded the current
> > version at http://verein.lst.de/~hch/generic-64bit-inode-export

OK, the problem is: fuse doesn't store the number indexing the inodes
in i_ino.  This is not entirely trivial to fix: we either supply the
inode numbers to the encoding function explicitly, or use something
like the get_inode callback in generic_fh_to_dentry/_parent.  At which
point I begin to doubt if it's worth the effort to share this code.
After all the encoding/decoding functions are not complicated at all.

Miklos

Re: [patch 6/9] fuse: add export operations

[Christoph Hellwig]

On Thu, May 15, 2008 at 03:07:01PM +0200, Miklos Szeredi wrote:
> in i_ino.  This is not entirely trivial to fix: we either supply the
> inode numbers to the encoding function explicitly, or use something
> like the get_inode callback in generic_fh_to_dentry/_parent.  At which
> point I begin to doubt if it's worth the effort to share this code.
> After all the encoding/decoding functions are not complicated at all.

Doh, in that case using the generic encoce helper doesn't make any
sense.  decode doesn't care about i_ino, so it should be useable.  I'd
say go ahead with your fuse-local implementation for now, but please use
the 0x8n fh_type values.