Skip to contentSkip to conversation

2.6.23-rc9-CITI_NFS4_ALL-1 connectathon suite testdir error

DISCUSSION Le Rouzic 2 participants 4 messages Oct 12, 2007 – Oct 23, 2007

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Le Rouzic

author
#0 lore raw

Hi,

Running 2.6.23-rc9-CITI_NFS4_ALL-1 on two Intel X86_64 bi-ways machines as client and server, get the following error with the runtest (-s) of the special tests of the connectathon suite when running it with several other robustness tests (fsx,iozone,fssbfss_stress) in the same time:

Second check for lost reply on non-idempotent requests testing 50 idempotencies in directory "testdir" rmdir 1: Directory not empty
special tests failed

-------------------------------------------------------------
Bug:
http://bugzilla.linux-nfs.org/show_bug.cgi?id=150

Cheers

Signature · 18 lines
-----------------------------------------------------------------
Company : Bull, Architect of an Open World TM (www.bull.com)
Name    : Aime Le Rouzic 
Mail    : Bull - BP 208 - 38432 Echirolles Cedex - France
E-Mail  : [email protected]
Phone   : 33 (4) 76.29.75.51
Fax     : 33 (4) 76.29.75.18
----------------------------------------------------------------- 


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
#1 lore raw
Quoted 14 lines · On Fri, 2007-10-12 at 16:12 +0200, Le Rouzic wrote:

Hi,

Running 2.6.23-rc9-CITI_NFS4_ALL-1 on two Intel X86_64 bi-ways machines as client and server, get the following error with the runtest (-s) of the special tests of the connectathon suite when running it with several other robustness tests (fsx,iozone,fssbfss_stress) in the same time:

Second check for lost reply on non-idempotent requests testing 50 idempotencies in directory "testdir" rmdir 1: Directory not empty
special tests failed

Quoted 5 lines

-------------------------------------------------------------
Bug:
http://bugzilla.linux-nfs.org/show_bug.cgi?id=150

Cheers

I'm hoping that the attached patch will fix this problem. It basically ensures that lookup(), readdir(), and open() cannot race with the sillyrename code.

Cheers
Trond

lookup() and sillyrename() can race one another because the sillyrename() completion cannot take the parent directory's inode->i_mutex since the latter may be held by whoever is calling dput().

We therefore have little option but to add extra locking to ensure that nfs_lookup() and nfs_atomic_open() do not race with the sillyrename completion.
If somebody has looked up the sillyrenamed file in the meantime, we just transfer the sillydelete information to the new dentry.

Signed-off-by Trond Myklebust <[email protected]>

---

 fs/nfs/dir.c           |   14 +++++-
 fs/nfs/inode.c         |    3 +
 fs/nfs/nfs4proc.c      |    6 +++
 fs/nfs/unlink.c        |  114 ++++++++++++++++++++++++++++++++++++++++++------
 include/linux/nfs_fs.h |    8 +++
 5 files changed, 127 insertions(+), 18 deletions(-)
fs/nfs/dir.c +11 -3
index 8ec7fbd..3533453 100644
@@ -562,6 +562,7 @@ static int nfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 	nfs_fattr_init(&fattr);
 	desc->entry = &my_entry;
 
+	nfs_block_sillyrename(dentry);
 	while(!desc->entry->eof) {
 		res = readdir_search_pagecache(desc);
 

@@ -592,6 +593,7 @@ static int nfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 			break;
 		}
 	}
+	nfs_unblock_sillyrename(dentry);
 	unlock_kernel();
 	if (res > 0)
 		res = 0;

@@ -866,6 +868,7 @@ struct dentry_operations nfs_dentry_operations = {
 static struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, struct nameidata *nd)
 {
 	struct dentry *res;
+	struct dentry *parent;
 	struct inode *inode = NULL;
 	int error;
 	struct nfs_fh fhandle;

@@ -894,26 +897,31 @@ static struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, stru
 		goto out_unlock;
 	}
 
+	parent = dentry->d_parent;
+	/* Protect against concurrent sillydeletes */
+	nfs_block_sillyrename(parent);
 	error = NFS_PROTO(dir)->lookup(dir, &dentry->d_name, &fhandle, &fattr);
 	if (error == -ENOENT)
 		goto no_entry;
 	if (error < 0) {
 		res = ERR_PTR(error);
-		goto out_unlock;
+		goto out_unblock_sillyrename;
 	}
 	inode = nfs_fhget(dentry->d_sb, &fhandle, &fattr);
 	res = (struct dentry *)inode;
 	if (IS_ERR(res))
-		goto out_unlock;
+		goto out_unblock_sillyrename;
 
 no_entry:
 	res = d_materialise_unique(dentry, inode);
 	if (res != NULL) {
 		if (IS_ERR(res))
-			goto out_unlock;
+			goto out_unblock_sillyrename;
 		dentry = res;
 	}
 	nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
+out_unblock_sillyrename:
+	nfs_unblock_sillyrename(parent);
 out_unlock:
 	unlock_kernel();
 out:
fs/nfs/inode.c +3 -0
index 035c769..facb065 100644
@@ -1165,6 +1165,9 @@ static void init_once(void * foo, struct kmem_cache * cachep, unsigned long flag
 	INIT_RADIX_TREE(&nfsi->nfs_page_tree, GFP_ATOMIC);
 	nfsi->ncommit = 0;
 	nfsi->npages = 0;
+	atomic_set(&nfsi->silly_count, 1);
+	INIT_HLIST_HEAD(&nfsi->silly_list);
+	init_waitqueue_head(&nfsi->waitqueue);
 	nfs4_init_once(nfsi);
 }
