mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 03/11] NFS: Fix error handling in nfs_direct_write_result()
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
                   ` (3 preceding siblings ...)
  2006-10-19 17:04 ` [PATCH 02/11] NFS: Fix oops in nfs_cancel_commit_list Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-19 17:04 ` [PATCH 06/11] NFS: Deal with failure of invalidate_inode_pages2() Trond Myklebust
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Trond Myklebust <Trond.Myklebust@netapp.com>

If the RPC call tanked, we should not be checking the return value
of data->res.verf->committed, since it is unlikely to even be
initialised.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/direct.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 9f7f8b9..1e873fc 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -532,10 +532,12 @@ static void nfs_direct_write_result(stru
 
 	spin_lock(&dreq->lock);
 
-	if (likely(status >= 0))
-		dreq->count += data->res.count;
-	else
-		dreq->error = task->tk_status;
+	if (unlikely(status < 0)) {
+		dreq->error = status;
+		goto out_unlock;
+	}
+
+	dreq->count += data->res.count;
 
 	if (data->res.verf->committed != NFS_FILE_SYNC) {
 		switch (dreq->flags) {
@@ -550,7 +552,7 @@ static void nfs_direct_write_result(stru
 				}
 		}
 	}
-
+out_unlock:
 	spin_unlock(&dreq->lock);
 }
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 04/11] nfs4: initialize cl_ipaddr
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
                   ` (7 preceding siblings ...)
  2006-10-19 17:04 ` [PATCH 08/11] NFS: __nfs_revalidate_inode() can use "inode" before checking it is non-NULL Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-19 17:04 ` [PATCH 07/11] NFS: fix minor bug in new NFS symlink code Trond Myklebust
  9 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: J. Bruce Fields <bfields@fieldses.org>

