On Mon, May 21, 2012 at 12:08:44PM -0400, bfields 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?
(Never mind, I see in the next patch you're still remembering the
verifier, just giving it a broader scope?)
--b.
>
> > 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?
>
> Would it be simpler to use some special value (all-zeros?) instead?
>
> --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
--
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