fs/nfs/nfs4proc.c +6 -0
index cb99fd9..2cb3b8b 100644
@@ -1372,6 +1372,7 @@ out_close:
 struct dentry *
 nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 {
+	struct dentry *parent;
 	struct path path = {
 		.mnt = nd->mnt,
 		.dentry = dentry,

@@ -1394,6 +1395,9 @@ nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 	cred = rpcauth_lookupcred(NFS_CLIENT(dir)->cl_auth, 0);
 	if (IS_ERR(cred))
 		return (struct dentry *)cred;
+	parent = dentry->d_parent;
+	/* Protect against concurrent sillydeletes */
+	nfs_block_sillyrename(parent);
 	state = nfs4_do_open(dir, &path, nd->intent.open.flags, &attr, cred);
 	put_rpccred(cred);
 	if (IS_ERR(state)) {

@@ -1401,12 +1405,14 @@ nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
 			d_add(dentry, NULL);
 			nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
 		}
+		nfs_unblock_sillyrename(parent);
 		return (struct dentry *)state;
 	}
 	res = d_add_unique(dentry, igrab(state->inode));
 	if (res != NULL)
 		path.dentry = res;
 	nfs_set_verifier(path.dentry, nfs_save_change_attribute(dir));
+	nfs_unblock_sillyrename(parent);
 	nfs4_intent_set_file(nd, &path, state);
 	return res;
 }
fs/nfs/unlink.c +99 -15

150 lines not on this page · raw message

include/linux/nfs_fs.h +8 -0
index c5164c2..e82a6eb 100644
@@ -160,6 +160,12 @@ struct nfs_inode {
 	/* Open contexts for shared mmap writes */
 	struct list_head	open_files;
 
+	/* Number of in-flight sillydelete RPC calls */
+	atomic_t		silly_count;
+	/* List of deferred sillydelete requests */
+	struct hlist_head	silly_list;
+	wait_queue_head_t	waitqueue;
+
 #ifdef CONFIG_NFS_V4
 	struct nfs4_cached_acl	*nfs4_acl;
         /* NFSv4 state */

@@ -394,6 +400,8 @@ extern void nfs_release_automount_timer(void);
  */
 extern int  nfs_async_unlink(struct inode *dir, struct dentry *dentry);
 extern void nfs_complete_unlink(struct dentry *dentry, struct inode *);
+extern void nfs_block_sillyrename(struct dentry *dentry);
+extern void nfs_unblock_sillyrename(struct dentry *dentry);
 
 /*
  * linux/fs/nfs/write.c

_______________________________________________
NFSv4 mailing list
[email protected]
http://linux-nfs.org/cgi-bin/mailman/listinfo/nfsv4

author
#2 lore raw

Hi,

Unfortunately not the case I was mentioning.
More on

------------------------------------------------------------- =

Bug: =

http://bugzilla.linux-nfs.org/show_bug.cgi?id=3D150 =

=

Cheers

Trond Myklebust a =E9crit :

Quoted 8 lines · On Fri, 2007-10-12 at 16:12 +0200, Le Rouzic wrote:

=

Quoted 4 lines

Hi,

Running 2.6.23-rc9-CITI_NFS4_ALL-1 on two Intel X86_64 bi-ways machines as client and server, get the following error with the runtest (-s) of the s=

pecial

Quoted 1 line
Quoted 1 line

tests of the connectathon suite when running it with several other robust=

ness

Quoted 53 lines
Quoted 12 lines

tests (fsx,iozone,fssbfss_stress) in the same time:

Second check for lost reply on non-idempotent requests testing 50 idempotencies in directory "testdir" rmdir 1: Directory not empty
special tests failed

=

------------------------------------------------------------- =

Quoted 10 lines

Bug: =

http://bugzilla.linux-nfs.org/show_bug.cgi?id=3D150 =

=

Cheers
=

I'm hoping that the attached patch will fix this problem. It basically ensures that lookup(), readdir(), and open() cannot race with the sillyrename code.

Cheers
Trond
=

------------------------------------------------------------------------

Sujet:
No Subject
Exp=E9diteur:
Trond Myklebust <[email protected]>
Date:
Mon, 15 Oct 2007 18:17:53 -0400

lookup() and sillyrename() can race one another because the sillyrename() completion cannot take the parent directory's inode->i_mutex since the latter may be held by whoever is calling dput().

We therefore have little option but to add extra locking to ensure that nfs_lookup() and nfs_atomic_open() do not race with the sillyrename completion.
If somebody has looked up the sillyrenamed file in the meantime, we just transfer the sillydelete information to the new dentry.

Quoted 7 lines
Signed-off-by Trond Myklebust <[email protected]>

---

fs/nfs/dir.c           |   14 +++++-
fs/nfs/inode.c         |    3 +
fs/nfs/nfs4proc.c      |    6 +++
fs/nfs/unlink.c        |  114 ++++++++++++++++++++++++++++++++++++++++++-=

-----

fs/nfs/dir.c static int nfs_readdir(struct file *filp, void *dirent=

Quoted 8 lines

include/linux/nfs_fs.h | 8 +++

5 files changed, 127 insertions(+), 18 deletions(-)
fs/nfs/dir.c +0 -0
index 8ec7fbd..3533453 100644
@@ -562,6 +562,7 @@ static int nfs_readdir(struct file *filp, void *dirent=

, filldir_t filldir)

fs/nfs/dir.c static int nfs_readdir(struct file *filp, void *dirent=

Quoted 10 lines
fs/nfs/dir.c +0 -0
 	nfs_fattr_init(&fattr);
 	desc->entry =3D &my_entry;

=

fs/nfs/dir.c +1 -0
+	nfs_block_sillyrename(dentry);

	while(!desc->entry->eof) {
		res =3D readdir_search_pagecache(desc);

=

fs/nfs/dir.c +0 -0
@@ -592,6 +593,7 @@ static int nfs_readdir(struct file *filp, void *dirent=

, filldir_t filldir)

Quoted 9 lines
fs/nfs/dir.c +1 -0
 			break;
 		}
 	}
+	nfs_unblock_sillyrename(dentry);
 	unlock_kernel();
 	if (res > 0)
 		res =3D 0;

@@ -866,6 +868,7 @@ struct dentry_operations nfs_dentry_operations =3D {

static struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentr=

y, struct nameidata *nd)

fs/nfs/dir.c static struct dentry *nfs_lookup(struct inode *dir, =

Quoted 7 lines
{
	struct dentry *res;
fs/nfs/dir.c +1 -0
+	struct dentry *parent;

	struct inode *inode =3D NULL;
	int error;
	struct nfs_fh fhandle;
fs/nfs/dir.c +0 -0
@@ -894,26 +897,31 @@ static struct dentry *nfs_lookup(struct inode *dir, =

struct dentry * dentry, stru

fs/nfs/inode.c static void init_once(void * foo, struct kmem_cache =

Quoted 41 lines
fs/nfs/dir.c +0 -0
 		goto out_unlock;
 	}

=

fs/nfs/dir.c +3 -0
+	parent =3D dentry->d_parent;
+	/* Protect against concurrent sillydeletes */
+	nfs_block_sillyrename(parent);

	error =3D NFS_PROTO(dir)->lookup(dir, &dentry->d_name, &fhandle, &fattr);
	if (error =3D=3D -ENOENT)
		goto no_entry;
	if (error < 0) {
		res =3D ERR_PTR(error);
fs/nfs/dir.c +1 -1
-		goto out_unlock;
+		goto out_unblock_sillyrename;

	}
	inode =3D nfs_fhget(dentry->d_sb, &fhandle, &fattr);
	res =3D (struct dentry *)inode;
	if (IS_ERR(res))
fs/nfs/dir.c +1 -1
-		goto out_unlock;
+		goto out_unblock_sillyrename;

=

no_entry:

	res =3D d_materialise_unique(dentry, inode);
	if (res !=3D NULL) {
		if (IS_ERR(res))
fs/nfs/dir.c +1 -1
-			goto out_unlock;
+			goto out_unblock_sillyrename;

		dentry =3D res;
	}
	nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
fs/nfs/dir.c +2 -0
+out_unblock_sillyrename:
+	nfs_unblock_sillyrename(parent);

out_unlock:

	unlock_kernel();

out:

fs/nfs/inode.c +0 -0
index 035c769..facb065 100644
@@ -1165,6 +1165,9 @@ static void init_once(void * foo, struct kmem_cache =

* cachep, unsigned long flag

Quoted 15 lines
fs/nfs/inode.c +3 -0
 	INIT_RADIX_TREE(&nfsi->nfs_page_tree, GFP_ATOMIC);
 	nfsi->ncommit =3D 0;
 	nfsi->npages =3D 0;
+	atomic_set(&nfsi->silly_count, 1);
+	INIT_HLIST_HEAD(&nfsi->silly_list);
+	init_waitqueue_head(&nfsi->waitqueue);
 	nfs4_init_once(nfsi);
 }

=

fs/nfs/nfs4proc.c +0 -0
index cb99fd9..2cb3b8b 100644
@@ -1372,6 +1372,7 @@ out_close:
Quoted 2 lines

struct dentry *
nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameida=

ta *nd)