David forgot to do this.  I'm not sure if this is the right place to put
it....

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/client.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 34c3996..8b123f6 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -849,6 +849,7 @@ #ifdef CONFIG_NFS_V4
  */
 static int nfs4_init_client(struct nfs_client *clp,
 		int proto, int timeo, int retrans,
+		const char *ip_addr,
 		rpc_authflavor_t authflavour)
 {
 	int error;
@@ -865,6 +866,7 @@ static int nfs4_init_client(struct nfs_c
 	error = nfs_create_rpc_client(clp, proto, timeo, retrans, authflavour);
 	if (error < 0)
 		goto error;
+	memcpy(clp->cl_ipaddr, ip_addr, sizeof(clp->cl_ipaddr));
 
 	error = nfs_idmap_new(clp);
 	if (error < 0) {
@@ -888,6 +890,7 @@ error:
  */
 static int nfs4_set_client(struct nfs_server *server,
 		const char *hostname, const struct sockaddr_in *addr,
+		const char *ip_addr,
 		rpc_authflavor_t authflavour,
 		int proto, int timeo, int retrans)
 {
@@ -902,7 +905,7 @@ static int nfs4_set_client(struct nfs_se
 		error = PTR_ERR(clp);
 		goto error;
 	}
-	error = nfs4_init_client(clp, proto, timeo, retrans, authflavour);
+	error = nfs4_init_client(clp, proto, timeo, retrans, ip_addr, authflavour);
 	if (error < 0)
 		goto error_put;
 
@@ -971,7 +974,7 @@ struct nfs_server *nfs4_create_server(co
 		return ERR_PTR(-ENOMEM);
 
 	/* Get a client record */
-	error = nfs4_set_client(server, hostname, addr, authflavour,
+	error = nfs4_set_client(server, hostname, addr, ip_addr, authflavour,
 			data->proto, data->timeo, data->retrans);
 	if (error < 0)
 		goto error;
@@ -1041,6 +1044,7 @@ struct nfs_server *nfs4_create_referral_
 	/* Get a client representation.
 	 * Note: NFSv4 always uses TCP, */
 	error = nfs4_set_client(server, data->hostname, data->addr,
+			parent_client->cl_ipaddr,
 			data->authflavor,
 			parent_server->client->cl_xprt->prot,
 			parent_client->retrans_timeo,

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 07/11] NFS: fix minor bug in new NFS symlink code
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
                   ` (8 preceding siblings ...)
  2006-10-19 17:04 ` [PATCH 04/11] nfs4: initialize cl_ipaddr Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-20  4:25   ` Andrew Morton
  9 siblings, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Chuck Lever <chuck.lever@oracle.com>

The original code confused a zero return code from pagevec_add() as
success.

Test plan:
None.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/dir.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 58d4405..c86a1ea 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1519,8 +1519,8 @@ static int nfs_symlink(struct inode *dir
 	pagevec_init(&lru_pvec, 0);
 	if (!add_to_page_cache(page, dentry->d_inode->i_mapping, 0,
 							GFP_KERNEL)) {
-		if (!pagevec_add(&lru_pvec, page))
-			__pagevec_lru_add(&lru_pvec);
+		pagevec_add(&lru_pvec, page);
+		pagevec_lru_add(&lru_pvec);
 		SetPageUptodate(page);
 		unlock_page(page);
 	} else

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 09/11] NFS: remove unused check in nfs4_open_revalidate
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
  2006-10-19 17:04 ` [PATCH 11/11] SUNRPC: fix a typo Trond Myklebust
  2006-10-19 17:04 ` [PATCH 05/11] NFS: Fix NFSv4 callback regression Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-19 17:04 ` [PATCH 02/11] NFS: Fix oops in nfs_cancel_commit_list Trond Myklebust
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Chuck Lever <chuck.lever@oracle.com>

Coverity spotted a superfluous error check in nfs4_open_revalidate().
Remove it.

Coverity: #cid 847

Test plan:
Code inspection; another pass through Coverity.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/nfs4proc.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 47c7e6e..7421bcb 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1314,11 +1314,9 @@ nfs4_open_revalidate(struct inode *dir, 
 			case -EROFS:
 				lookup_instantiate_filp(nd, (struct dentry *)state, NULL);
 				return 1;
-			case -ENOENT:
-				if (dentry->d_inode == NULL)
-					return 1;
+			default:
+				goto out_drop;
 		}
-		goto out_drop;
 	}
 	if (state->inode == dentry->d_inode) {
 		nfs4_intent_set_file(nd, dentry, state);

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 08/11] NFS: __nfs_revalidate_inode() can use "inode" before checking it is non-NULL
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
                   ` (6 preceding siblings ...)
  2006-10-19 17:04 ` [PATCH 10/11] SUNRPC: fix race in in-kernel RPC portmapper client Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-19 17:04 ` [PATCH 04/11] nfs4: initialize cl_ipaddr Trond Myklebust
  2006-10-19 17:04 ` [PATCH 07/11] NFS: fix minor bug in new NFS symlink code Trond Myklebust
  9 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Chuck Lever <chuck.lever@oracle.com>

The "!inode" check in __nfs_revalidate_inode() occurs well after the first
time it is dereferenced, so get rid of it.

Coverity: #cid 1372, 1373

Test plan:
Code review; recheck with Coverity.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/inode.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 9979ad1..08cc4c5 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -583,7 +583,7 @@ __nfs_revalidate_inode(struct nfs_server
 
 	nfs_inc_stats(inode, NFSIOS_INODEREVALIDATE);
 	lock_kernel();
-	if (!inode || is_bad_inode(inode))
+	if (is_bad_inode(inode))
  		goto out_nowait;
 	if (NFS_STALE(inode))
  		goto out_nowait;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 06/11] NFS: Deal with failure of invalidate_inode_pages2()
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
                   ` (4 preceding siblings ...)
  2006-10-19 17:04 ` [PATCH 03/11] NFS: Fix error handling in nfs_direct_write_result() Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-19 17:04 ` [PATCH 10/11] SUNRPC: fix race in in-kernel RPC portmapper client Trond Myklebust
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Trond Myklebust <Trond.Myklebust@netapp.com>

If invalidate_inode_pages2() fails, then it should in principle just be
because the current process was signalled. In that case, we just want to
ensure that the inode's page cache remains marked as invalid.

Also add a helper to allow the O_DIRECT code to simply mark the page cache
as invalid once it is finished writing, instead of calling
invalidate_inode_pages2() itself.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/dir.c           |    6 ++++--
 fs/nfs/direct.c        |   13 ++-----------
 fs/nfs/inode.c         |   28 +++++++++++++++++++++++-----
 include/linux/nfs_fs.h |    1 +
 4 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 481f889..58d4405 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -203,8 +203,10 @@ int nfs_readdir_filler(nfs_readdir_descr
 	 * Note: assumes we have exclusive access to this mapping either
 	 *	 through inode->i_mutex or some other mechanism.
 	 */
-	if (page->index == 0)
-		invalidate_inode_pages2_range(inode->i_mapping, PAGE_CACHE_SIZE, -1);
+	if (page->index == 0 && invalidate_inode_pages2_range(inode->i_mapping, PAGE_CACHE_SIZE, -1) < 0) {
+		/* Should never happen */
+		nfs_zap_mapping(inode, inode->i_mapping);
+	}
 	unlock_page(page);
 	return 0;
  error:
diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 1e873fc..bdfabf8 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -497,6 +497,7 @@ static void nfs_direct_write_complete(st
 			if (dreq->commit_data != NULL)
 				nfs_commit_free(dreq->commit_data);
 			nfs_direct_free_writedata(dreq);
+			nfs_zap_mapping(inode, inode->i_mapping);
 			nfs_direct_complete(dreq);
 	}
 }
@@ -517,6 +518,7 @@ static void nfs_direct_write_complete(st
 {
 	nfs_end_data_update(inode);
 	nfs_direct_free_writedata(dreq);
+	nfs_zap_mapping(inode, inode->i_mapping);
 	nfs_direct_complete(dreq);
 }
 #endif
@@ -830,17 +832,6 @@ ssize_t nfs_file_direct_write(struct kio
 
 	retval = nfs_direct_write(iocb, (unsigned long) buf, count, pos);
 
-	/*
-	 * XXX: nfs_end_data_update() already ensures this file's
-	 *      cached data is subsequently invalidated.  Do we really
-	 *      need to call invalidate_inode_pages2() again here?
-	 *
-	 *      For aio writes, this invalidation will almost certainly
-	 *      occur before the writes complete.  Kind of racey.
-	 */
-	if (mapping->nrpages)
-		invalidate_inode_pages2(mapping);
-
 	if (retval > 0)
 		iocb->ki_pos = pos + retval;
 
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index bc9376c..9979ad1 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -131,6 +131,15 @@ void nfs_zap_caches(struct inode *inode)
 	spin_unlock(&inode->i_lock);
 }
 
+void nfs_zap_mapping(struct inode *inode, struct address_space *mapping)
+{
+	if (mapping->nrpages != 0) {
+		spin_lock(&inode->i_lock);
+		NFS_I(inode)->cache_validity |= NFS_INO_INVALID_DATA;
+		spin_unlock(&inode->i_lock);
+	}
+}
+
 static void nfs_zap_acl_cache(struct inode *inode)
 {
 	void (*clear_acl_cache)(struct inode *);
@@ -671,13 +680,20 @@ int nfs_revalidate_mapping(struct inode 
 	if ((nfsi->cache_validity & NFS_INO_REVAL_PAGECACHE)
 			|| nfs_attribute_timeout(inode))
 		ret = __nfs_revalidate_inode(NFS_SERVER(inode), inode);
+	if (ret < 0)
+		goto out;
 
 	if (nfsi->cache_validity & NFS_INO_INVALID_DATA) {
-		nfs_inc_stats(inode, NFSIOS_DATAINVALIDATE);
-		if (S_ISREG(inode->i_mode))
-			nfs_sync_mapping(mapping);
-		invalidate_inode_pages2(mapping);
-
+		if (mapping->nrpages != 0) {
+			if (S_ISREG(inode->i_mode)) {
+				ret = nfs_sync_mapping(mapping);
+				if (ret < 0)
+					goto out;
+			}
+			ret = invalidate_inode_pages2(mapping);
+			if (ret < 0)
+				goto out;
+		}
 		spin_lock(&inode->i_lock);
 		nfsi->cache_validity &= ~NFS_INO_INVALID_DATA;
 		if (S_ISDIR(inode->i_mode)) {
@@ -687,10 +703,12 @@ int nfs_revalidate_mapping(struct inode 
 		}
 		spin_unlock(&inode->i_lock);
 
+		nfs_inc_stats(inode, NFSIOS_DATAINVALIDATE);
 		dfprintk(PAGECACHE, "NFS: (%s/%Ld) data cache invalidated\n",
 				inode->i_sb->s_id,
 				(long long)NFS_FILEID(inode));
 	}
+out:
 	return ret;
 }
 
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 76ff548..6b2de1b 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -290,6 +290,7 @@ static inline int nfs_verify_change_attr
  * linux/fs/nfs/inode.c
  */
 extern int nfs_sync_mapping(struct address_space *mapping);
+extern void nfs_zap_mapping(struct inode *inode, struct address_space *mapping);
 extern void nfs_zap_caches(struct inode *);
 extern struct inode *nfs_fhget(struct super_block *, struct nfs_fh *,
 				struct nfs_fattr *);

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c
@ 2006-10-19 17:04 Trond Myklebust
  2006-10-19 17:04 ` [PATCH 11/11] SUNRPC: fix a typo Trond Myklebust
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Duh. addr.sin_port should be in network byte order.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/super.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 28659a9..28108c8 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -834,7 +834,7 @@ static int nfs4_get_sb(struct file_syste
 	}
 	/* RFC3530: The default port for NFS is 2049 */
 	if (addr.sin_port == 0)
-		addr.sin_port = NFS_PORT;
+		addr.sin_port = htons(NFS_PORT);
 
 	/* Grab the authentication type */
 	authflavour = RPC_AUTH_UNIX;

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 05/11] NFS: Fix NFSv4 callback regression
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
  2006-10-19 17:04 ` [PATCH 11/11] SUNRPC: fix a typo Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-19 17:04 ` [PATCH 09/11] NFS: remove unused check in nfs4_open_revalidate Trond Myklebust
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Trond Myklebust <Trond.Myklebust@netapp.com>

The change in semantics for nfs_find_client() introduced by David breaks
the NFSv4 callback channel.

Also, replace another completely broken BUG_ON() in nfs_find_client().
In initialised clients, clp->cl_cons_state == 0, and callers of that
function should in any case never want to see clients that are
uninitialised.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/client.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 8b123f6..5fea638 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -232,11 +232,15 @@ void nfs_put_client(struct nfs_client *c
  * Find a client by address
  * - caller must hold nfs_client_lock
  */
-static struct nfs_client *__nfs_find_client(const struct sockaddr_in *addr, int nfsversion)
+static struct nfs_client *__nfs_find_client(const struct sockaddr_in *addr, int nfsversion, int match_port)
 {
 	struct nfs_client *clp;
 
 	list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
+		/* Don't match clients that failed to initialise properly */
+		if (clp->cl_cons_state < 0)
+			continue;
+
 		/* Different NFS versions cannot share the same nfs_client */
 		if (clp->cl_nfsversion != nfsversion)
 			continue;
@@ -245,7 +249,7 @@ static struct nfs_client *__nfs_find_cli
 			   sizeof(clp->cl_addr.sin_addr)) != 0)
 			continue;
 
-		if (clp->cl_addr.sin_port == addr->sin_port)
+		if (!match_port || clp->cl_addr.sin_port == addr->sin_port)
 			goto found;
 	}
 
@@ -265,11 +269,12 @@ struct nfs_client *nfs_find_client(const
 	struct nfs_client *clp;
 
 	spin_lock(&nfs_client_lock);
-	clp = __nfs_find_client(addr, nfsversion);
+	clp = __nfs_find_client(addr, nfsversion, 0);
 	spin_unlock(&nfs_client_lock);
-
-	BUG_ON(clp && clp->cl_cons_state == 0);
-
+	if (clp != NULL && clp->cl_cons_state != NFS_CS_READY) {
+		nfs_put_client(clp);
+		clp = NULL;
+	}
 	return clp;
 }
 
@@ -292,7 +297,7 @@ static struct nfs_client *nfs_get_client
 	do {
 		spin_lock(&nfs_client_lock);
 
-		clp = __nfs_find_client(addr, nfsversion);
+		clp = __nfs_find_client(addr, nfsversion, 1);
 		if (clp)
 			goto found_client;
 		if (new)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 02/11] NFS: Fix oops in nfs_cancel_commit_list
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
                   ` (2 preceding siblings ...)
  2006-10-19 17:04 ` [PATCH 09/11] NFS: remove unused check in nfs4_open_revalidate Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-19 17:04 ` [PATCH 03/11] NFS: Fix error handling in nfs_direct_write_result() Trond Myklebust
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Fix two bugs:
 - nfs_inode_remove_request will call nfs_clear_request, so we cannot
   reference req->wb_page after it. Move the call to dec_zone_page_state so
   that it occurs while req->wb_page is still valid.
 - Calling nfs_clear_page_writeback is unnecessary since the radix tree
   tags will have been cleared by the call to nfs_inode_remove_request.
   Replace with a simple call to nfs_unlock_request.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/write.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index f6675d2..d80fa8c 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -588,10 +588,10 @@ static void nfs_cancel_commit_list(struc
 
 	while(!list_empty(head)) {
 		req = nfs_list_entry(head->next);
+		dec_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
 		nfs_list_remove_request(req);
 		nfs_inode_remove_request(req);
-		dec_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
-		nfs_clear_page_writeback(req);
+		nfs_unlock_request(req);
 	}
 }
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 10/11] SUNRPC: fix race in in-kernel RPC portmapper client
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
                   ` (5 preceding siblings ...)
  2006-10-19 17:04 ` [PATCH 06/11] NFS: Deal with failure of invalidate_inode_pages2() Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-19 17:04 ` [PATCH 08/11] NFS: __nfs_revalidate_inode() can use "inode" before checking it is non-NULL Trond Myklebust
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Chuck Lever <chuck.lever@oracle.com>

When submitting a request to a fast portmapper (such as the local rpcbind
daemon), the request can complete before the parent task is even queued up
on xprt->binding.  Fix this by queuing before submitting the rpcbind
request.

Test plan:
Connectathon locking test with UDP.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 net/sunrpc/pmap_clnt.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/sunrpc/pmap_clnt.c b/net/sunrpc/pmap_clnt.c
index 919d5ba..e52afab 100644
--- a/net/sunrpc/pmap_clnt.c
+++ b/net/sunrpc/pmap_clnt.c
@@ -101,11 +101,13 @@ void rpc_getport(struct rpc_task *task)
 	/* Autobind on cloned rpc clients is discouraged */
 	BUG_ON(clnt->cl_parent != clnt);
 
-	if (xprt_test_and_set_binding(xprt)) {
-		task->tk_status = -EACCES;	/* tell caller to check again */
-		rpc_sleep_on(&xprt->binding, task, NULL, NULL);
-		return;
-	}
+	/* Put self on queue before sending rpcbind request, in case
+	 * pmap_getport_done completes before we return from rpc_run_task */
+	rpc_sleep_on(&xprt->binding, task, NULL, NULL);
+
+	status = -EACCES;		/* tell caller to check again */
+	if (xprt_test_and_set_binding(xprt))
+		goto bailout_nofree;
 
 	/* Someone else may have bound if we slept */
 	status = 0;
@@ -134,8 +136,6 @@ void rpc_getport(struct rpc_task *task)
 		goto bailout;
 	rpc_release_task(child);
 
-	rpc_sleep_on(&xprt->binding, task, NULL, NULL);
-
 	task->tk_xprt->stat.bind_count++;
 	return;
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 11/11] SUNRPC: fix a typo
  2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
@ 2006-10-19 17:04 ` Trond Myklebust
  2006-10-19 17:04 ` [PATCH 05/11] NFS: Fix NFSv4 callback regression Trond Myklebust
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2006-10-19 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, nfs

From: Chuck Lever <chuck.lever@oracle.com>

Yes, this actually passed tests the way it was.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 net/sunrpc/xprtsock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 28100e0..757fc91 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1366,7 +1366,7 @@ int xs_setup_udp(struct rpc_xprt *xprt, 
 	if (xprt->slot == NULL)
 		return -ENOMEM;
 
-	if (ntohs(addr->sin_port != 0))
+	if (ntohs(addr->sin_port) != 0)
 		xprt_set_bound(xprt);
 	xprt->port = xs_get_random_port();
 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 07/11] NFS: fix minor bug in new NFS symlink code
  2006-10-19 17:04 ` [PATCH 07/11] NFS: fix minor bug in new NFS symlink code Trond Myklebust
@ 2006-10-20  4:25   ` Andrew Morton
  2006-10-20  4:57     ` [NFS] " Chuck Lever
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2006-10-20  4:25 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Linus Torvalds, linux-kernel, nfs

On Thu, 19 Oct 2006 13:04:32 -0400
Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 58d4405..c86a1ea 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1519,8 +1519,8 @@ static int nfs_symlink(struct inode *dir
>  	pagevec_init(&lru_pvec, 0);
>  	if (!add_to_page_cache(page, dentry->d_inode->i_mapping, 0,
>  							GFP_KERNEL)) {
> -		if (!pagevec_add(&lru_pvec, page))
> -			__pagevec_lru_add(&lru_pvec);
> +		pagevec_add(&lru_pvec, page);
> +		pagevec_lru_add(&lru_pvec);
>  		SetPageUptodate(page);
>  		unlock_page(page);
>  	} else

One could export add_to_page_cache_lru() to modules..

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [NFS] [PATCH 07/11] NFS: fix minor bug in new NFS symlink code
  2006-10-20  4:25   ` Andrew Morton
@ 2006-10-20  4:57     ` Chuck Lever
  0 siblings, 0 replies; 13+ messages in thread
From: Chuck Lever @ 2006-10-20  4:57 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Trond Myklebust, Linus Torvalds, nfs, linux-kernel

On 10/20/06, Andrew Morton <akpm@osdl.org> wrote:
> On Thu, 19 Oct 2006 13:04:32 -0400
> Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
>
> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index 58d4405..c86a1ea 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -1519,8 +1519,8 @@ static int nfs_symlink(struct inode *dir
> >       pagevec_init(&lru_pvec, 0);
> >       if (!add_to_page_cache(page, dentry->d_inode->i_mapping, 0,
> >                                                       GFP_KERNEL)) {
> > -             if (!pagevec_add(&lru_pvec, page))
> > -                     __pagevec_lru_add(&lru_pvec);
> > +             pagevec_add(&lru_pvec, page);
> > +             pagevec_lru_add(&lru_pvec);
> >               SetPageUptodate(page);
> >               unlock_page(page);
> >       } else
>
> One could export add_to_page_cache_lru() to modules..

I assumed there was probably a good reason that this had not already been done.

-- 
"We who cut mere stones must always be envisioning cathedrals"
   -- Quarry worker's creed

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2006-10-20  4:57 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-19 17:04 [PATCH 01/11] NFSv4: Fix thinko in fs/nfs/super.c Trond Myklebust
2006-10-19 17:04 ` [PATCH 11/11] SUNRPC: fix a typo Trond Myklebust
2006-10-19 17:04 ` [PATCH 05/11] NFS: Fix NFSv4 callback regression Trond Myklebust
2006-10-19 17:04 ` [PATCH 09/11] NFS: remove unused check in nfs4_open_revalidate Trond Myklebust
2006-10-19 17:04 ` [PATCH 02/11] NFS: Fix oops in nfs_cancel_commit_list Trond Myklebust
2006-10-19 17:04 ` [PATCH 03/11] NFS: Fix error handling in nfs_direct_write_result() Trond Myklebust
2006-10-19 17:04 ` [PATCH 06/11] NFS: Deal with failure of invalidate_inode_pages2() Trond Myklebust
2006-10-19 17:04 ` [PATCH 10/11] SUNRPC: fix race in in-kernel RPC portmapper client Trond Myklebust
2006-10-19 17:04 ` [PATCH 08/11] NFS: __nfs_revalidate_inode() can use "inode" before checking it is non-NULL Trond Myklebust
2006-10-19 17:04 ` [PATCH 04/11] nfs4: initialize cl_ipaddr Trond Myklebust
2006-10-19 17:04 ` [PATCH 07/11] NFS: fix minor bug in new NFS symlink code Trond Myklebust
2006-10-20  4:25   ` Andrew Morton
2006-10-20  4:57     ` [NFS] " Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®