Hi,
The following patch fixes a long-standing problems with the Linux
implementation of NFS sillydeletes: that of the sillydelete being done
in the context of the last process to dput() the dentry.
This has causes several problems for users, not least being the
fact that if the closing process did not have the required
permissions, then the sillydelete was never completed. Ditto if the
close was due to a signal.
At least as worrying was the fact that the last dput() could be
performed upon the completion of an asynchronous read/write/commit. In
this case, the context would be the rpciod() process. Since the call
to rename() in nfs_dentry_iput() was a synchronous RPC call (i.e. the
process is made to sleep), this could cause deadlocks where rpciod is
unable to read in from the TCP sockets.
More often than not, though, it would put out complaints about 'RPC:
rpciod waiting on sync task!'.
The following patch attempts to address both these issues by
converting the sillydelete call to an asynchronous RPC call. Basically
this means that after sillyrenaming the file, sillydelete() will set
up the RPC task (saving the user credentials), and will put it to
sleep on a queue. nfs_dentry_iput() then just has to wake it up,
completing the remove.
Since the actual RPC task gets run asynchronously by rpciod() the
closing process never needs to sleep, and neither of the above
problems become an issue.
Cheers,
Trond
diff -u --recursive --new-file linux-2.4.0-test7/fs/nfs/Makefile
linux-2.4.0-silly/fs/nfs/Makefile
--- linux-2.4.0-test7/fs/nfs/Makefile Sat Apr 1 18:04:27 2000
+++ linux-2.4.0-silly/fs/nfs/Makefile Mon Sep 4 12:06:02 2000
@@ -9,7 +9,7 @@
O_TARGET := nfs.o
O_OBJS := inode.o file.o read.o write.o dir.o symlink.o proc.o \
- nfs2xdr.o flushd.o
+ nfs2xdr.o flushd.o unlink.o
ifdef CONFIG_ROOT_NFS
O_OBJS += nfsroot.o mount_clnt.o
diff -u --recursive --new-file linux-2.4.0-test7/fs/nfs/dir.c
linux-2.4.0-silly/fs/nfs/dir.c
--- linux-2.4.0-test7/fs/nfs/dir.c Mon Aug 21 22:00:25 2000
+++ linux-2.4.0-silly/fs/nfs/dir.c Mon Sep 4 11:34:33 2000
@@ -606,16 +606,8 @@
static void nfs_dentry_iput(struct dentry *dentry, struct inode *inode)
{
if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
- struct dentry *dir = dentry->d_parent;
- struct inode *dir_i = dir->d_inode;
- int error;
-
lock_kernel();
- dir = dentry->d_parent;
- dir_i = dir->d_inode;
- nfs_zap_caches(dir_i);
- NFS_CACHEINV(inode);
- error = NFS_PROTO(dir_i)->remove(dir, &dentry->d_name);
+ nfs_complete_unlink(dentry);
unlock_kernel();
}
iput(inode);
@@ -868,7 +860,7 @@
if (!error) {
nfs_renew_times(dentry);
d_move(dentry, sdentry);
- dentry->d_flags |= DCACHE_NFSFS_RENAMED;
+ error = nfs_async_unlink(dentry);
/* If we return 0 we don't unlink */
}
dput(sdentry);
diff -u --recursive --new-file linux-2.4.0-test7/fs/nfs/nfs3proc.c
linux-2.4.0-silly/fs/nfs/nfs3proc.c
--- linux-2.4.0-test7/fs/nfs/nfs3proc.c Sat Jun 24 06:12:53 2000
+++ linux-2.4.0-silly/fs/nfs/nfs3proc.c Mon Sep 4 13:27:41 2000
@@ -267,6 +267,34 @@
}
static int
+nfs3_proc_unlink_setup(struct rpc_message *msg, struct dentry *dir, struct qstr *name)
+{
+ struct nfs3_diropargs *arg;
+ struct nfs_fattr *res;
+
+ arg = (struct nfs3_diropargs *)kmalloc(sizeof(*arg)+sizeof(*res), GFP_KERNEL);
+ if (!arg)
+ return -ENOMEM;
+ res = (struct nfs_fattr*)(arg + 1);
+ arg->fh = NFS_FH(dir);
+ arg->name = name->name;
+ arg->len = name->len;
+ msg->rpc_proc = NFS3PROC_REMOVE;
+ msg->rpc_argp = arg;
+ msg->rpc_resp = res;
+ return 0;
+}
+
+static void
+nfs3_proc_unlink_done(struct dentry *dir, struct rpc_message *msg)
+{
+ struct nfs_fattr *dir_attr = (struct nfs_fattr*)msg->rpc_resp;
+
+ nfs_refresh_inode(dir->d_inode, dir_attr);
+ kfree(msg->rpc_argp);
+}
+
+static int
nfs3_proc_rename(struct dentry *old_dir, struct qstr *old_name,
struct dentry *new_dir, struct qstr *new_name)
{
@@ -469,6 +497,8 @@
NULL, /* commit */
nfs3_proc_create,
nfs3_proc_remove,
+ nfs3_proc_unlink_setup,
+ nfs3_proc_unlink_done,
nfs3_proc_rename,
nfs3_proc_link,
nfs3_proc_symlink,
diff -u --recursive --new-file linux-2.4.0-test7/fs/nfs/proc.c
linux-2.4.0-silly/fs/nfs/proc.c
--- linux-2.4.0-test7/fs/nfs/proc.c Mon Aug 21 22:00:25 2000
+++ linux-2.4.0-silly/fs/nfs/proc.c Mon Sep 4 13:23:30 2000
@@ -233,6 +233,29 @@
}
static int
+nfs_proc_unlink_setup(struct rpc_message *msg, struct dentry *dir, struct qstr *name)
+{
+ struct nfs_diropargs *arg;
+
+ arg = (struct nfs_diropargs *)kmalloc(sizeof(*arg), GFP_KERNEL);
+ if (!arg)
+ return -ENOMEM;
+ arg->fh = NFS_FH(dir);
+ arg->name = name->name;
+ arg->len = name->len;
+ msg->rpc_proc = NFSPROC_REMOVE;
+ msg->rpc_argp = arg;
+ return 0;
+}
+
+static void
+nfs_proc_unlink_done(struct dentry *dir, struct rpc_message *msg)
+{
+ NFS_CACHEINV(dir->d_inode);
+ kfree(msg->rpc_argp);
+}
+
+static int
nfs_proc_rename(struct dentry *old_dir, struct qstr *old_name,
struct dentry *new_dir, struct qstr *new_name)
{
@@ -365,6 +388,8 @@
NULL, /* commit */
nfs_proc_create,
nfs_proc_remove,
+ nfs_proc_unlink_setup,
+ nfs_proc_unlink_done,
nfs_proc_rename,
nfs_proc_link,
nfs_proc_symlink,
diff -u --recursive --new-file linux-2.4.0-test7/fs/nfs/unlink.c
linux-2.4.0-silly/fs/nfs/unlink.c
--- linux-2.4.0-test7/fs/nfs/unlink.c Thu Jan 1 01:00:00 1970
+++ linux-2.4.0-silly/fs/nfs/unlink.c Mon Sep 4 14:10:31 2000
@@ -0,0 +1,212 @@
+/*
+ * linux/fs/nfs/unlink.c
+ *
+ * nfs sillydelete handling
+ *
+ * NOTE: we rely on holding the BKL for list manipulation protection.
+ */
+
+#include <linux/malloc.h>
+#include <linux/string.h>
+#include <linux/dcache.h>
+#include <linux/sunrpc/sched.h>
+#include <linux/sunrpc/clnt.h>
+#include <linux/nfs_fs.h>
+
+
+struct nfs_unlinkdata {
+ struct nfs_unlinkdata *next;
+ struct dentry *dir, *dentry;
+ struct qstr name;
+ struct rpc_task task;
+ struct rpc_cred *cred;
+ unsigned int count;
+};
+
+static struct nfs_unlinkdata *nfs_deletes = NULL;
+static struct rpc_wait_queue nfs_delete_queue = RPC_INIT_WAITQ("nfs_delete_queue");
+
+/**
+ * nfs_detach_unlinkdata - Remove asynchronous unlink from global list
+ * @data: pointer to descriptor
+ */
+static inline void
+nfs_detach_unlinkdata(struct nfs_unlinkdata *data)
+{
+ struct nfs_unlinkdata **q;
+
+ for (q = &nfs_deletes; *q != NULL; q = &((*q)->next)) {
+ if (*q == data) {
+ *q = data->next;
+ break;
+ }
+ }
+}
+
+/**
+ * nfs_put_unlinkdata - release data from a sillydelete operation.
+ * @data: pointer to unlink structure.
+ */
+static void
+nfs_put_unlinkdata(struct nfs_unlinkdata *data)
+{
+ if (--data->count == 0) {
+ nfs_detach_unlinkdata(data);
+ if (data->name.name != NULL)
+ kfree(data->name.name);
+ kfree(data);
+ }
+}
+
+#define NAME_ALLOC_LEN(len) ((len+16) & ~15)
+/**
+ * nfs_copy_dname - copy dentry name to data structure
+ * @dentry: pointer to dentry
+ * @data: nfs_unlinkdata
+ */
+static inline void
+nfs_copy_dname(struct dentry *dentry, struct nfs_unlinkdata *data)
+{
+ char *str;
+ int len = dentry->d_name.len;
+
+ str = kmalloc(NAME_ALLOC_LEN(len), GFP_KERNEL);
+ if (!str)
+ return;
+ memcpy(str, dentry->d_name.name, len);
+ if (!data->name.len) {
+ data->name.len = len;
+ data->name.name = str;
+ } else
+ kfree(str);
+}
+
+/**
+ * nfs_async_unlink_init - Initialize the RPC info
+ * @task: rpc_task of the sillydelete
+ *
+ * We delay initializing RPC info until after the call to dentry_iput()
+ * in order to minimize races against rename().
+ */
+static void
+nfs_async_unlink_init(struct rpc_task *task)
+{
+ struct nfs_unlinkdata *data = (struct nfs_unlinkdata *)task->tk_calldata;
+ struct dentry *dir = data->dir;
+ struct rpc_message msg;
+ int status = -ENOENT;
+
+ if (!data->name.len)
+ goto out_err;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.rpc_cred = data->cred;
+ status = NFS_PROTO(dir->d_inode)->unlink_setup(&msg, dir, &data->name);
+ if (status < 0)
+ goto out_err;
+ rpc_call_setup(task, &msg, 0);
+ return;
+ out_err:
+ rpc_exit(task, status);
+}
+
+/**
+ * nfs_async_unlink_done - Sillydelete post-processing
+ * @task: rpc_task of the sillydelete
+ *
+ * Do the directory attribute update.
+ */
+static void
+nfs_async_unlink_done(struct rpc_task *task)
+{
+ struct nfs_unlinkdata *data = (struct nfs_unlinkdata *)task->tk_calldata;
+ struct dentry *dir = data->dir;
+ struct inode *dir_i = dir->d_inode;
+
+ nfs_zap_caches(dir_i);
+ NFS_PROTO(dir_i)->unlink_done(dir, &task->tk_msg);
+ rpcauth_releasecred(task->tk_auth, data->cred);
+ data->cred = NULL;
+ dput(dir);
+}
+
+/**
+ * nfs_async_unlink_release - Release the sillydelete data.
+ * @task: rpc_task of the sillydelete
+ *
+ * We need to call nfs_put_unlinkdata as a 'tk_release' task since the
+ * rpc_task would be freed too.
+ */
+static void
+nfs_async_unlink_release(struct rpc_task *task)
+{
+ struct nfs_unlinkdata *data = (struct nfs_unlinkdata *)task->tk_calldata;
+ nfs_put_unlinkdata(data);
+}
+
+/**
+ * nfs_async_unlink - asynchronous unlinking of a file
+ * @dir: directory in which the file resides.
+ * @name: name of the file to unlink.
+ */
+int
+nfs_async_unlink(struct dentry *dentry)
+{
+ struct dentry *dir = dentry->d_parent;
+ struct nfs_unlinkdata *data;
+ struct rpc_task *task;
+ struct rpc_clnt *clnt = NFS_CLIENT(dir->d_inode);
+ int status = -ENOMEM;
+
+ data = kmalloc(sizeof(*data), GFP_KERNEL);
+ if (!data)
+ goto out;
+ memset(data, 0, sizeof(*data));
+
+ data->dir = dget(dir);
+ data->dentry = dentry;
+
+ data->next = nfs_deletes;
+ nfs_deletes = data;
+ data->count = 1;
+
+ task = &data->task;
+ rpc_init_task(task, clnt, nfs_async_unlink_done , RPC_TASK_ASYNC);
+ task->tk_calldata = data;
+ task->tk_action = nfs_async_unlink_init;
+ task->tk_release = nfs_async_unlink_release;
+
+ dentry->d_flags |= DCACHE_NFSFS_RENAMED;
+ data->cred = rpcauth_lookupcred(clnt->cl_auth, 0);
+
+ rpc_sleep_on(&nfs_delete_queue, task, NULL, NULL);
+ status = 0;
+ out:
+ return status;
+}
+
+/**
+ * nfs_complete_remove - Initialize completion of the sillydelete
+ * @dentry: dentry to delete
+ *
+ * Since we're most likely to be called by dentry_iput(), we
+ * only use the dentry to find the sillydelete. We then copy the name
+ * into the qstr.
+ */
+void
+nfs_complete_unlink(struct dentry *dentry)
+{
+ struct nfs_unlinkdata *data;
+
+ for(data = nfs_deletes; data != NULL; data = data->next) {
+ if (dentry == data->dentry)
+ break;
+ }
+ if (!data)
+ return;
+ data->count++;
+ nfs_copy_dname(dentry, data);
+ if (data->task.tk_rpcwait == &nfs_delete_queue)
+ rpc_wake_up_task(&data->task);
+ nfs_put_unlinkdata(data);
+}
diff -u --recursive --new-file linux-2.4.0-test7/include/linux/nfs_fs.h
linux-2.4.0-silly/include/linux/nfs_fs.h
--- linux-2.4.0-test7/include/linux/nfs_fs.h Thu Aug 24 12:07:30 2000
+++ linux-2.4.0-silly/include/linux/nfs_fs.h Mon Sep 4 13:27:38 2000
@@ -184,6 +184,12 @@
extern int nfs_lock(struct file *, int, struct file_lock *);
/*
+ * linux/fs/nfs/unlink.c
+ */
+extern int nfs_async_unlink(struct dentry *);
+extern void nfs_complete_unlink(struct dentry *);
+
+/*
* linux/fs/nfs/write.c
*/
extern int nfs_writepage(struct file *file, struct page *);
diff -u --recursive --new-file linux-2.4.0-test7/include/linux/nfs_xdr.h
linux-2.4.0-silly/include/linux/nfs_xdr.h
--- linux-2.4.0-test7/include/linux/nfs_xdr.h Sat May 20 19:27:57 2000
+++ linux-2.4.0-silly/include/linux/nfs_xdr.h Mon Sep 4 13:27:32 2000
@@ -334,6 +334,9 @@
int (*create) (struct dentry *, struct qstr *, struct iattr *,
int, struct nfs_fh *, struct nfs_fattr *);
int (*remove) (struct dentry *, struct qstr *);
+ int (*unlink_setup) (struct rpc_message *,
+ struct dentry *, struct qstr *);
+ void (*unlink_done) (struct dentry *, struct rpc_message *);
int (*rename) (struct dentry *, struct qstr *,
struct dentry *, struct qstr *);
int (*link) (struct dentry *, struct dentry *, struct qstr *);
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]