From: Trond Myklebust <[email protected]>
Duh. addr.sin_port should be in network byte order.
Signed-off-by: Trond Myklebust <[email protected]>
---
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;
From: Trond Myklebust <[email protected]>
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 <[email protected]>
---
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);
}
From: J. Bruce Fields <[email protected]>
David forgot to do this. I'm not sure if this is the right place to put
it....
Signed-off-by: J. Bruce Fields <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
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,
From: Chuck Lever <[email protected]>
The original code confused a zero return code from pagevec_add() as
success.
Test plan:
None.
Signed-off-by: Chuck Lever <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
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
From: Chuck Lever <[email protected]>
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 <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
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);
From: Chuck Lever <[email protected]>
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 <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
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;
From: Trond Myklebust <[email protected]>
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 <[email protected]>
---
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 *);
From: Trond Myklebust <[email protected]>
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 <[email protected]>
---
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)
From: Trond Myklebust <[email protected]>
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 <[email protected]>
---
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);
}
}
From: Chuck Lever <[email protected]>
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 <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
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;
From: Chuck Lever <[email protected]>
Yes, this actually passed tests the way it was.
Signed-off-by: Chuck Lever <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
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();
On Thu, 19 Oct 2006 13:04:32 -0400
Trond Myklebust <[email protected]> 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..
On 10/20/06, Andrew Morton <[email protected]> wrote:
> On Thu, 19 Oct 2006 13:04:32 -0400
> Trond Myklebust <[email protected]> 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