* [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session
@ 2011-04-20 23:50 Trond Myklebust
2011-04-20 23:50 ` [PATCH 2/3] SUNRPC: Allow RPC calls to return ETIMEDOUT instead of EIO Trond Myklebust
2011-04-21 15:39 ` [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session Chuck Lever
0 siblings, 2 replies; 6+ messages in thread
From: Trond Myklebust @ 2011-04-20 23:50 UTC (permalink / raw)
To: linux-nfs
If a server for some reason keeps sending NFS4ERR_DELAY errors, we can end
up looping forever inside nfs4_proc_create_session, and so the usual
mechanisms for detecting if the nfs_client is dead don't work.
Fix this by ensuring that we loop inside the nfs4_state_manager thread
instead.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
fs/nfs/nfs4_fs.h | 1 +
fs/nfs/nfs4proc.c | 84 ++++++++++++---------------------------------
fs/nfs/nfs4state.c | 47 +++++++++++++++++--------
include/linux/nfs_fs_sb.h | 1 +
4 files changed, 56 insertions(+), 77 deletions(-)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index e1c261d..c4a6983 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -47,6 +47,7 @@ enum nfs4_client_state {
NFS4CLNT_LAYOUTRECALL,
NFS4CLNT_SESSION_RESET,
NFS4CLNT_RECALL_SLOT,
+ NFS4CLNT_LEASE_CONFIRM,
};
enum nfs4_session_state {
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 628e35f..2507f1f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3726,46 +3726,36 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
.rpc_cred = cred,
};
__be32 *p;
- int loop = 0;
int status;
p = (__be32*)sc_verifier.data;
*p++ = htonl((u32)clp->cl_boot_time.tv_sec);
*p = htonl((u32)clp->cl_boot_time.tv_nsec);
- for(;;) {
- setclientid.sc_name_len = scnprintf(setclientid.sc_name,
- sizeof(setclientid.sc_name), "%s/%s %s %s %u",
- clp->cl_ipaddr,
- rpc_peeraddr2str(clp->cl_rpcclient,
- RPC_DISPLAY_ADDR),
- rpc_peeraddr2str(clp->cl_rpcclient,
- RPC_DISPLAY_PROTO),
- clp->cl_rpcclient->cl_auth->au_ops->au_name,
- clp->cl_id_uniquifier);
- setclientid.sc_netid_len = scnprintf(setclientid.sc_netid,
- sizeof(setclientid.sc_netid),
- rpc_peeraddr2str(clp->cl_rpcclient,
- RPC_DISPLAY_NETID));
- setclientid.sc_uaddr_len = scnprintf(setclientid.sc_uaddr,
- sizeof(setclientid.sc_uaddr), "%s.%u.%u",
- clp->cl_ipaddr, port >> 8, port & 255);
-
- status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
- if (status != -NFS4ERR_CLID_INUSE)
- break;
- if (signalled())
- break;
- if (loop++ & 1)
- ssleep(clp->cl_lease_time / HZ + 1);
- else
- if (++clp->cl_id_uniquifier == 0)
- break;
- }
+ setclientid.sc_name_len = scnprintf(setclientid.sc_name,
+ sizeof(setclientid.sc_name), "%s/%s %s %s %u",
+ clp->cl_ipaddr,
+ rpc_peeraddr2str(clp->cl_rpcclient,
+ RPC_DISPLAY_ADDR),
+ rpc_peeraddr2str(clp->cl_rpcclient,
+ RPC_DISPLAY_PROTO),
+ clp->cl_rpcclient->cl_auth->au_ops->au_name,
+ clp->cl_id_uniquifier);
+ setclientid.sc_netid_len = scnprintf(setclientid.sc_netid,
+ sizeof(setclientid.sc_netid),
+ rpc_peeraddr2str(clp->cl_rpcclient,
+ RPC_DISPLAY_NETID));
+ setclientid.sc_uaddr_len = scnprintf(setclientid.sc_uaddr,
+ sizeof(setclientid.sc_uaddr), "%s.%u.%u",
+ clp->cl_ipaddr, port >> 8, port & 255);
+
+ status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
+ if (status == -NFS4ERR_CLID_INUSE)
+ ssleep(clp->cl_lease_time / HZ + 1);
return status;
}
-static int _nfs4_proc_setclientid_confirm(struct nfs_client *clp,
+int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
struct nfs4_setclientid_res *arg,
struct rpc_cred *cred)
{
@@ -3790,26 +3780,6 @@ static int _nfs4_proc_setclientid_confirm(struct nfs_client *clp,
return status;
}
-int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
- struct nfs4_setclientid_res *arg,
- struct rpc_cred *cred)
-{
- long timeout = 0;
- int err;
- do {
- err = _nfs4_proc_setclientid_confirm(clp, arg, cred);
- switch (err) {
- case 0:
- return err;
- case -NFS4ERR_RESOURCE:
- /* The IBM lawyers misread another document! */
- case -NFS4ERR_DELAY:
- err = nfs4_delay(clp->cl_rpcclient, &timeout);
- }
- } while (err == 0);
- return err;
-}
-
struct nfs4_delegreturndata {
struct nfs4_delegreturnargs args;
struct nfs4_delegreturnres res;
@@ -5222,20 +5192,10 @@ int nfs4_proc_create_session(struct nfs_client *clp)
int status;
unsigned *ptr;
struct nfs4_session *session = clp->cl_session;
- long timeout = 0;
- int err;
dprintk("--> %s clp=%p session=%p\n", __func__, clp, session);
- do {
- status = _nfs4_proc_create_session(clp);
- if (status == -NFS4ERR_DELAY) {
- err = nfs4_delay(clp->cl_rpcclient, &timeout);
- if (err)
- status = err;
- }
- } while (status == -NFS4ERR_DELAY);
-
+ status = _nfs4_proc_create_session(clp);
if (status)
goto out;
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 4dfb34b..9a7f9df 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -64,10 +64,15 @@ static LIST_HEAD(nfs4_clientid_list);
int nfs4_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
{
- struct nfs4_setclientid_res clid;
+ struct nfs4_setclientid_res clid = {
+ .clientid = clp->cl_clientid,
+ .confirm = clp->cl_confirm,
+ };
unsigned short port;
int status;
+ if (test_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state))
+ goto do_confirm;
port = nfs_callback_tcpport;
if (clp->cl_addr.ss_family == AF_INET6)
port = nfs_callback_tcpport6;
@@ -75,10 +80,14 @@ int nfs4_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
status = nfs4_proc_setclientid(clp, NFS4_CALLBACK, port, cred, &clid);
if (status != 0)
goto out;
+ clp->cl_clientid = clid.clientid;
+ clp->cl_confirm = clid.confirm;
+ set_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
+do_confirm:
status = nfs4_proc_setclientid_confirm(clp, &clid, cred);
if (status != 0)
goto out;
- clp->cl_clientid = clid.clientid;
+ clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
nfs4_schedule_state_renewal(clp);
out:
return status;
@@ -230,13 +239,18 @@ int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
{
int status;
+ if (test_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state))
+ goto do_confirm;
nfs4_begin_drain_session(clp);
status = nfs4_proc_exchange_id(clp, cred);
if (status != 0)
goto out;
+ set_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
+do_confirm:
status = nfs4_proc_create_session(clp);
if (status != 0)
goto out;
+ clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
nfs41_setup_state_renewal(clp);
nfs_mark_client_ready(clp, NFS_CS_READY);
out:
@@ -1584,20 +1598,23 @@ static int nfs4_recall_slot(struct nfs_client *clp) { return 0; }
*/
static void nfs4_set_lease_expired(struct nfs_client *clp, int status)
{
- if (nfs4_has_session(clp)) {
- switch (status) {
- case -NFS4ERR_DELAY:
- case -NFS4ERR_CLID_INUSE:
- case -EAGAIN:
- break;
+ switch (status) {
+ case -NFS4ERR_CLID_INUSE:
+ clp->cl_id_uniquifier++;
+ case -NFS4ERR_STALE_CLIENTID:
+ clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
+ break;
+ case -NFS4ERR_DELAY:
+ case -EAGAIN:
+ ssleep(1);
+ break;
- case -EKEYEXPIRED:
- nfs4_warn_keyexpired(clp->cl_hostname);
- case -NFS4ERR_NOT_SAME: /* FixMe: implement recovery
- * in nfs4_exchange_id */
- default:
- return;
- }
+ case -EKEYEXPIRED:
+ nfs4_warn_keyexpired(clp->cl_hostname);
+ case -NFS4ERR_NOT_SAME: /* FixMe: implement recovery
+ * in nfs4_exchange_id */
+ default:
+ return;
}
set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
}
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 216cea5..87694ca 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -47,6 +47,7 @@ struct nfs_client {
#ifdef CONFIG_NFS_V4
u64 cl_clientid; /* constant */
+ nfs4_verifier cl_confirm; /* Clientid verifier */
unsigned long cl_state;
spinlock_t cl_lock;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] SUNRPC: Allow RPC calls to return ETIMEDOUT instead of EIO
2011-04-20 23:50 [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session Trond Myklebust
@ 2011-04-20 23:50 ` Trond Myklebust
2011-04-20 23:50 ` [PATCH 3/3] NFSv4: Ensure that clientid and session establishment can time out Trond Myklebust
2011-04-21 15:39 ` [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session Chuck Lever
1 sibling, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2011-04-20 23:50 UTC (permalink / raw)
To: linux-nfs
On occasion, it is useful for the NFS layer to distinguish between
soft timeouts and other EIO errors due to (say) encoding errors,
or authentication errors.
The following patch ensures that the default behaviour of the RPC
layer remains to return EIO on soft timeouts (until we have
audited all the callers).
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
include/linux/sunrpc/sched.h | 3 ++-
net/sunrpc/clnt.c | 5 ++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
index 3b94f80..f73c482 100644
--- a/include/linux/sunrpc/sched.h
+++ b/include/linux/sunrpc/sched.h
@@ -128,12 +128,13 @@ struct rpc_task_setup {
#define RPC_TASK_SOFT 0x0200 /* Use soft timeouts */
#define RPC_TASK_SOFTCONN 0x0400 /* Fail if can't connect */
#define RPC_TASK_SENT 0x0800 /* message was sent */
+#define RPC_TASK_TIMEOUT 0x1000 /* fail with ETIMEDOUT on timeout */
#define RPC_IS_ASYNC(t) ((t)->tk_flags & RPC_TASK_ASYNC)
#define RPC_IS_SWAPPER(t) ((t)->tk_flags & RPC_TASK_SWAPPER)
#define RPC_DO_ROOTOVERRIDE(t) ((t)->tk_flags & RPC_TASK_ROOTCREDS)
#define RPC_ASSASSINATED(t) ((t)->tk_flags & RPC_TASK_KILLED)
-#define RPC_IS_SOFT(t) ((t)->tk_flags & RPC_TASK_SOFT)
+#define RPC_IS_SOFT(t) ((t)->tk_flags & (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
#define RPC_IS_SOFTCONN(t) ((t)->tk_flags & RPC_TASK_SOFTCONN)
#define RPC_WAS_SENT(t) ((t)->tk_flags & RPC_TASK_SENT)
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index e7a96e4..8d83f9d 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1508,7 +1508,10 @@ call_timeout(struct rpc_task *task)
if (clnt->cl_chatty)
printk(KERN_NOTICE "%s: server %s not responding, timed out\n",
clnt->cl_protname, clnt->cl_server);
- rpc_exit(task, -EIO);
+ if (task->tk_flags & RPC_TASK_TIMEOUT)
+ rpc_exit(task, -ETIMEDOUT);
+ else
+ rpc_exit(task, -EIO);
return;
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] NFSv4: Ensure that clientid and session establishment can time out
2011-04-20 23:50 ` [PATCH 2/3] SUNRPC: Allow RPC calls to return ETIMEDOUT instead of EIO Trond Myklebust
@ 2011-04-20 23:50 ` Trond Myklebust
0 siblings, 0 replies; 6+ messages in thread
From: Trond Myklebust @ 2011-04-20 23:50 UTC (permalink / raw)
To: linux-nfs
The following patch ensures that we do not get permanently trapped in
the RPC layer when trying to establish a new client id or session.
This again ensures that the state manager can finish in a timely
fashion when the last filesystem to reference the nfs_client exits.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
fs/nfs/nfs4proc.c | 13 +++++++------
fs/nfs/nfs4state.c | 1 +
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 2507f1f..afccc9e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3749,7 +3749,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
sizeof(setclientid.sc_uaddr), "%s.%u.%u",
clp->cl_ipaddr, port >> 8, port & 255);
- status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
+ status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
if (status == -NFS4ERR_CLID_INUSE)
ssleep(clp->cl_lease_time / HZ + 1);
return status;
@@ -3770,7 +3770,7 @@ int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
int status;
now = jiffies;
- status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
+ status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
if (status == 0) {
spin_lock(&clp->cl_lock);
clp->cl_lease_time = fsinfo.lease_time * HZ;
@@ -4784,7 +4784,7 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
init_utsname()->domainname,
clp->cl_rpcclient->cl_auth->au_flavor);
- status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
+ status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
if (!status)
status = nfs4_check_cl_exchange_flags(clp->cl_exchange_flags);
dprintk("<-- %s status= %d\n", __func__, status);
@@ -4867,7 +4867,8 @@ int nfs4_proc_get_lease_time(struct nfs_client *clp, struct nfs_fsinfo *fsinfo)
.rpc_client = clp->cl_rpcclient,
.rpc_message = &msg,
.callback_ops = &nfs4_get_lease_time_ops,
- .callback_data = &data
+ .callback_data = &data,
+ .flags = RPC_TASK_TIMEOUT,
};
int status;
@@ -5169,7 +5170,7 @@ static int _nfs4_proc_create_session(struct nfs_client *clp)
nfs4_init_channel_attrs(&args);
args.flags = (SESSION4_PERSIST | SESSION4_BACK_CHAN);
- status = rpc_call_sync(session->clp->cl_rpcclient, &msg, 0);
+ status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
if (!status)
/* Verify the session's negotiated channel_attrs values */
@@ -5236,7 +5237,7 @@ int nfs4_proc_destroy_session(struct nfs4_session *session)
msg.rpc_argp = session;
msg.rpc_resp = NULL;
msg.rpc_cred = NULL;
- status = rpc_call_sync(session->clp->cl_rpcclient, &msg, 0);
+ status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
if (status)
printk(KERN_WARNING
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 9a7f9df..530b99f 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1605,6 +1605,7 @@ static void nfs4_set_lease_expired(struct nfs_client *clp, int status)
clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
break;
case -NFS4ERR_DELAY:
+ case -ETIMEDOUT:
case -EAGAIN:
ssleep(1);
break;
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session
2011-04-20 23:50 [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session Trond Myklebust
2011-04-20 23:50 ` [PATCH 2/3] SUNRPC: Allow RPC calls to return ETIMEDOUT instead of EIO Trond Myklebust
@ 2011-04-21 15:39 ` Chuck Lever
2011-04-23 19:27 ` Trond Myklebust
1 sibling, 1 reply; 6+ messages in thread
From: Chuck Lever @ 2011-04-21 15:39 UTC (permalink / raw)
To: Trond Myklebust; +Cc: linux-nfs
Hi-
This will require some significant human intervention to merge with my code that caches the long-form client ID.
On Apr 20, 2011, at 7:50 PM, Trond Myklebust wrote:
> If a server for some reason keeps sending NFS4ERR_DELAY errors, we can end
> up looping forever inside nfs4_proc_create_session, and so the usual
> mechanisms for detecting if the nfs_client is dead don't work.
>
> Fix this by ensuring that we loop inside the nfs4_state_manager thread
> instead.
>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
> fs/nfs/nfs4_fs.h | 1 +
> fs/nfs/nfs4proc.c | 84 ++++++++++++---------------------------------
> fs/nfs/nfs4state.c | 47 +++++++++++++++++--------
> include/linux/nfs_fs_sb.h | 1 +
> 4 files changed, 56 insertions(+), 77 deletions(-)
>
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index e1c261d..c4a6983 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -47,6 +47,7 @@ enum nfs4_client_state {
> NFS4CLNT_LAYOUTRECALL,
> NFS4CLNT_SESSION_RESET,
> NFS4CLNT_RECALL_SLOT,
> + NFS4CLNT_LEASE_CONFIRM,
> };
>
> enum nfs4_session_state {
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 628e35f..2507f1f 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3726,46 +3726,36 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> .rpc_cred = cred,
> };
> __be32 *p;
> - int loop = 0;
> int status;
>
> p = (__be32*)sc_verifier.data;
> *p++ = htonl((u32)clp->cl_boot_time.tv_sec);
> *p = htonl((u32)clp->cl_boot_time.tv_nsec);
>
> - for(;;) {
> - setclientid.sc_name_len = scnprintf(setclientid.sc_name,
> - sizeof(setclientid.sc_name), "%s/%s %s %s %u",
> - clp->cl_ipaddr,
> - rpc_peeraddr2str(clp->cl_rpcclient,
> - RPC_DISPLAY_ADDR),
> - rpc_peeraddr2str(clp->cl_rpcclient,
> - RPC_DISPLAY_PROTO),
> - clp->cl_rpcclient->cl_auth->au_ops->au_name,
> - clp->cl_id_uniquifier);
> - setclientid.sc_netid_len = scnprintf(setclientid.sc_netid,
> - sizeof(setclientid.sc_netid),
> - rpc_peeraddr2str(clp->cl_rpcclient,
> - RPC_DISPLAY_NETID));
> - setclientid.sc_uaddr_len = scnprintf(setclientid.sc_uaddr,
> - sizeof(setclientid.sc_uaddr), "%s.%u.%u",
> - clp->cl_ipaddr, port >> 8, port & 255);
> -
> - status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
> - if (status != -NFS4ERR_CLID_INUSE)
> - break;
> - if (signalled())
> - break;
> - if (loop++ & 1)
> - ssleep(clp->cl_lease_time / HZ + 1);
> - else
> - if (++clp->cl_id_uniquifier == 0)
> - break;
> - }
> + setclientid.sc_name_len = scnprintf(setclientid.sc_name,
> + sizeof(setclientid.sc_name), "%s/%s %s %s %u",
> + clp->cl_ipaddr,
> + rpc_peeraddr2str(clp->cl_rpcclient,
> + RPC_DISPLAY_ADDR),
> + rpc_peeraddr2str(clp->cl_rpcclient,
> + RPC_DISPLAY_PROTO),
> + clp->cl_rpcclient->cl_auth->au_ops->au_name,
> + clp->cl_id_uniquifier);
> + setclientid.sc_netid_len = scnprintf(setclientid.sc_netid,
> + sizeof(setclientid.sc_netid),
> + rpc_peeraddr2str(clp->cl_rpcclient,
> + RPC_DISPLAY_NETID));
> + setclientid.sc_uaddr_len = scnprintf(setclientid.sc_uaddr,
> + sizeof(setclientid.sc_uaddr), "%s.%u.%u",
> + clp->cl_ipaddr, port >> 8, port & 255);
> +
> + status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
> + if (status == -NFS4ERR_CLID_INUSE)
> + ssleep(clp->cl_lease_time / HZ + 1);
> return status;
> }
>
> -static int _nfs4_proc_setclientid_confirm(struct nfs_client *clp,
> +int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
> struct nfs4_setclientid_res *arg,
> struct rpc_cred *cred)
> {
> @@ -3790,26 +3780,6 @@ static int _nfs4_proc_setclientid_confirm(struct nfs_client *clp,
> return status;
> }
>
> -int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
> - struct nfs4_setclientid_res *arg,
> - struct rpc_cred *cred)
> -{
> - long timeout = 0;
> - int err;
> - do {
> - err = _nfs4_proc_setclientid_confirm(clp, arg, cred);
> - switch (err) {
> - case 0:
> - return err;
> - case -NFS4ERR_RESOURCE:
> - /* The IBM lawyers misread another document! */
> - case -NFS4ERR_DELAY:
> - err = nfs4_delay(clp->cl_rpcclient, &timeout);
> - }
> - } while (err == 0);
> - return err;
> -}
> -
> struct nfs4_delegreturndata {
> struct nfs4_delegreturnargs args;
> struct nfs4_delegreturnres res;
> @@ -5222,20 +5192,10 @@ int nfs4_proc_create_session(struct nfs_client *clp)
> int status;
> unsigned *ptr;
> struct nfs4_session *session = clp->cl_session;
> - long timeout = 0;
> - int err;
>
> dprintk("--> %s clp=%p session=%p\n", __func__, clp, session);
>
> - do {
> - status = _nfs4_proc_create_session(clp);
> - if (status == -NFS4ERR_DELAY) {
> - err = nfs4_delay(clp->cl_rpcclient, &timeout);
> - if (err)
> - status = err;
> - }
> - } while (status == -NFS4ERR_DELAY);
> -
> + status = _nfs4_proc_create_session(clp);
> if (status)
> goto out;
>
> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> index 4dfb34b..9a7f9df 100644
> --- a/fs/nfs/nfs4state.c
> +++ b/fs/nfs/nfs4state.c
> @@ -64,10 +64,15 @@ static LIST_HEAD(nfs4_clientid_list);
>
> int nfs4_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
> {
> - struct nfs4_setclientid_res clid;
> + struct nfs4_setclientid_res clid = {
> + .clientid = clp->cl_clientid,
> + .confirm = clp->cl_confirm,
> + };
> unsigned short port;
> int status;
>
> + if (test_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state))
> + goto do_confirm;
> port = nfs_callback_tcpport;
> if (clp->cl_addr.ss_family == AF_INET6)
> port = nfs_callback_tcpport6;
> @@ -75,10 +80,14 @@ int nfs4_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
> status = nfs4_proc_setclientid(clp, NFS4_CALLBACK, port, cred, &clid);
> if (status != 0)
> goto out;
> + clp->cl_clientid = clid.clientid;
> + clp->cl_confirm = clid.confirm;
> + set_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> +do_confirm:
> status = nfs4_proc_setclientid_confirm(clp, &clid, cred);
> if (status != 0)
> goto out;
> - clp->cl_clientid = clid.clientid;
> + clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> nfs4_schedule_state_renewal(clp);
> out:
> return status;
> @@ -230,13 +239,18 @@ int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
> {
> int status;
>
> + if (test_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state))
> + goto do_confirm;
> nfs4_begin_drain_session(clp);
> status = nfs4_proc_exchange_id(clp, cred);
> if (status != 0)
> goto out;
> + set_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> +do_confirm:
> status = nfs4_proc_create_session(clp);
> if (status != 0)
> goto out;
> + clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> nfs41_setup_state_renewal(clp);
> nfs_mark_client_ready(clp, NFS_CS_READY);
> out:
> @@ -1584,20 +1598,23 @@ static int nfs4_recall_slot(struct nfs_client *clp) { return 0; }
> */
> static void nfs4_set_lease_expired(struct nfs_client *clp, int status)
> {
> - if (nfs4_has_session(clp)) {
> - switch (status) {
> - case -NFS4ERR_DELAY:
> - case -NFS4ERR_CLID_INUSE:
> - case -EAGAIN:
> - break;
> + switch (status) {
> + case -NFS4ERR_CLID_INUSE:
> + clp->cl_id_uniquifier++;
> + case -NFS4ERR_STALE_CLIENTID:
> + clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> + break;
> + case -NFS4ERR_DELAY:
> + case -EAGAIN:
> + ssleep(1);
> + break;
>
> - case -EKEYEXPIRED:
> - nfs4_warn_keyexpired(clp->cl_hostname);
> - case -NFS4ERR_NOT_SAME: /* FixMe: implement recovery
> - * in nfs4_exchange_id */
> - default:
> - return;
> - }
> + case -EKEYEXPIRED:
> + nfs4_warn_keyexpired(clp->cl_hostname);
> + case -NFS4ERR_NOT_SAME: /* FixMe: implement recovery
> + * in nfs4_exchange_id */
> + default:
> + return;
> }
> set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
> }
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 216cea5..87694ca 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -47,6 +47,7 @@ struct nfs_client {
>
> #ifdef CONFIG_NFS_V4
> u64 cl_clientid; /* constant */
> + nfs4_verifier cl_confirm; /* Clientid verifier */
> unsigned long cl_state;
>
> spinlock_t cl_lock;
> --
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session
2011-04-21 15:39 ` [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session Chuck Lever
@ 2011-04-23 19:27 ` Trond Myklebust
2011-04-25 13:51 ` Chuck Lever
0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2011-04-23 19:27 UTC (permalink / raw)
To: Chuck Lever; +Cc: linux-nfs
On Thu, 2011-04-21 at 11:39 -0400, Chuck Lever wrote:
> Hi-
>
> This will require some significant human intervention to merge with my code that caches the long-form client ID.
How so? It really isn't doing much more than removing those 'for(;;)'
loops.
> On Apr 20, 2011, at 7:50 PM, Trond Myklebust wrote:
>
> > If a server for some reason keeps sending NFS4ERR_DELAY errors, we can end
> > up looping forever inside nfs4_proc_create_session, and so the usual
> > mechanisms for detecting if the nfs_client is dead don't work.
> >
> > Fix this by ensuring that we loop inside the nfs4_state_manager thread
> > instead.
> >
> > Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> > ---
> > fs/nfs/nfs4_fs.h | 1 +
> > fs/nfs/nfs4proc.c | 84 ++++++++++++---------------------------------
> > fs/nfs/nfs4state.c | 47 +++++++++++++++++--------
> > include/linux/nfs_fs_sb.h | 1 +
> > 4 files changed, 56 insertions(+), 77 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> > index e1c261d..c4a6983 100644
> > --- a/fs/nfs/nfs4_fs.h
> > +++ b/fs/nfs/nfs4_fs.h
> > @@ -47,6 +47,7 @@ enum nfs4_client_state {
> > NFS4CLNT_LAYOUTRECALL,
> > NFS4CLNT_SESSION_RESET,
> > NFS4CLNT_RECALL_SLOT,
> > + NFS4CLNT_LEASE_CONFIRM,
> > };
> >
> > enum nfs4_session_state {
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 628e35f..2507f1f 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -3726,46 +3726,36 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
> > .rpc_cred = cred,
> > };
> > __be32 *p;
> > - int loop = 0;
> > int status;
> >
> > p = (__be32*)sc_verifier.data;
> > *p++ = htonl((u32)clp->cl_boot_time.tv_sec);
> > *p = htonl((u32)clp->cl_boot_time.tv_nsec);
> >
> > - for(;;) {
> > - setclientid.sc_name_len = scnprintf(setclientid.sc_name,
> > - sizeof(setclientid.sc_name), "%s/%s %s %s %u",
> > - clp->cl_ipaddr,
> > - rpc_peeraddr2str(clp->cl_rpcclient,
> > - RPC_DISPLAY_ADDR),
> > - rpc_peeraddr2str(clp->cl_rpcclient,
> > - RPC_DISPLAY_PROTO),
> > - clp->cl_rpcclient->cl_auth->au_ops->au_name,
> > - clp->cl_id_uniquifier);
> > - setclientid.sc_netid_len = scnprintf(setclientid.sc_netid,
> > - sizeof(setclientid.sc_netid),
> > - rpc_peeraddr2str(clp->cl_rpcclient,
> > - RPC_DISPLAY_NETID));
> > - setclientid.sc_uaddr_len = scnprintf(setclientid.sc_uaddr,
> > - sizeof(setclientid.sc_uaddr), "%s.%u.%u",
> > - clp->cl_ipaddr, port >> 8, port & 255);
> > -
> > - status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
> > - if (status != -NFS4ERR_CLID_INUSE)
> > - break;
> > - if (signalled())
> > - break;
> > - if (loop++ & 1)
> > - ssleep(clp->cl_lease_time / HZ + 1);
> > - else
> > - if (++clp->cl_id_uniquifier == 0)
> > - break;
> > - }
> > + setclientid.sc_name_len = scnprintf(setclientid.sc_name,
> > + sizeof(setclientid.sc_name), "%s/%s %s %s %u",
> > + clp->cl_ipaddr,
> > + rpc_peeraddr2str(clp->cl_rpcclient,
> > + RPC_DISPLAY_ADDR),
> > + rpc_peeraddr2str(clp->cl_rpcclient,
> > + RPC_DISPLAY_PROTO),
> > + clp->cl_rpcclient->cl_auth->au_ops->au_name,
> > + clp->cl_id_uniquifier);
> > + setclientid.sc_netid_len = scnprintf(setclientid.sc_netid,
> > + sizeof(setclientid.sc_netid),
> > + rpc_peeraddr2str(clp->cl_rpcclient,
> > + RPC_DISPLAY_NETID));
> > + setclientid.sc_uaddr_len = scnprintf(setclientid.sc_uaddr,
> > + sizeof(setclientid.sc_uaddr), "%s.%u.%u",
> > + clp->cl_ipaddr, port >> 8, port & 255);
> > +
> > + status = rpc_call_sync(clp->cl_rpcclient, &msg, 0);
> > + if (status == -NFS4ERR_CLID_INUSE)
> > + ssleep(clp->cl_lease_time / HZ + 1);
> > return status;
> > }
> >
> > -static int _nfs4_proc_setclientid_confirm(struct nfs_client *clp,
> > +int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
> > struct nfs4_setclientid_res *arg,
> > struct rpc_cred *cred)
> > {
> > @@ -3790,26 +3780,6 @@ static int _nfs4_proc_setclientid_confirm(struct nfs_client *clp,
> > return status;
> > }
> >
> > -int nfs4_proc_setclientid_confirm(struct nfs_client *clp,
> > - struct nfs4_setclientid_res *arg,
> > - struct rpc_cred *cred)
> > -{
> > - long timeout = 0;
> > - int err;
> > - do {
> > - err = _nfs4_proc_setclientid_confirm(clp, arg, cred);
> > - switch (err) {
> > - case 0:
> > - return err;
> > - case -NFS4ERR_RESOURCE:
> > - /* The IBM lawyers misread another document! */
> > - case -NFS4ERR_DELAY:
> > - err = nfs4_delay(clp->cl_rpcclient, &timeout);
> > - }
> > - } while (err == 0);
> > - return err;
> > -}
> > -
> > struct nfs4_delegreturndata {
> > struct nfs4_delegreturnargs args;
> > struct nfs4_delegreturnres res;
> > @@ -5222,20 +5192,10 @@ int nfs4_proc_create_session(struct nfs_client *clp)
> > int status;
> > unsigned *ptr;
> > struct nfs4_session *session = clp->cl_session;
> > - long timeout = 0;
> > - int err;
> >
> > dprintk("--> %s clp=%p session=%p\n", __func__, clp, session);
> >
> > - do {
> > - status = _nfs4_proc_create_session(clp);
> > - if (status == -NFS4ERR_DELAY) {
> > - err = nfs4_delay(clp->cl_rpcclient, &timeout);
> > - if (err)
> > - status = err;
> > - }
> > - } while (status == -NFS4ERR_DELAY);
> > -
> > + status = _nfs4_proc_create_session(clp);
> > if (status)
> > goto out;
> >
> > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> > index 4dfb34b..9a7f9df 100644
> > --- a/fs/nfs/nfs4state.c
> > +++ b/fs/nfs/nfs4state.c
> > @@ -64,10 +64,15 @@ static LIST_HEAD(nfs4_clientid_list);
> >
> > int nfs4_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
> > {
> > - struct nfs4_setclientid_res clid;
> > + struct nfs4_setclientid_res clid = {
> > + .clientid = clp->cl_clientid,
> > + .confirm = clp->cl_confirm,
> > + };
> > unsigned short port;
> > int status;
> >
> > + if (test_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state))
> > + goto do_confirm;
> > port = nfs_callback_tcpport;
> > if (clp->cl_addr.ss_family == AF_INET6)
> > port = nfs_callback_tcpport6;
> > @@ -75,10 +80,14 @@ int nfs4_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
> > status = nfs4_proc_setclientid(clp, NFS4_CALLBACK, port, cred, &clid);
> > if (status != 0)
> > goto out;
> > + clp->cl_clientid = clid.clientid;
> > + clp->cl_confirm = clid.confirm;
> > + set_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> > +do_confirm:
> > status = nfs4_proc_setclientid_confirm(clp, &clid, cred);
> > if (status != 0)
> > goto out;
> > - clp->cl_clientid = clid.clientid;
> > + clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> > nfs4_schedule_state_renewal(clp);
> > out:
> > return status;
> > @@ -230,13 +239,18 @@ int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred)
> > {
> > int status;
> >
> > + if (test_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state))
> > + goto do_confirm;
> > nfs4_begin_drain_session(clp);
> > status = nfs4_proc_exchange_id(clp, cred);
> > if (status != 0)
> > goto out;
> > + set_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> > +do_confirm:
> > status = nfs4_proc_create_session(clp);
> > if (status != 0)
> > goto out;
> > + clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> > nfs41_setup_state_renewal(clp);
> > nfs_mark_client_ready(clp, NFS_CS_READY);
> > out:
> > @@ -1584,20 +1598,23 @@ static int nfs4_recall_slot(struct nfs_client *clp) { return 0; }
> > */
> > static void nfs4_set_lease_expired(struct nfs_client *clp, int status)
> > {
> > - if (nfs4_has_session(clp)) {
> > - switch (status) {
> > - case -NFS4ERR_DELAY:
> > - case -NFS4ERR_CLID_INUSE:
> > - case -EAGAIN:
> > - break;
> > + switch (status) {
> > + case -NFS4ERR_CLID_INUSE:
> > + clp->cl_id_uniquifier++;
> > + case -NFS4ERR_STALE_CLIENTID:
> > + clear_bit(NFS4CLNT_LEASE_CONFIRM, &clp->cl_state);
> > + break;
> > + case -NFS4ERR_DELAY:
> > + case -EAGAIN:
> > + ssleep(1);
> > + break;
> >
> > - case -EKEYEXPIRED:
> > - nfs4_warn_keyexpired(clp->cl_hostname);
> > - case -NFS4ERR_NOT_SAME: /* FixMe: implement recovery
> > - * in nfs4_exchange_id */
> > - default:
> > - return;
> > - }
> > + case -EKEYEXPIRED:
> > + nfs4_warn_keyexpired(clp->cl_hostname);
> > + case -NFS4ERR_NOT_SAME: /* FixMe: implement recovery
> > + * in nfs4_exchange_id */
> > + default:
> > + return;
> > }
> > set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
> > }
> > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> > index 216cea5..87694ca 100644
> > --- a/include/linux/nfs_fs_sb.h
> > +++ b/include/linux/nfs_fs_sb.h
> > @@ -47,6 +47,7 @@ struct nfs_client {
> >
> > #ifdef CONFIG_NFS_V4
> > u64 cl_clientid; /* constant */
> > + nfs4_verifier cl_confirm; /* Clientid verifier */
> > unsigned long cl_state;
> >
> > spinlock_t cl_lock;
> > --
> > 1.7.4.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@netapp.com
www.netapp.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session
2011-04-23 19:27 ` Trond Myklebust
@ 2011-04-25 13:51 ` Chuck Lever
0 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2011-04-25 13:51 UTC (permalink / raw)
To: Trond Myklebust; +Cc: linux-nfs
On Apr 23, 2011, at 3:27 PM, Trond Myklebust wrote:
> On Thu, 2011-04-21 at 11:39 -0400, Chuck Lever wrote:
>> Hi-
>>
>> This will require some significant human intervention to merge with my code that caches the long-form client ID.
>
> How so? It really isn't doing much more than removing those 'for(;;)'
> loops.
Essentially the merge of these with my migration patches will not be automatic, because I touch this same code to do long-form client ID caching. I'm simply pointing out where some effort will be needed as we assemble for-2.6.40.
I'll post my migration patches for review later today, then we can discuss what is needed.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-04-25 13:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-20 23:50 [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session Trond Myklebust
2011-04-20 23:50 ` [PATCH 2/3] SUNRPC: Allow RPC calls to return ETIMEDOUT instead of EIO Trond Myklebust
2011-04-20 23:50 ` [PATCH 3/3] NFSv4: Ensure that clientid and session establishment can time out Trond Myklebust
2011-04-21 15:39 ` [PATCH 1/3] NFSv4.1: Don't loop forever in nfs4_proc_create_session Chuck Lever
2011-04-23 19:27 ` Trond Myklebust
2011-04-25 13:51 ` Chuck Lever
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.