On May 21, 2012, at 12:08 PM, J. Bruce Fields wrote:
> On Fri, May 18, 2012 at 06:06:33PM -0400, Chuck Lever wrote:
>> A SETCLIENTID boot verifier is nothing more than the client's boot
>> timestamp. An NFSv4 server is obligated to wipe all NFSv4 state for
>> an NFS client when the client presents an updated SETCLIENTID boot
>> verifier. This is how servers detect client reboots.
>>
>> nfs4_reset_all_state() refreshes our client's boot verifier to trigger
>> a server to wipe this client's state. This function is invoked when
>> an NFSv4.1 server reports that it has revoked some or all of a
>> client's NFSv4 state.
>>
>> Soon we want to get rid of the per-nfs_client cl_boot_time field,
>
> OK, I guess you don't need it if you never intend to send a callback
> update or anything?
"Get rid of" is perhaps too strong. I'm moving it to nfs_net.
>> however. Without cl_boot_time, our NFS client will need to find a
>> different way to force the server to purge the client's NFSv4 state.
>>
>> Because these verifiers are opaque (ie, the server doesn't know or
>> care that they happen to be timestamps), we can do force the server
>> to wipe NFSv4 state by using the same trick we use now, but then
>> immediately afterwards establish a fresh client ID using the old boot
>> verifier again.
>>
>> Hopefully there are no extra paranoid server implementations that keep
>> track of the client's boot verifiers and prevent clients from reusing
>> a previous one.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>>
>> fs/nfs/nfs4_fs.h | 1 +
>> fs/nfs/nfs4proc.c | 9 +++++++--
>> fs/nfs/nfs4state.c | 13 +++++++++++--
>> 3 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>> index 03d3ff0..f164c73 100644
>> --- a/fs/nfs/nfs4_fs.h
>> +++ b/fs/nfs/nfs4_fs.h
>> @@ -24,6 +24,7 @@ enum nfs4_client_state {
>> NFS4CLNT_RECALL_SLOT,
>> NFS4CLNT_LEASE_CONFIRM,
>> NFS4CLNT_SERVER_SCOPE_MISMATCH,
>> + NFS4CLNT_PURGE_STATE,
>> };
>>
>> enum nfs4_session_state {
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 73cfd9e..c56d547 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -3937,8 +3937,13 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
>> {
>> __be32 verf[2];
>>
>> - verf[0] = (__be32)clp->cl_boot_time.tv_sec;
>> - verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
>> + if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
>> + verf[0] = (__be32)CURRENT_TIME.tv_sec;
>> + verf[1] = (__be32)CURRENT_TIME.tv_nsec;
>
> I suppose it's pretty unlikely this could happen within a jiffy of
> setting cl_boot_time?
Boot time has nanosecond resolution. I'm pretty sure we are safe here.
> Would it be simpler to use some special value (all-zeros?) instead?
We'd have to pick a value that was guaranteed never to be a valid time stamp.
>
> --b.
>
>> + } else {
>> + verf[0] = (__be32)clp->cl_boot_time.tv_sec;
>> + verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
>> + }
>> memcpy(bootverf->data, verf, sizeof(bootverf->data));
>> }
>>
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index f8c06de..32cce4a 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -1615,7 +1615,7 @@ void nfs41_handle_recall_slot(struct nfs_client *clp)
>> static void nfs4_reset_all_state(struct nfs_client *clp)
>> {
>> if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
>> - clp->cl_boot_time = CURRENT_TIME;
>> + set_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
>> nfs4_state_start_reclaim_nograce(clp);
>> nfs4_schedule_state_manager(clp);
>> }
>> @@ -1631,7 +1631,6 @@ static void nfs41_handle_server_reboot(struct nfs_client *clp)
>>
>> static void nfs41_handle_state_revoked(struct nfs_client *clp)
>> {
>> - /* Temporary */
>> nfs4_reset_all_state(clp);
>> }
>>
>> @@ -1652,6 +1651,10 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
>> {
>> if (!flags)
>> return;
>> +
>> + dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n",
>> + __func__, clp->cl_hostname, clp->cl_clientid, flags);
>> +
>> if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED)
>> nfs41_handle_server_reboot(clp);
>> if (flags & (SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED |
>> @@ -1762,6 +1765,12 @@ static void nfs4_state_manager(struct nfs_client *clp)
>>
>> /* Ensure exclusive access to NFSv4 state */
>> do {
>> + if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
>> + nfs4_reclaim_lease(clp);
>> + clear_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
>> + set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
>> + }
>> +
>> if (test_and_clear_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state)) {
>> /* We're going to have to re-establish a clientid */
>> status = nfs4_reclaim_lease(clp);
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