fs/nfs/nfs4proc.c nfs4_atomic_open(struct inode *dir, struct dentry *d=

Quoted 6 lines

{

fs/nfs/nfs4proc.c +1 -0
+	struct dentry *parent;

	struct path path =3D {
		.mnt =3D nd->mnt,
		.dentry =3D dentry,
fs/nfs/nfs4proc.c +0 -0
@@ -1394,6 +1395,9 @@ nfs4_atomic_open(struct inode *dir, struct dentry *d=

entry, struct nameidata *nd)

fs/nfs/nfs4proc.c nfs4_atomic_open(struct inode *dir, struct dentry =

Quoted 10 lines
fs/nfs/nfs4proc.c +3 -0
 	cred =3D rpcauth_lookupcred(NFS_CLIENT(dir)->cl_auth, 0);
 	if (IS_ERR(cred))
 		return (struct dentry *)cred;
+	parent =3D dentry->d_parent;
+	/* Protect against concurrent sillydeletes */
+	nfs_block_sillyrename(parent);
 	state =3D nfs4_do_open(dir, &path, nd->intent.open.flags, &attr, cred);
 	put_rpccred(cred);
 	if (IS_ERR(state)) {

@@ -1401,12 +1405,14 @@ nfs4_atomic_open(struct inode *dir, struct dentry =

*dentry, struct nameidata *nd)

fs/nfs/unlink.c static int nfs_copy_dname(struct dentry *dentry, struct=

Quoted 29 lines
fs/nfs/nfs4proc.c +2 -0
 			d_add(dentry, NULL);
 			nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
 		}
+		nfs_unblock_sillyrename(parent);
 		return (struct dentry *)state;
 	}
 	res =3D d_add_unique(dentry, igrab(state->inode));
 	if (res !=3D NULL)
 		path.dentry =3D res;
 	nfs_set_verifier(path.dentry, nfs_save_change_attribute(dir));
+	nfs_unblock_sillyrename(parent);
 	nfs4_intent_set_file(nd, &path, state);
 	return res;
 }
fs/nfs/unlink.c +0 -0
index 1aed850..6ecd46c 100644
@@ -14,6 +14,7 @@

=

=

struct nfs_unlinkdata {

fs/nfs/unlink.c +1 -0
+	struct hlist_node list;

	struct nfs_removeargs args;
	struct nfs_removeres res;
	struct inode *dir;
fs/nfs/unlink.c +0 -0
@@ -52,6 +53,20 @@ static int nfs_copy_dname(struct dentry *dentry, struct=

nfs_unlinkdata *data)

fs/nfs/unlink.c static void nfs_async_unlink_done(struct rpc_task *task=

Quoted 22 lines
fs/nfs/unlink.c +0 -0
 	return 0;
 }

=

fs/nfs/unlink.c +14 -0
+static void nfs_free_dname(struct nfs_unlinkdata *data)
+{
+	kfree(data->args.name.name);
+	data->args.name.name =3D NULL;
+	data->args.name.len =3D 0;
+}
+
+static void nfs_dec_sillycount(struct inode *dir)
+{
+	struct nfs_inode *nfsi =3D NFS_I(dir);
+	if (atomic_dec_return(&nfsi->silly_count) =3D=3D 1)
+		wake_up(&nfsi->waitqueue);
+}
+

/**
* nfs_async_unlink_init - Initialize the RPC info
* task: rpc_task of the sillydelete

fs/nfs/unlink.c +0 -0
@@ -95,6 +110,8 @@ static void nfs_async_unlink_done(struct rpc_task *task=

, void *calldata)

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops =3D=

Quoted 10 lines
fs/nfs/unlink.c +2 -0
 static void nfs_async_unlink_release(void *calldata)
 {
 	struct nfs_unlinkdata	*data =3D calldata;
+
+	nfs_dec_sillycount(data->dir);
 	nfs_free_unlinkdata(data);
 }

=

fs/nfs/unlink.c +0 -0
@@ -104,33 +121,100 @@ static const struct rpc_call_ops nfs_unlink_ops =3D=

{

Quoted 5 lines
fs/nfs/unlink.c +0 -0
 	.rpc_release =3D nfs_async_unlink_release,
 };

=

-static int nfs_call_unlink(struct dentry *dentry, struct nfs_unlinkdata *=

data)

Quoted 1 line

+static int nfs_do_call_unlink(struct dentry *parent, struct inode *dir, s=

truct nfs_unlinkdata *data)

Quoted 22 lines
{
	struct rpc_task *task;
fs/nfs/unlink.c +20 -0
+	struct dentry *alias;
+
+	alias =3D d_lookup(parent, &data->args.name);
+	if (alias !=3D NULL) {
+		int ret =3D 0;
+		/*
+		 * Hey, we raced with lookup... See if we need to transfer
+		 * the sillyrename information to the aliased dentry.
+		 */
+		nfs_free_dname(data);
+		spin_lock(&alias->d_lock);
+		if (!(alias->d_flags & DCACHE_NFSFS_RENAMED)) {
+			alias->d_fsdata =3D data;
+			alias->d_flags ^=3D DCACHE_NFSFS_RENAMED;
+			ret =3D 1;
+		}
+		spin_unlock(&alias->d_lock);
+		nfs_dec_sillycount(dir);
+		dput(alias);
+		return ret;

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = {

Quoted 10 lines
fs/nfs/unlink.c +10 -0
+	}
+	data->dir =3D igrab(dir);
+	if (!data->dir) {
+		nfs_dec_sillycount(dir);
+		return 0;
+	}
+	data->args.fh =3D NFS_FH(dir);
+	nfs_fattr_init(&data->res.dir_attr);
+
+	task =3D rpc_run_task(NFS_CLIENT(dir), RPC_TASK_ASYNC, &nfs_unlink_ops, =

data);

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = {

Quoted 6 lines
fs/nfs/unlink.c +6 -0
+	if (!IS_ERR(task))
+		rpc_put_task(task);
+	return 1;
+}
+
+static int nfs_call_unlink(struct dentry *dentry, struct nfs_unlinkdata *=

data)

Quoted 45 lines
+{
	struct dentry *parent;
	struct inode *dir;
fs/nfs/unlink.c +1 -0
+	int ret =3D 0;

=

fs/nfs/unlink.c +0 -2
-	if (nfs_copy_dname(dentry, data) < 0)
-		goto out_free;

=

	parent =3D dget_parent(dentry);
	if (parent =3D=3D NULL)
		goto out_free;
fs/nfs/unlink.c +15 -1
-	dir =3D igrab(parent->d_inode);
+	dir =3D parent->d_inode;
+	if (nfs_copy_dname(dentry, data) =3D=3D 0)
+		goto out_dput;
+	/* Non-exclusive lock protects against concurrent lookup() calls */
+	spin_lock(&dir->i_lock);
+	if (atomic_inc_not_zero(&NFS_I(dir)->silly_count) =3D=3D 0) {
+		/* Deferred delete */
+		hlist_add_head(&data->list, &NFS_I(dir)->silly_list);
+		spin_unlock(&dir->i_lock);
+		ret =3D 1;
+		goto out_dput;
+	}
+	spin_unlock(&dir->i_lock);
+	ret =3D nfs_do_call_unlink(parent, dir, data);
+out_dput:

	dput(parent);
fs/nfs/unlink.c +3 -2
-	if (dir =3D=3D NULL)
-		goto out_free;
+out_free:
+	return ret;
+}

=

fs/nfs/unlink.c +3 -3
-	data->dir =3D dir;
-	data->args.fh =3D NFS_FH(dir);
-	nfs_fattr_init(&data->res.dir_attr);
+void nfs_block_sillyrename(struct dentry *dentry)
+{
+	struct nfs_inode *nfsi =3D NFS_I(dentry->d_inode);

=

fs/nfs/unlink.c

Quoted 1 line
fs/nfs/unlink.c +0 -1
-	task =3D rpc_run_task(NFS_CLIENT(dir), RPC_TASK_ASYNC, &nfs_unlink_ops, =

data);

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = {

Quoted 6 lines
fs/nfs/unlink.c +1 -5
-	if (!IS_ERR(task))
-		rpc_put_task(task);
-	return 1;
-out_free:
-	return 0;
+	wait_event(nfsi->waitqueue, atomic_cmpxchg(&nfsi->silly_count, 1, 0) =3D=

=3D 1);

Quoted 4 lines
fs/nfs/unlink.c +4 -0
+}
+
+void nfs_unblock_sillyrename(struct dentry *dentry)
+{

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = {

Quoted 10 lines
fs/nfs/unlink.c +10 -0
+	struct inode *dir =3D dentry->d_inode;
+	struct nfs_inode *nfsi =3D NFS_I(dir);
+	struct nfs_unlinkdata *data;
+
+	atomic_inc(&nfsi->silly_count);
+	spin_lock(&dir->i_lock);
+	while (!hlist_empty(&nfsi->silly_list)) {
+		if (!atomic_inc_not_zero(&nfsi->silly_count))
+			break;
+		data =3D hlist_entry(nfsi->silly_list.first, struct nfs_unlinkdata, lis=

t);

Quoted 37 lines
fs/nfs/unlink.c +7 -0
+		hlist_del(&data->list);
+		spin_unlock(&dir->i_lock);
+		if (nfs_do_call_unlink(dentry, dir, data) =3D=3D 0)
+			nfs_free_unlinkdata(data);
+		spin_lock(&dir->i_lock);
+	}
+	spin_unlock(&dir->i_lock);

}
=

/**

include/linux/nfs_fs.h +0 -0
index c5164c2..e82a6eb 100644
@@ -160,6 +160,12 @@ struct nfs_inode {
 	/* Open contexts for shared mmap writes */
 	struct list_head	open_files;

=

include/linux/nfs_fs.h +6 -0
+	/* Number of in-flight sillydelete RPC calls */
+	atomic_t		silly_count;
+	/* List of deferred sillydelete requests */
+	struct hlist_head	silly_list;
+	wait_queue_head_t	waitqueue;
+

#ifdef CONFIG_NFS_V4
	struct nfs4_cached_acl	*nfs4_acl;
        /* NFSv4 state */
include/linux/nfs_fs.h +2 -0
@@ -394,6 +400,8 @@ extern void nfs_release_automount_timer(void);
 */
 extern int  nfs_async_unlink(struct inode *dir, struct dentry *dentry);
 extern void nfs_complete_unlink(struct dentry *dentry, struct inode *);
+extern void nfs_block_sillyrename(struct dentry *dentry);
+extern void nfs_unblock_sillyrename(struct dentry *dentry);

=

Quoted 5 lines

/*
* linux/fs/nfs/write.c
=

-- =

-----------------------------------------------------------------
Company : Bull, Architect of an Open World TM (www.bull.com)
Name : Aime Le Rouzic =

Mail : Bull - BP 208 - 38432 Echirolles Cedex - France
E-Mail : [email protected]
Phone : 33 (4) 76.29.75.51
Fax : 33 (4) 76.29.75.18
----------------------------------------------------------------- =

author
#3 lore raw

Hi,

It is fixed now thanks to a second patch delivered by Trond.

You will find this second patch at :

Bug: =

http://bugzilla.linux-nfs.org/show_bug.cgi?id=3D150 =

=

Regards

Le Rouzic a =E9crit :

Quoted 15 lines

Hi,

Unfortunately not the case I was mentioning.
More on

------------------------------------------------------------- =

Bug: =

http://bugzilla.linux-nfs.org/show_bug.cgi?id=3D150 =

=

Quoted 19 lines

Cheers

Trond Myklebust a =E9crit :

=

Quoted 10 lines · On Fri, 2007-10-12 at 16:12 +0200, Le Rouzic wrote:

=

   =
Quoted 3 lines

Hi,

Running 2.6.23-rc9-CITI_NFS4_ALL-1 on two Intel X86_64 bi-ways machines =

as

Quoted 1 line
Quoted 1 line
Quoted 1 line

client and server, get the following error with the runtest (-s) of the =

special

Quoted 1 line
Quoted 1 line
Quoted 1 line

tests of the connectathon suite when running it with several other robus=

tness

Quoted 54 lines
Quoted 54 lines
Quoted 15 lines

tests (fsx,iozone,fssbfss_stress) in the same time:

Second check for lost reply on non-idempotent requests testing 50 idempotencies in directory "testdir" rmdir 1: Directory not empty
special tests failed

------------------------------------------------------------- =

Bug: =

http://bugzilla.linux-nfs.org/show_bug.cgi?id=3D150 =

Quoted 9 lines

=

Cheers
=

     =

I'm hoping that the attached patch will fix this problem. It basically ensures that lookup(), readdir(), and open() cannot race with the sillyrename code.

Cheers
Trond
=

------------------------------------------------------------------------

Sujet:
No Subject
Exp=E9diteur:
Trond Myklebust <[email protected]>
Date:
Mon, 15 Oct 2007 18:17:53 -0400

lookup() and sillyrename() can race one another because the sillyrename() completion cannot take the parent directory's inode->i_mutex since the latter may be held by whoever is calling dput().

We therefore have little option but to add extra locking to ensure that nfs_lookup() and nfs_atomic_open() do not race with the sillyrename completion.
If somebody has looked up the sillyrenamed file in the meantime, we just transfer the sillydelete information to the new dentry.

Quoted 7 lines
Quoted 7 lines
Signed-off-by Trond Myklebust <[email protected]>

---

fs/nfs/dir.c           |   14 +++++-
fs/nfs/inode.c         |    3 +
fs/nfs/nfs4proc.c      |    6 +++
fs/nfs/unlink.c        |  114 ++++++++++++++++++++++++++++++++++++++++++-=

-----

Quoted 8 lines

fs/nfs/dir.c static int nfs_readdir(struct file *filp, void *diren=

Quoted 8 lines

include/linux/nfs_fs.h | 8 +++

5 files changed, 127 insertions(+), 18 deletions(-)
fs/nfs/dir.c +0 -0
index 8ec7fbd..3533453 100644
@@ -562,6 +562,7 @@ static int nfs_readdir(struct file *filp, void *diren=

t, filldir_t filldir)

Quoted 8 lines

fs/nfs/dir.c static int nfs_readdir(struct file *filp, void *diren=

Quoted 8 lines
fs/nfs/dir.c +1 -0
 	nfs_fattr_init(&fattr);
 	desc->entry =3D &my_entry;
 
+	nfs_block_sillyrename(dentry);
 	while(!desc->entry->eof) {
 		res =3D readdir_search_pagecache(desc);
 

@@ -592,6 +593,7 @@ static int nfs_readdir(struct file *filp, void *diren=

t, filldir_t filldir)

Quoted 9 lines
Quoted 9 lines
fs/nfs/dir.c +1 -0
 			break;
 		}
 	}
+	nfs_unblock_sillyrename(dentry);
 	unlock_kernel();
 	if (res > 0)
 		res =3D 0;

@@ -866,6 +868,7 @@ struct dentry_operations nfs_dentry_operations =3D {

static struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentr=

y, struct nameidata *nd)

Quoted 7 lines

fs/nfs/dir.c static struct dentry *nfs_lookup(struct inode *dir,=

Quoted 7 lines
{
	struct dentry *res;
fs/nfs/dir.c +1 -0
+	struct dentry *parent;

	struct inode *inode =3D NULL;
	int error;
	struct nfs_fh fhandle;
fs/nfs/dir.c +0 -0
@@ -894,26 +897,31 @@ static struct dentry *nfs_lookup(struct inode *dir,=

struct dentry * dentry, stru

Quoted 31 lines
Quoted 31 lines
fs/nfs/dir.c +5 -2
 		goto out_unlock;
 	}
 
+	parent =3D dentry->d_parent;
+	/* Protect against concurrent sillydeletes */
+	nfs_block_sillyrename(parent);
 	error =3D NFS_PROTO(dir)->lookup(dir, &dentry->d_name, &fhandle, &fattr);
 	if (error =3D=3D -ENOENT)
 		goto no_entry;
 	if (error < 0) {
 		res =3D ERR_PTR(error);
-		goto out_unlock;
+		goto out_unblock_sillyrename;
 	}
 	inode =3D nfs_fhget(dentry->d_sb, &fhandle, &fattr);
 	res =3D (struct dentry *)inode;
 	if (IS_ERR(res))
-		goto out_unlock;
+		goto out_unblock_sillyrename;

no_entry:

	res =3D d_materialise_unique(dentry, inode);
	if (res !=3D NULL) {
		if (IS_ERR(res))
fs/nfs/dir.c +1 -1
-			goto out_unlock;
+			goto out_unblock_sillyrename;

		dentry =3D res;
	}
	nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
fs/nfs/dir.c +2 -0
+out_unblock_sillyrename:
+	nfs_unblock_sillyrename(parent);
Quoted 8 lines

fs/nfs/inode.c static void init_once(void * foo, struct kmem_cache=

Quoted 8 lines

out_unlock:

	unlock_kernel();

out:

fs/nfs/inode.c +0 -0
index 035c769..facb065 100644
@@ -1165,6 +1165,9 @@ static void init_once(void * foo, struct kmem_cache=

* cachep, unsigned long flag

Quoted 9 lines
Quoted 9 lines
fs/nfs/inode.c +3 -0
 	INIT_RADIX_TREE(&nfsi->nfs_page_tree, GFP_ATOMIC);
 	nfsi->ncommit =3D 0;
 	nfsi->npages =3D 0;
+	atomic_set(&nfsi->silly_count, 1);
+	INIT_HLIST_HEAD(&nfsi->silly_list);
+	init_waitqueue_head(&nfsi->waitqueue);
 	nfs4_init_once(nfsi);
 }
Quoted 7 lines
Quoted 7 lines
fs/nfs/nfs4proc.c +0 -0
index cb99fd9..2cb3b8b 100644
@@ -1372,6 +1372,7 @@ out_close:

struct dentry *
nfs4_atomic_open(struct inode *dir, struct dentry *dentry, struct nameida=

ta *nd)

Quoted 6 lines

fs/nfs/nfs4proc.c nfs4_atomic_open(struct inode *dir, struct dentry *=

Quoted 6 lines

{

fs/nfs/nfs4proc.c +1 -0
+	struct dentry *parent;

	struct path path =3D {
		.mnt =3D nd->mnt,
		.dentry =3D dentry,
fs/nfs/nfs4proc.c +0 -0
@@ -1394,6 +1395,9 @@ nfs4_atomic_open(struct inode *dir, struct dentry *=

dentry, struct nameidata *nd)

Quoted 10 lines

fs/nfs/nfs4proc.c nfs4_atomic_open(struct inode *dir, struct dentry=

Quoted 10 lines
fs/nfs/nfs4proc.c +3 -0
 	cred =3D rpcauth_lookupcred(NFS_CLIENT(dir)->cl_auth, 0);
 	if (IS_ERR(cred))
 		return (struct dentry *)cred;
+	parent =3D dentry->d_parent;
+	/* Protect against concurrent sillydeletes */
+	nfs_block_sillyrename(parent);
 	state =3D nfs4_do_open(dir, &path, nd->intent.open.flags, &attr, cred);
 	put_rpccred(cred);
 	if (IS_ERR(state)) {

@@ -1401,12 +1405,14 @@ nfs4_atomic_open(struct inode *dir, struct dentry=

*dentry, struct nameidata *nd)

Quoted 21 lines
Quoted 21 lines
fs/nfs/nfs4proc.c +2 -0
 			d_add(dentry, NULL);
 			nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
 		}
+		nfs_unblock_sillyrename(parent);
 		return (struct dentry *)state;
 	}
 	res =3D d_add_unique(dentry, igrab(state->inode));
 	if (res !=3D NULL)
 		path.dentry =3D res;
 	nfs_set_verifier(path.dentry, nfs_save_change_attribute(dir));
+	nfs_unblock_sillyrename(parent);
 	nfs4_intent_set_file(nd, &path, state);
 	return res;
 }
fs/nfs/unlink.c +0 -0
index 1aed850..6ecd46c 100644
@@ -14,6 +14,7 @@
Quoted 6 lines

fs/nfs/unlink.c static int nfs_copy_dname(struct dentry *dentry, struc=

Quoted 6 lines
fs/nfs/unlink.c +1 -0
 struct nfs_unlinkdata {
+	struct hlist_node list;
 	struct nfs_removeargs args;
 	struct nfs_removeres res;
 	struct inode *dir;

@@ -52,6 +53,20 @@ static int nfs_copy_dname(struct dentry *dentry, struc=

t nfs_unlinkdata *data)

Quoted 13 lines
Quoted 13 lines
fs/nfs/unlink.c +10 -0
 	return 0;
 }
 
+static void nfs_free_dname(struct nfs_unlinkdata *data)
+{
+	kfree(data->args.name.name);
+	data->args.name.name =3D NULL;
+	data->args.name.len =3D 0;
+}
+
+static void nfs_dec_sillycount(struct inode *dir)
+{
+	struct nfs_inode *nfsi =3D NFS_I(dir);
Quoted 8 lines

fs/nfs/unlink.c static void nfs_async_unlink_done(struct rpc_task *tas=

Quoted 8 lines
fs/nfs/unlink.c +4 -0
+	if (atomic_dec_return(&nfsi->silly_count) =3D=3D 1)
+		wake_up(&nfsi->waitqueue);
+}
+
 /**
 * nfs_async_unlink_init - Initialize the RPC info
 * task: rpc_task of the sillydelete

@@ -95,6 +110,8 @@ static void nfs_async_unlink_done(struct rpc_task *tas=

k, void *calldata)

Quoted 9 lines

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops =

Quoted 9 lines
fs/nfs/unlink.c +2 -0
 static void nfs_async_unlink_release(void *calldata)
 {
 	struct nfs_unlinkdata	*data =3D calldata;
+
+	nfs_dec_sillycount(data->dir);
 	nfs_free_unlinkdata(data);
 }
 

@@ -104,33 +121,100 @@ static const struct rpc_call_ops nfs_unlink_ops =

=3D {

Quoted 4 lines

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = · line 107

Quoted 4 lines
fs/nfs/unlink.c +0 -1
 	.rpc_release =3D nfs_async_unlink_release,
 };
 
-static int nfs_call_unlink(struct dentry *dentry, struct nfs_unlinkdata =

*data)

Quoted 1 line

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = · line 124

Quoted 1 line
fs/nfs/unlink.c +1 -0
+static int nfs_do_call_unlink(struct dentry *parent, struct inode *dir, =

struct nfs_unlinkdata *data)

Quoted 24 lines
Quoted 24 lines
fs/nfs/unlink.c +22 -0
 {
 	struct rpc_task *task;
+	struct dentry *alias;
+
+	alias =3D d_lookup(parent, &data->args.name);
+	if (alias !=3D NULL) {
+		int ret =3D 0;
+		/*
+		 * Hey, we raced with lookup... See if we need to transfer
+		 * the sillyrename information to the aliased dentry.
+		 */
+		nfs_free_dname(data);
+		spin_lock(&alias->d_lock);
+		if (!(alias->d_flags & DCACHE_NFSFS_RENAMED)) {
+			alias->d_fsdata =3D data;
+			alias->d_flags ^=3D DCACHE_NFSFS_RENAMED;
+			ret =3D 1;
+		}
+		spin_unlock(&alias->d_lock);
+		nfs_dec_sillycount(dir);
+		dput(alias);
+		return ret;
+	}
+	data->dir =3D igrab(dir);
Quoted 8 lines

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = {

Quoted 8 lines
fs/nfs/unlink.c +8 -0
+	if (!data->dir) {
+		nfs_dec_sillycount(dir);
+		return 0;
+	}
+	data->args.fh =3D NFS_FH(dir);
+	nfs_fattr_init(&data->res.dir_attr);
+
+	task =3D rpc_run_task(NFS_CLIENT(dir), RPC_TASK_ASYNC, &nfs_unlink_ops,=

data);

Quoted 6 lines

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = {

Quoted 6 lines
fs/nfs/unlink.c +6 -0
+	if (!IS_ERR(task))
+		rpc_put_task(task);
+	return 1;
+}
+
+static int nfs_call_unlink(struct dentry *dentry, struct nfs_unlinkdata =

*data)

Quoted 34 lines
Quoted 34 lines
fs/nfs/unlink.c +20 -5
+{
 	struct dentry *parent;
 	struct inode *dir;
+	int ret =3D 0;
 
-	if (nfs_copy_dname(dentry, data) < 0)
-		goto out_free;
 
 	parent =3D dget_parent(dentry);
 	if (parent =3D=3D NULL)
 		goto out_free;
-	dir =3D igrab(parent->d_inode);
+	dir =3D parent->d_inode;
+	if (nfs_copy_dname(dentry, data) =3D=3D 0)
+		goto out_dput;
+	/* Non-exclusive lock protects against concurrent lookup() calls */
+	spin_lock(&dir->i_lock);
+	if (atomic_inc_not_zero(&NFS_I(dir)->silly_count) =3D=3D 0) {
+		/* Deferred delete */
+		hlist_add_head(&data->list, &NFS_I(dir)->silly_list);
+		spin_unlock(&dir->i_lock);
+		ret =3D 1;
+		goto out_dput;
+	}
+	spin_unlock(&dir->i_lock);
+	ret =3D nfs_do_call_unlink(parent, dir, data);
+out_dput:
 	dput(parent);
-	if (dir =3D=3D NULL)
-		goto out_free;
+out_free:
+	return ret;
+}
Quoted 8 lines

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = {

Quoted 8 lines
fs/nfs/unlink.c +3 -4
-	data->dir =3D dir;
-	data->args.fh =3D NFS_FH(dir);
-	nfs_fattr_init(&data->res.dir_attr);
+void nfs_block_sillyrename(struct dentry *dentry)
+{
+	struct nfs_inode *nfsi =3D NFS_I(dentry->d_inode);
 
-	task =3D rpc_run_task(NFS_CLIENT(dir), RPC_TASK_ASYNC, &nfs_unlink_ops,=

data);

Quoted 6 lines

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = {

Quoted 6 lines
fs/nfs/unlink.c +1 -5
-	if (!IS_ERR(task))
-		rpc_put_task(task);
-	return 1;
-out_free:
-	return 0;
+	wait_event(nfsi->waitqueue, atomic_cmpxchg(&nfsi->silly_count, 1, 0) =

=3D=3D 1);

Quoted 6 lines
Quoted 6 lines
fs/nfs/unlink.c +6 -0
+}
+
+void nfs_unblock_sillyrename(struct dentry *dentry)
+{
+	struct inode *dir =3D dentry->d_inode;
+	struct nfs_inode *nfsi =3D NFS_I(dir);
Quoted 8 lines

fs/nfs/unlink.c static const struct rpc_call_ops nfs_unlink_ops = {

Quoted 8 lines
fs/nfs/unlink.c +8 -0
+	struct nfs_unlinkdata *data;
+
+	atomic_inc(&nfsi->silly_count);
+	spin_lock(&dir->i_lock);
+	while (!hlist_empty(&nfsi->silly_list)) {
+		if (!atomic_inc_not_zero(&nfsi->silly_count))
+			break;
+		data =3D hlist_entry(nfsi->silly_list.first, struct nfs_unlinkdata, li=

st);

Quoted 39 lines
Quoted 39 lines
fs/nfs/unlink.c +7 -0
+		hlist_del(&data->list);
+		spin_unlock(&dir->i_lock);
+		if (nfs_do_call_unlink(dentry, dir, data) =3D=3D 0)
+			nfs_free_unlinkdata(data);
+		spin_lock(&dir->i_lock);
+	}
+	spin_unlock(&dir->i_lock);
 }
 
 /**
include/linux/nfs_fs.h +8 -0
index c5164c2..e82a6eb 100644
@@ -160,6 +160,12 @@ struct nfs_inode {
 	/* Open contexts for shared mmap writes */
 	struct list_head	open_files;
 
+	/* Number of in-flight sillydelete RPC calls */
+	atomic_t		silly_count;
+	/* List of deferred sillydelete requests */
+	struct hlist_head	silly_list;
+	wait_queue_head_t	waitqueue;
+
 #ifdef CONFIG_NFS_V4
 	struct nfs4_cached_acl	*nfs4_acl;
       /* NFSv4 state */

@@ -394,6 +400,8 @@ extern void nfs_release_automount_timer(void);
 */
 extern int  nfs_async_unlink(struct inode *dir, struct dentry *dentry);
 extern void nfs_complete_unlink(struct dentry *dentry, struct inode *);
+extern void nfs_block_sillyrename(struct dentry *dentry);
+extern void nfs_unblock_sillyrename(struct dentry *dentry);
 
 /*
 * linux/fs/nfs/write.c

=

Quoted 8 lines
Quoted 3 lines
   =

=

-- =

-----------------------------------------------------------------
Company : Bull, Architect of an Open World TM (www.bull.com)
Name : Aime Le Rouzic =

Mail : Bull - BP 208 - 38432 Echirolles Cedex - France
E-Mail : [email protected]
Phone : 33 (4) 76.29.75.51
Fax : 33 (4) 76.29.75.18
----------------------------------------------------------------- =