From: Trond Myklebust Subject: Re: [PATCH 1/2] nfs41: nfs41_setup_state_renewal Date: Sun, 06 Dec 2009 13:02:35 -0500 Message-ID: <1260122555.11862.10.camel@localhost> References: <> <1260098184-19209-1-git-send-email-Ricardo.Labiaga@netapp.com> <1260098184-19209-2-git-send-email-Ricardo.Labiaga@netapp.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: linux-nfs@vger.kernel.org To: Ricardo Labiaga Return-path: Received: from mx2.netapp.com ([216.240.18.37]:46402 "EHLO mx2.netapp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934107AbZLFSCa convert rfc822-to-8bit (ORCPT ); Sun, 6 Dec 2009 13:02:30 -0500 Received: from sacrsexc2-prd.hq.netapp.com (sacrsexc2-prd.hq.netapp.com [10.99.115.28]) by smtp2.corp.netapp.com (8.13.1/8.13.1/NTAP-1.6) with ESMTP id nB6I2aRH013067 for ; Sun, 6 Dec 2009 10:02:37 -0800 (PST) In-Reply-To: <1260098184-19209-2-git-send-email-Ricardo.Labiaga@netapp.com> Sender: linux-nfs-owner@vger.kernel.org List-ID: On Sun, 2009-12-06 at 03:16 -0800, Ricardo Labiaga wrote: > Move call to get the lease time and the setup of the state > renewal out of nfs4_create_session so that it can be called > after clearing the DRAINING flag. We use the getattr RPC > to obtain the lease time, which requires a sequence slot. > > Signed-off-by: Ricardo Labiaga > status = nfs4_proc_exchange_id(clp, cred); > if (status == 0) > - /* create session schedules state renewal upon success */ > status = nfs4_proc_create_session(clp); > + if (status == 0 && test_and_clear_bit( > + NFS4CLNT_SESSION_DRAINING, &clp->cl_state)) > + rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq); This was clearly supposed to be part of PATCH 2/2... > + if (status == 0) > + nfs41_setup_state_renewal(clp); Hrm... Lots of tests of 'status == 0' without status actually changing. I've fixed this up (see below). > if (status == 0) > nfs_mark_client_ready(clp, NFS_CS_READY); > return status; ------------------------------------------------------------------------------------------------------- nfs41: nfs41_setup_state_renewal From: Ricardo Labiaga Move call to get the lease time and the setup of the state renewal out of nfs4_create_session so that it can be called after clearing the DRAINING flag. We use the getattr RPC to obtain the lease time, which requires a sequence slot. Signed-off-by: Ricardo Labiaga Signed-off-by: Trond Myklebust --- fs/nfs/nfs4_fs.h | 2 ++ fs/nfs/nfs4proc.c | 13 ------------- fs/nfs/nfs4state.c | 34 +++++++++++++++++++++++++++++----- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h index 50dd550..7e57b04 100644 --- a/fs/nfs/nfs4_fs.h +++ b/fs/nfs/nfs4_fs.h @@ -225,6 +225,8 @@ extern struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp); extern int nfs4_proc_create_session(struct nfs_client *); extern int nfs4_proc_destroy_session(struct nfs4_session *); extern int nfs4_init_session(struct nfs_server *server); +extern int nfs4_proc_get_lease_time(struct nfs_client *clp, + struct nfs_fsinfo *fsinfo); #else /* CONFIG_NFS_v4_1 */ static inline int nfs4_setup_sequence(struct nfs_client *clp, struct nfs4_sequence_args *args, struct nfs4_sequence_res *res, diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index b4ef570..4be0369 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -4745,7 +4745,6 @@ int nfs4_proc_create_session(struct nfs_client *clp) { int status; unsigned *ptr; - struct nfs_fsinfo fsinfo; struct nfs4_session *session = clp->cl_session; dprintk("--> %s clp=%p session=%p\n", __func__, clp, session); @@ -4767,18 +4766,6 @@ int nfs4_proc_create_session(struct nfs_client *clp) ptr = (unsigned *)&session->sess_id.data[0]; dprintk("%s client>seqid %d sessionid %u:%u:%u:%u\n", __func__, clp->cl_seqid, ptr[0], ptr[1], ptr[2], ptr[3]); - - /* Get the lease time */ - status = nfs4_proc_get_lease_time(clp, &fsinfo); - if (status == 0) { - /* Update lease time and schedule renewal */ - spin_lock(&clp->cl_lock); - clp->cl_lease_time = fsinfo.lease_time * HZ; - clp->cl_last_renewal = jiffies; - spin_unlock(&clp->cl_lock); - - nfs4_schedule_state_renewal(clp); - } out: dprintk("<-- %s\n", __func__); return status; diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index ef9622e..9cfe686 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -116,16 +116,38 @@ struct rpc_cred *nfs4_get_renew_cred_locked(struct nfs_client *clp) #if defined(CONFIG_NFS_V4_1) +static int nfs41_setup_state_renewal(struct nfs_client *clp) +{ + int status; + struct nfs_fsinfo fsinfo; + + status = nfs4_proc_get_lease_time(clp, &fsinfo); + if (status == 0) { + /* Update lease time and schedule renewal */ + spin_lock(&clp->cl_lock); + clp->cl_lease_time = fsinfo.lease_time * HZ; + clp->cl_last_renewal = jiffies; + spin_unlock(&clp->cl_lock); + + nfs4_schedule_state_renewal(clp); + } + + return status; +} + int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred) { int status; status = nfs4_proc_exchange_id(clp, cred); - if (status == 0) - /* create session schedules state renewal upon success */ - status = nfs4_proc_create_session(clp); - if (status == 0) - nfs_mark_client_ready(clp, NFS_CS_READY); + if (status != 0) + goto out; + status = nfs4_proc_create_session(clp); + if (status != 0) + goto out; + nfs41_setup_state_renewal(clp); + nfs_mark_client_ready(clp, NFS_CS_READY); +out: return status; } @@ -1248,6 +1270,8 @@ out: /* Wake up the next rpc task even on error */ clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state); rpc_wake_up(&clp->cl_session->fc_slot_table.slot_tbl_waitq); + if (status == 0) + nfs41_setup_state_renewal(clp); return status; }