From: Chuck Lever <chuck.lever@oracle.com>
To: Tom Haynes <tdh-8AdZ+HgO7noAvxtiuMwx3w@public.gmane.org>
Cc: Trond Myklebust <trond.myklebust@netapp.com>,
Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 16/16] NFS: Implement support for NFS4ERR_LEASE_MOVED
Date: Wed, 11 May 2011 10:04:20 -0400 [thread overview]
Message-ID: <044331B2-5128-4F4B-8B6C-949AF76162FE@oracle.com> (raw)
In-Reply-To: <4DC9D636.3050307-8AdZ+HgO7noAvxtiuMwx3w@public.gmane.org>
On May 10, 2011, at 8:20 PM, Tom Haynes wrote:
> On 5/9/11 5:48 PM, Chuck Lever wrote:
>> On May 9, 2011, at 3:38 PM, Chuck Lever wrote:
>>
>>> To recover from NFS4ERR_LEASE_MOVED, walk the cl_superblocks list and
>>> invoke nfs4_try_migration() on each server's root file handle.
>>> nfs4_try_migration() should automatically determine whether that file
>>> system has migrated, and then perform recovery for it.
>>>
>>> The per-filesystem migration probe also informs minor version zero
>>> servers that this client should no longer receive NFS4ERR_LEASE_MOVED.
>> I see one thing that may be missing here.
>>
>> RFC 3530, section 8.14.3, lists OPEN, CLOSE, READ, WRITE, RENEW, LOCK, LOCKU, and LOCKT as the only procedures that return NFS4ERR_LEASE_MOVED. A code audit suggests that handling NFS4ERR_LEASE_MOVED in the two generic error handlers in nfs4proc.c is sufficient for all of these but RENEW and OPEN.
>
> 3503bis, section 13.4:
>
> | NFS4ERR_LEASE_MOVED | CLOSE, DELEGPURGE, DELEGRETURN, LOCK, |
> | | LOCKT, LOCKU, OPEN_CONFIRM, |
> | | OPEN_DOWNGRADE, READ, |
> | | RELEASE_LOCKOWNER, RENEW, SETATTR, |
> | | WRITE |
>
> And the equivalent text as to what is in 3530 is section 9.14.3:
>
> To accomplish this, all
> operations which implicitly renew leases for a client (such as OPEN,
> CLOSE, READ, WRITE, RENEW, LOCK, and others), will return the error
> NFS4ERR_LEASE_MOVED if responsibility for any of the leases to be
> renewed has been transferred to a new server.
Thanks, will take a look (and refresh my copy of 3530... oops).
>
>
>
>
>> RENEW is part of the lease_moved recovery logic, so I've left NFS4ERR_LEASE_MOVED handling there pretty sparse. The caller wants to deal with it.
>>
>> OPEN, as far as I can tell, won't deal with it at all, at least not directly. Should we look for it in nfs4_open_done() and invoke nfs4_schedule_lease_moved_recovery()?
>>
>>> Signed-off-by: Chuck Lever<chuck.lever@oracle.com>
>>> ---
>>>
>>> fs/nfs/client.c | 1 +
>>> fs/nfs/nfs4_fs.h | 2 +
>>> fs/nfs/nfs4proc.c | 11 ++++++++
>>> fs/nfs/nfs4state.c | 63 ++++++++++++++++++++++++++++++++++++++++++++-
>>> include/linux/nfs_fs_sb.h | 2 +
>>> 5 files changed, 77 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>> index 2f5e29f..b89af4d 100644
>>> --- a/fs/nfs/client.c
>>> +++ b/fs/nfs/client.c
>>> @@ -188,6 +188,7 @@ static struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_
>>> if (cl_init->long_clientid != NULL)
>>> clp->cl_cached_clientid = kstrdup(cl_init->long_clientid,
>>> GFP_KERNEL);
>>> + clp->cl_mig_counter = 1;
>>> #endif
>>> cred = rpc_lookup_machine_cred();
>>> if (!IS_ERR(cred))
>>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>>> index c3e8641..2ad6c9b 100644
>>> --- a/fs/nfs/nfs4_fs.h
>>> +++ b/fs/nfs/nfs4_fs.h
>>> @@ -51,6 +51,7 @@ enum nfs4_client_state {
>>> NFS4CLNT_UPDATE_CALLBACK,
>>> NFS4CLNT_CLONED_CLIENT,
>>> NFS4CLNT_MOVED,
>>> + NFS4CLNT_LEASE_MOVED,
>>> };
>>>
>>> enum nfs4_session_state {
>>> @@ -351,6 +352,7 @@ extern void nfs4_close_sync(struct path *, struct nfs4_state *, fmode_t);
>>> extern void nfs4_state_set_mode_locked(struct nfs4_state *, fmode_t);
>>> extern void nfs4_schedule_lease_recovery(struct nfs_client *);
>>> extern void nfs4_schedule_migration_recovery(struct nfs_server *);
>>> +extern void nfs4_schedule_lease_moved_recovery(struct nfs_client *);
>>> extern void nfs4_schedule_state_manager(struct nfs_client *);
>>> extern void nfs4_schedule_stateid_recovery(const struct nfs_server *, struct nfs4_state *);
>>> extern void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags);
>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>> index a545d46..f4e07ba 100644
>>> --- a/fs/nfs/nfs4proc.c
>>> +++ b/fs/nfs/nfs4proc.c
>>> @@ -297,6 +297,9 @@ static int nfs4_handle_exception(struct nfs_server *server, int errorcode,
>>> case -NFS4ERR_MOVED:
>>> nfs4_schedule_migration_recovery(server);
>>> goto wait_on_recovery;
>>> + case -NFS4ERR_LEASE_MOVED:
>>> + nfs4_schedule_lease_moved_recovery(clp);
>>> + goto wait_on_recovery;
>>> case -NFS4ERR_FILE_OPEN:
>>> if (exception->timeout> HZ) {
>>> /* We have retried a decent amount, time to
>>> @@ -3721,6 +3724,14 @@ static int nfs4_async_handle_error(struct rpc_task *task,
>>> rpc_wake_up_queued_task(&clp->cl_rpcwaitq,
>>> task);
>>> goto restart_call;
>>> + case -NFS4ERR_LEASE_MOVED:
>>> + rpc_sleep_on(&clp->cl_rpcwaitq, task, NULL);
>>> + nfs4_schedule_lease_moved_recovery(clp);
>>> + if (test_bit(NFS4CLNT_MANAGER_RUNNING,
>>> + &clp->cl_state) == 0)
>>> + rpc_wake_up_queued_task(&clp->cl_rpcwaitq,
>>> + task);
>>> + goto restart_call;
>>> case -NFS4ERR_DELAY:
>>> nfs_inc_server_stats(server, NFSIOS_DELAY);
>>> case -NFS4ERR_GRACE:
>>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>>> index c7b414a..8bef9d8 100644
>>> --- a/fs/nfs/nfs4state.c
>>> +++ b/fs/nfs/nfs4state.c
>>> @@ -1071,7 +1071,32 @@ void nfs4_schedule_migration_recovery(struct nfs_server *server)
>>> dprintk("<-- %s\n", __func__);
>>> }
>>>
>>> -static int nfs4_state_mark_reclaim_reboot(struct nfs_client *clp, struct nfs4_state *state)
>>> +/**
>>> + * nfs4_schedule_lease_moved_recovery - start lease moved recovery
>>> + *
>>> + * @clp: nfs_client of server that may have migrated file systems
>>> + *
>>> + */
>>> +void nfs4_schedule_lease_moved_recovery(struct nfs_client *clp)
>>> +{
>>> + dprintk("--> %s: \"%s\" (client ID %llx)\n",
>>> + __func__, clp->cl_hostname, clp->cl_clientid);
>>> +
>>> + if (test_and_set_bit(NFS4CLNT_LEASE_MOVED,&clp->cl_state) == 0)
>>> + nfs4_schedule_state_manager(clp);
>>> +
>>> + dprintk("<-- %s\n", __func__);
>>> +}
>>> +
>>> +/**
>>> + * nfs4_state_mark_reclaim_reboot - Mark nfs_client for reboot recovery
>>> + * @clp: nfs_client of server that may have rebooted
>>> + * @state: state flags to test
>>> + *
>>> + * Returns 1 if reboot recovery is needed.
>>> + */
>>> +int nfs4_state_mark_reclaim_reboot(struct nfs_client *clp,
>>> + struct nfs4_state *state)
>>> {
>>>
>>> set_bit(NFS_STATE_RECLAIM_REBOOT,&state->flags);
>>> @@ -1384,7 +1409,6 @@ static int nfs4_recovery_handle_error(struct nfs_client *clp, int error)
>>> nfs4_state_end_reclaim_reboot(clp);
>>> return 0;
>>> case -NFS4ERR_STALE_CLIENTID:
>>> - case -NFS4ERR_LEASE_MOVED:
>>> set_bit(NFS4CLNT_LEASE_EXPIRED,&clp->cl_state);
>>> nfs4_state_clear_reclaim_reboot(clp);
>>> nfs4_state_start_reclaim_reboot(clp);
>>> @@ -1597,6 +1621,37 @@ out_err:
>>> kfree(locations);
>>> }
>>>
>>> +static void nfs4_handle_lease_moved(struct nfs_client *clp)
>>> +{
>>> + struct nfs_server *server;
>>> +
>>> + dprintk("--> %s: \"%s\" (client ID %llx)\n",
>>> + __func__, clp->cl_hostname, clp->cl_clientid);
>>> +
>>> + /*
>>> + * rcu_read_lock() must be dropped before trying each individual
>>> + * migration. cl_mig_counter is used to skip servers that have
>>> + * already been visited for this lease_moved event when the list
>>> + * walk is restarted.
>>> + */
>>> + clp->cl_mig_counter++;
>>> +
>>> +restart:
>>> + rcu_read_lock();
>>> + list_for_each_entry_rcu(server,&clp->cl_superblocks, client_link)
>>> + if (server->mig_counter != clp->cl_mig_counter) {
>>> + server->mig_counter = clp->cl_mig_counter;
>>> + rcu_read_unlock();
>>> + nfs4_try_migration(server);
>>> + /* Ask the server if there's more work to do */
>>> + if (nfs4_check_lease(clp) == NFS4ERR_LEASE_MOVED)
>>> + goto restart;
>>> + break;
>>> + }
>>> + rcu_read_unlock();
>>> + dprintk("<-- %s\n", __func__);
>>> +}
>>> +
>>> #ifdef CONFIG_NFS_V4_1
>>> void nfs4_schedule_session_recovery(struct nfs4_session *session)
>>> {
>>> @@ -1814,6 +1869,10 @@ static void nfs4_state_manager(struct nfs_client *clp)
>>> nfs4_try_migration(clp->cl_moved_server);
>>> continue;
>>> }
>>> + if (test_and_clear_bit(NFS4CLNT_LEASE_MOVED,&clp->cl_state)) {
>>> + nfs4_handle_lease_moved(clp);
>>> + continue;
>>> + }
>>>
>>> /* First recover reboot state... */
>>> if (test_bit(NFS4CLNT_RECLAIM_REBOOT,&clp->cl_state)) {
>>> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
>>> index 091abf0..58050db 100644
>>> --- a/include/linux/nfs_fs_sb.h
>>> +++ b/include/linux/nfs_fs_sb.h
>>> @@ -60,6 +60,7 @@ struct nfs_client {
>>>
>>> /* accessed only when NFS4CLNT_MOVED bit is set */
>>> struct nfs_server * cl_moved_server;
>>> + unsigned long cl_mig_counter;
>>>
>>> /* used for the setclientid verifier */
>>> struct timespec cl_boot_time;
>>> @@ -156,6 +157,7 @@ struct nfs_server {
>>> struct list_head delegations;
>>> void (*destroy)(struct nfs_server *);
>>> struct nfs_fh *rootfh;
>>> + unsigned long mig_counter;
>>>
>>> atomic_t active; /* Keep trace of any activity to this server */
>>>
>>>
>>> --
>>> 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
next prev parent reply other threads:[~2011-05-11 18:35 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-09 19:36 [PATCH 00/16] Client-side migration support for 2.6.40 [take 3] Chuck Lever
2011-05-09 19:36 ` [PATCH 01/16] SUNRPC: Allow temporary blocking of an rpc client Chuck Lever
2011-05-09 19:36 ` [PATCH 02/16] SUNRPC: Use RCU to dereference the rpc_clnt.cl_xprt field Chuck Lever
2011-05-09 19:36 ` [PATCH 03/16] SUNRPC: Move clnt->cl_server into struct rpc_xprt Chuck Lever
2011-05-09 19:36 ` [PATCH 04/16] SUNRPC: Add a helper to switch the transport of the rpc_client Chuck Lever
2011-05-09 19:37 ` [PATCH 05/16] SUNRPC: Add API to acquire source address Chuck Lever
2011-05-09 19:37 ` [PATCH 06/16] NFS: Add a client-side function to display file handles Chuck Lever
2011-05-09 19:37 ` [PATCH 07/16] NFS: Save root file handle in nfs_server Chuck Lever
2011-05-09 19:37 ` [PATCH 08/16] NFS: Introduce NFS_ATTR_FATTR_V4_LOCATIONS Chuck Lever
2011-05-09 19:37 ` [PATCH 09/16] NFS: Introduce nfs4_proc_get_mig_status() Chuck Lever
2011-05-09 19:37 ` [PATCH 10/16] NFS: Add infrastructure for updating callback data Chuck Lever
2011-05-09 19:38 ` [PATCH 11/16] NFS: Add an API for cloning an nfs_client Chuck Lever
2011-05-12 17:30 ` Chuck Lever
2011-05-12 19:30 ` Trond Myklebust
2011-05-09 19:38 ` [PATCH 12/16] NFS: Add functions to swap transports during migration recovery Chuck Lever
2011-05-09 19:38 ` [PATCH 13/16] NFS: Add basic migration support to state manager thread Chuck Lever
2011-05-09 19:38 ` [PATCH 14/16] NFS: Remove "const" from "struct nfs_server *" fields Chuck Lever
2011-05-09 19:38 ` [PATCH 15/16] NFS: Add migration recovery callouts in nfs4proc.c Chuck Lever
2011-05-09 19:38 ` [PATCH 16/16] NFS: Implement support for NFS4ERR_LEASE_MOVED Chuck Lever
2011-05-09 22:48 ` Chuck Lever
2011-05-11 0:20 ` Tom Haynes
[not found] ` <4DC9D636.3050307-8AdZ+HgO7noAvxtiuMwx3w@public.gmane.org>
2011-05-11 14:04 ` Chuck Lever [this message]
2011-05-12 15:37 ` Chuck Lever
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=044331B2-5128-4F4B-8B6C-949AF76162FE@oracle.com \
--to=chuck.lever@oracle.com \
--cc=linux-nfs@vger.kernel.org \
--cc=tdh-8AdZ+HgO7noAvxtiuMwx3w@public.gmane.org \
--cc=trond.myklebust@netapp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).