Re: [PATCH 0/3] Fix session draining and back channel processing

Re: [PATCH 0/3] Fix session draining and back channel processing

[Andy Adamson]

We need to write tests for the back channel draining. This code passes Cthon, the pynfs session reset tests and cb_layoutrecall tests.

-->Andy

On Nov 11, 2010, at 9:10 AM, andros@netapp.com wrote:

> 
> Currently the state manager only waits for the fore channel to drain.
> The back channel processing must also be considered.
> Have the state manager thread also wait for the any current back channel
> processing to complete (drain).
> 
> Guarantee that the nfs_client exists during callback processing.
> 
> 0001-pnfs-submit-set-back-channel-highest-slot-used.patch
> 0002-pnfs-submit-add-back-channel-draining.patch
> 0003-pnfs-submit-callbacks-cannot-use-an-nfs_client-that-.patch
> 
> -->Andy
> 
> --
> 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

Re: [PATCH 3/3] pnfs-submit: callbacks cannot use an nfs_client that is being freed

[Benny Halevy]

On 2010-11-11 16:10, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Guarantee that the nfs_client exists when referenced to by callback processing
> by not procssing callbacks on an nfs_client in the process of being freed.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/client.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index dbf43e7..86657ee 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -392,7 +392,9 @@ struct nfs_client *nfs_find_client(const struct sockaddr *addr, u32 nfsversion)
>  		if (!nfs_sockaddr_match_ipaddr(addr, clap))
>  			continue;
>  
> -		atomic_inc(&clp->cl_count);
> +		/* Don't return an nfs_client that is being freed */
> +		if (!atomic_inc_not_zero(&clp->cl_count))
> +			continue;
>  		spin_unlock(&nfs_client_lock);
>  		return clp;
>  	}
> @@ -425,7 +427,9 @@ struct nfs_client *nfs_find_client_next(struct nfs_client *clp)
>  		if (!nfs_sockaddr_match_ipaddr(sap, clap))
>  			continue;
>  
> -		atomic_inc(&clp->cl_count);
> +		/* Don't return an nfs_client that is being freed */
> +		if (!atomic_inc_not_zero(&clp->cl_count))
> +			continue;
>  		spin_unlock(&nfs_client_lock);
>  		return clp;
>  	}

Hmm, nfs_put_client deletes the client when cl_count reaches zero
so how can cl_count be zero while clp is listed?

Benny

In put_nfs_client 

[PATCH 0/3] Fix session draining and back channel processing

[andros]

Currently the state manager only waits for the fore channel to drain.
The back channel processing must also be considered.
Have the state manager thread also wait for the any current back channel
processing to complete (drain).

Guarantee that the nfs_client exists during callback processing.

0001-pnfs-submit-set-back-channel-highest-slot-used.patch
0002-pnfs-submit-add-back-channel-draining.patch
0003-pnfs-submit-callbacks-cannot-use-an-nfs_client-that-.patch

-->Andy

[PATCH 1/3] pnfs-submit: set back channel highest slot used

[andros]

From: Andy Adamson <andros@netapp.com>

Prepare for the state manager thread to drain back channel.
Use the back channel highest_slot_used to indicate to the state manager
that a callback is being processed by the callback thread.

Fix the BADSLOT check - the first and only slotid is 0.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/callback_proc.c |   13 ++++++++++++-
 fs/nfs/callback_xdr.c  |   16 +++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 6e21add..6ba528a 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -469,6 +469,9 @@ int nfs41_validate_delegation_stateid(struct nfs_delegation *delegation, const n
  * We have a single slot backchannel at this time, so we don't bother
  * checking the used_slots bit array on the table.  The lower layer guarantees
  * a single outstanding callback request at a time.
+ *
+ * We set the highest slot used to let the state manager know when a
+ * callback is being processed and to wait until completion.
  */
 static __be32
 validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
@@ -478,10 +481,18 @@ validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
 	dprintk("%s enter. slotid %d seqid %d\n",
 		__func__, args->csa_slotid, args->csa_sequenceid);
 
-	if (args->csa_slotid > NFS41_BC_MAX_CALLBACKS)
+	/* Single slot, so slotid must be 0 */
+	if (args->csa_slotid > NFS41_BC_MAX_CALLBACKS - 1 ||
+	    args->csa_slotid < 0)
 		return htonl(NFS4ERR_BADSLOT);
 
+	spin_lock(&tbl->slot_tbl_lock);
 	slot = tbl->slots + args->csa_slotid;
+
+	/* Only a single slot */
+	tbl->highest_used_slotid = args->csa_slotid;
+	spin_unlock(&tbl->slot_tbl_lock);
+
 	dprintk("%s slot table seqid: %d\n", __func__, slot->seq_nr);
 
 	/* Normal */
diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index 695d15a..e05d1b3 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -773,8 +773,22 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
 
 	*hdr_res.status = status;
 	*hdr_res.nops = htonl(nops);
-	if (cps.session) /* matched by cb_sequence find_client_with_session */
+	if (cps.session) {
+		struct nfs4_slot_table *tbl = &cps.session->bc_slot_table;
+
+		spin_lock(&tbl->slot_tbl_lock);
+		/*
+		 * Let the state manager know callback processing done.
+		 * A single slot, so highest used slotid is either 0 or -1
+		 */
+		tbl->highest_used_slotid--;
+		spin_unlock(&tbl->slot_tbl_lock);
+		dprintk("%s highest_used_slotid %d\n", __func__,
+			tbl->highest_used_slotid);
+
+		/* matched by cb_sequence find_client_with_session */
 		nfs_put_client(cps.session->clp);
+	}
 	dprintk("%s: done, status = %u\n", __func__, ntohl(status));
 	return rpc_success;
 }
-- 
1.6.6

[PATCH 2/3] pnfs-submit: add back channel draining

[andros]

From: Andy Adamson <andros@netapp.com>

Currently the state manager only waits for the fore channel to drain.
The back channel processing must also be considered.
The current complete is really for the forechannel. Add a back channel complete.
Once the fore channel is drained, the state manager is the only thread
running - except for the back channel thread.

Have the state manager thread wait for the any current back channel processing
to complete (drain) before proceeding.

When the state manager is draining, stop all new back channel processing by
returning NFS4ERR_DELAY to the back channel client.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/callback_proc.c    |    6 ++++++
 fs/nfs/callback_xdr.c     |    1 +
 fs/nfs/internal.h         |    1 +
 fs/nfs/nfs4proc.c         |   26 +++++++++++++++++++-------
 fs/nfs/nfs4state.c        |   21 +++++++++++++++++++--
 include/linux/nfs_fs_sb.h |    2 +-
 6 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 6ba528a..9a2c051 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -630,6 +630,12 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
 	if (clp == NULL)
 		goto out;
 
+	/* state manager is resetting the session */
+	if (test_bit(NFS4_SESSION_DRAINING, &clp->cl_session->session_state)) {
+		status = NFS4ERR_DELAY;
+		goto out_putclient;
+	}
+
 	status = validate_seqid(&clp->cl_session->bc_slot_table, args);
 	if (status)
 		goto out_putclient;
diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index e05d1b3..67ad022 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -782,6 +782,7 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
 		 * A single slot, so highest used slotid is either 0 or -1
 		 */
 		tbl->highest_used_slotid--;
+		nfs41_check_drain_bc_complete(cps.session);
 		spin_unlock(&tbl->slot_tbl_lock);
 		dprintk("%s highest_used_slotid %d\n", __func__,
 			tbl->highest_used_slotid);
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 755e555..1ec5b95 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -311,6 +311,7 @@ extern int _nfs4_call_sync_session(struct nfs_server *server,
 				   struct nfs4_sequence_res *res,
 				   int cache_reply);
 
+void nfs41_check_drain_bc_complete(struct nfs4_session *ses);
 /*
  * Determine the device name as a string
  */
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index bf179bb..ab34149 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -357,9 +357,9 @@ nfs4_free_slot(struct nfs4_slot_table *tbl, struct nfs4_slot *free_slot)
 }
 
 /*
- * Signal state manager thread if session is drained
+ * Signal state manager thread if session fore channel is drained
  */
-static void nfs41_check_drain_session_complete(struct nfs4_session *ses)
+static void nfs41_check_drain_fc_complete(struct nfs4_session *ses)
 {
 	struct rpc_task *task;
 
@@ -373,8 +373,20 @@ static void nfs41_check_drain_session_complete(struct nfs4_session *ses)
 	if (ses->fc_slot_table.highest_used_slotid != -1)
 		return;
 
-	dprintk("%s COMPLETE: Session Drained\n", __func__);
-	complete(&ses->complete);
+	dprintk("%s COMPLETE: Session Fore Channel Drained\n", __func__);
+	complete(&ses->fc_slot_table.complete);
+}
+
+/*
+ * Signal state manager thread if session back channel is drained
+ */
+void nfs41_check_drain_bc_complete(struct nfs4_session *ses)
+{
+	if (!test_bit(NFS4_SESSION_DRAINING, &ses->session_state) ||
+	    ses->bc_slot_table.highest_used_slotid != -1)
+		return;
+	dprintk("%s COMPLETE: Session Back Channel Drained\n", __func__);
+	complete(&ses->bc_slot_table.complete);
 }
 
 static void nfs41_sequence_free_slot(struct nfs4_sequence_res *res)
@@ -391,7 +403,7 @@ static void nfs41_sequence_free_slot(struct nfs4_sequence_res *res)
 
 	spin_lock(&tbl->slot_tbl_lock);
 	nfs4_free_slot(tbl, res->sr_slot);
-	nfs41_check_drain_session_complete(res->sr_session);
+	nfs41_check_drain_fc_complete(res->sr_session);
 	spin_unlock(&tbl->slot_tbl_lock);
 	res->sr_slot = NULL;
 }
@@ -4864,17 +4876,17 @@ struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp)
 	if (!session)
 		return NULL;
 
-	init_completion(&session->complete);
-
 	tbl = &session->fc_slot_table;
 	tbl->highest_used_slotid = -1;
 	spin_lock_init(&tbl->slot_tbl_lock);
 	rpc_init_priority_wait_queue(&tbl->slot_tbl_waitq, "ForeChannel Slot table");
+	init_completion(&tbl->complete);
 
 	tbl = &session->bc_slot_table;
 	tbl->highest_used_slotid = -1;
 	spin_lock_init(&tbl->slot_tbl_lock);
 	rpc_init_wait_queue(&tbl->slot_tbl_waitq, "BackChannel Slot table");
+	init_completion(&tbl->complete);
 
 	session->session_state = 1<<NFS4_SESSION_INITING;
 
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 784f122..304cd30 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -147,6 +147,11 @@ static int nfs41_setup_state_renewal(struct nfs_client *clp)
 	return status;
 }
 
+/*
+ * Back channel returns NFS4ERR_DELAY for new requests when
+ * NFS4_SESSION_DRAINING is set so there is no work to be done when draining
+ * is ended.
+ */
 static void nfs4_end_drain_session(struct nfs_client *clp)
 {
 	struct nfs4_session *ses = clp->cl_session;
@@ -175,12 +180,24 @@ static int nfs4_begin_drain_session(struct nfs_client *clp)
 	struct nfs4_session *ses = clp->cl_session;
 	struct nfs4_slot_table *tbl = &ses->fc_slot_table;
 
+	/* fore channel */
 	spin_lock(&tbl->slot_tbl_lock);
 	set_bit(NFS4_SESSION_DRAINING, &ses->session_state);
 	if (tbl->highest_used_slotid != -1) {
-		INIT_COMPLETION(ses->complete);
+		INIT_COMPLETION(tbl->complete);
+		spin_unlock(&tbl->slot_tbl_lock);
+		wait_for_completion_interruptible(&tbl->complete);
+		goto backchannel;
+	}
+	spin_unlock(&tbl->slot_tbl_lock);
+backchannel:
+	/* back channel */
+	tbl = &ses->bc_slot_table;
+	spin_lock(&tbl->slot_tbl_lock);
+	if (tbl->highest_used_slotid != -1) {
+		INIT_COMPLETION(tbl->complete);
 		spin_unlock(&tbl->slot_tbl_lock);
-		return wait_for_completion_interruptible(&ses->complete);
+		return wait_for_completion_interruptible(&tbl->complete);
 	}
 	spin_unlock(&tbl->slot_tbl_lock);
 	return 0;
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 295d449..c3127cc 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -211,6 +211,7 @@ struct nfs4_slot_table {
 						 * op for dynamic resizing */
 	int		target_max_slots;	/* Set by CB_RECALL_SLOT as
 						 * the new max_slots */
+	struct completion complete;
 };
 
 static inline int slot_idx(struct nfs4_slot_table *tbl, struct nfs4_slot *sp)
@@ -227,7 +228,6 @@ struct nfs4_session {
 	unsigned long			session_state;
 	u32				hash_alg;
 	u32				ssv_len;
-	struct completion		complete;
 
 	/* The fore and back channel */
 	struct nfs4_channel_attrs	fc_attrs;
-- 
1.6.6

[PATCH 3/3] pnfs-submit: callbacks cannot use an nfs_client that is being freed

[andros]

From: Andy Adamson <andros@netapp.com>

Guarantee that the nfs_client exists when referenced to by callback processing
by not procssing callbacks on an nfs_client in the process of being freed.

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/client.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index dbf43e7..86657ee 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -392,7 +392,9 @@ struct nfs_client *nfs_find_client(const struct sockaddr *addr, u32 nfsversion)
 		if (!nfs_sockaddr_match_ipaddr(addr, clap))
 			continue;
 
-		atomic_inc(&clp->cl_count);
+		/* Don't return an nfs_client that is being freed */
+		if (!atomic_inc_not_zero(&clp->cl_count))
+			continue;
 		spin_unlock(&nfs_client_lock);
 		return clp;
 	}
@@ -425,7 +427,9 @@ struct nfs_client *nfs_find_client_next(struct nfs_client *clp)
 		if (!nfs_sockaddr_match_ipaddr(sap, clap))
 			continue;
 
-		atomic_inc(&clp->cl_count);
+		/* Don't return an nfs_client that is being freed */
+		if (!atomic_inc_not_zero(&clp->cl_count))
+			continue;
 		spin_unlock(&nfs_client_lock);
 		return clp;
 	}
-- 
1.6.6

Re: [PATCH 3/3] pnfs-submit: callbacks cannot use an nfs_client that is being freed

[William A. (Andy) Adamson]

On Thu, Nov 11, 2010 at 8:22 AM, Benny Halevy <bhalevy@panasas.com> wrote:
> On 2010-11-11 16:10, andros@netapp.com wrote:
>> From: Andy Adamson <andros@netapp.com>
>>
>> Guarantee that the nfs_client exists when referenced to by callback processing
>> by not procssing callbacks on an nfs_client in the process of being freed.
>>
>> Signed-off-by: Andy Adamson <andros@netapp.com>
>> ---
>>  fs/nfs/client.c |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c

>> index dbf43e7..86657ee 100644
>> --- a/fs/nfs/client.c
>> +++ b/fs/nfs/client.c
>> @@ -392,7 +392,9 @@ struct nfs_client *nfs_find_client(const struct sockaddr *addr, u32 nfsversion)
>>               if (!nfs_sockaddr_match_ipaddr(addr, clap))
>>                       continue;
>>
>> -             atomic_inc(&clp->cl_count);
>> +             /* Don't return an nfs_client that is being freed */
>> +             if (!atomic_inc_not_zero(&clp->cl_count))
>> +                     continue;
>>               spin_unlock(&nfs_client_lock);
>>               return clp;
>>       }
>> @@ -425,7 +427,9 @@ struct nfs_client *nfs_find_client_next(struct nfs_client *clp)
>>               if (!nfs_sockaddr_match_ipaddr(sap, clap))
>>                       continue;
>>
>> -             atomic_inc(&clp->cl_count);
>> +             /* Don't return an nfs_client that is being freed */
>> +             if (!atomic_inc_not_zero(&clp->cl_count))
>> +                     continue;
>>               spin_unlock(&nfs_client_lock);
>>               return clp;
>>       }
>
> Hmm, nfs_put_client deletes the client when cl_count reaches zero
> so how can cl_count be zero while clp is listed?


For some reason, I missed the lock part of atomic_dec_and_lock in
nfs_put_client which removes the nfs_client from the list under the
lock. We don't need this patch.

What's weird about the back channel server processing is that the RPC
layer pg_authenticate (nfs_callback_authenticate) call in
svc_process_common finds an nfs_client struct based solely the
callback client address and so may find the wrong nfs_client struct
(nfsv4.0 instead of v4.1, or wrong session). So the nfs_client has to
be put at the end of pg_authenticate and another nfs_find_client call
is needed in the dispatcher routines after decoding. This means the
callback server could start processing a callback and have the
nfs_client struct freed between the pg_authenticate call and the
dispatcher operation call, or it could have found the wrong nfs_client
in the first place.

If the nfs_client is not found in pg_authenticate, the request is
simply dropped (SVC_DROP). But if an nfs_client is not found in the
dispatcher routines NFS4ERR_BADSESSION is returned for v4.1 requests
and NFS4ERR_BADHANDLE for v4.0 requests.

I guess there's not much we can do about this.

-->Andy

> Benny
>
> In put_nfs_client
> --
> 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
>

Re: [PATCH 3/3] pnfs-submit: callbacks cannot use an nfs_client that is being freed

[Benny Halevy]

On 2010-11-12 10:13, William A. (Andy) Adamson wrote:
> On Thu, Nov 11, 2010 at 8:22 AM, Benny Halevy <bhalevy@panasas.com> wrote:
>> On 2010-11-11 16:10, andros@netapp.com wrote:
>>> From: Andy Adamson <andros@netapp.com>
>>>
>>> Guarantee that the nfs_client exists when referenced to by callback processing
>>> by not procssing callbacks on an nfs_client in the process of being freed.
>>>
>>> Signed-off-by: Andy Adamson <andros@netapp.com>
>>> ---
>>>  fs/nfs/client.c |    8 ++++++--
>>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> 
>>> index dbf43e7..86657ee 100644
>>> --- a/fs/nfs/client.c
>>> +++ b/fs/nfs/client.c
>>> @@ -392,7 +392,9 @@ struct nfs_client *nfs_find_client(const struct sockaddr *addr, u32 nfsversion)
>>>               if (!nfs_sockaddr_match_ipaddr(addr, clap))
>>>                       continue;
>>>
>>> -             atomic_inc(&clp->cl_count);
>>> +             /* Don't return an nfs_client that is being freed */
>>> +             if (!atomic_inc_not_zero(&clp->cl_count))
>>> +                     continue;
>>>               spin_unlock(&nfs_client_lock);
>>>               return clp;
>>>       }
>>> @@ -425,7 +427,9 @@ struct nfs_client *nfs_find_client_next(struct nfs_client *clp)
>>>               if (!nfs_sockaddr_match_ipaddr(sap, clap))
>>>                       continue;
>>>
>>> -             atomic_inc(&clp->cl_count);
>>> +             /* Don't return an nfs_client that is being freed */
>>> +             if (!atomic_inc_not_zero(&clp->cl_count))
>>> +                     continue;
>>>               spin_unlock(&nfs_client_lock);
>>>               return clp;
>>>       }
>>
>> Hmm, nfs_put_client deletes the client when cl_count reaches zero
>> so how can cl_count be zero while clp is listed?
> 
> 
> For some reason, I missed the lock part of atomic_dec_and_lock in
> nfs_put_client which removes the nfs_client from the list under the
> lock. We don't need this patch.
> 
> What's weird about the back channel server processing is that the RPC
> layer pg_authenticate (nfs_callback_authenticate) call in
> svc_process_common finds an nfs_client struct based solely the
> callback client address and so may find the wrong nfs_client struct
> (nfsv4.0 instead of v4.1, or wrong session). So the nfs_client has to
> be put at the end of pg_authenticate and another nfs_find_client call
> is needed in the dispatcher routines after decoding. This means the
> callback server could start processing a callback and have the
> nfs_client struct freed between the pg_authenticate call and the
> dispatcher operation call, or it could have found the wrong nfs_client
> in the first place.

Seems bad enough to fix :)

Benny

> 
> If the nfs_client is not found in pg_authenticate, the request is
> simply dropped (SVC_DROP). But if an nfs_client is not found in the
> dispatcher routines NFS4ERR_BADSESSION is returned for v4.1 requests
> and NFS4ERR_BADHANDLE for v4.0 requests.
> 
> I guess there's not much we can do about this.
> 
> -->Andy
> 
>> Benny
>>
>> In put_nfs_client
>> --
>> 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
>>

[PATCH] SQUASHME: pnfs-submit: highest backchannel slot used for !CONFIG_NFS_V4_1

[Benny Halevy]

Signed-off-by: Benny Halevy <bhalevy@panasas.com>
---
 fs/nfs/callback_xdr.c |   38 +++++++++++++++++++++-----------------
 1 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index 39da338..a77877c 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -728,6 +728,24 @@ encode_hdr:
 	return status;
 }
 
+static void nfs4_callback_free_slot(struct nfs4_session *session)
+{
+#if defined(CONFIG_NFS_V4_1)
+	struct nfs4_slot_table *tbl = &session->bc_slot_table;
+
+	spin_lock(&tbl->slot_tbl_lock);
+	/*
+	 * Let the state manager know callback processing done.
+	 * A single slot, so highest used slotid is either 0 or -1
+	 */
+	tbl->highest_used_slotid--;
+	nfs41_check_drain_bc_complete(session);
+	spin_unlock(&tbl->slot_tbl_lock);
+	dprintk("%s highest_used_slotid %d\n", __func__,
+		tbl->highest_used_slotid);
+#endif /* CONFIG_NFS_V4_1 */
+}
+
 /*
  * Decode, process and encode a COMPOUND
  */
@@ -773,23 +791,9 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
 
 	*hdr_res.status = status;
 	*hdr_res.nops = htonl(nops);
-	if (cps.session) {
-		struct nfs4_slot_table *tbl = &cps.session->bc_slot_table;
-
-		spin_lock(&tbl->slot_tbl_lock);
-		/*
-		 * Let the state manager know callback processing done.
-		 * A single slot, so highest used slotid is either 0 or -1
-		 */
-		tbl->highest_used_slotid--;
-		nfs41_check_drain_bc_complete(cps.session);
-		spin_unlock(&tbl->slot_tbl_lock);
-		dprintk("%s highest_used_slotid %d\n", __func__,
-			tbl->highest_used_slotid);
-
-		/* matched by cb_sequence find_client_with_session */
-                put_session_client(cps.session);
-        }
+	nfs4_callback_free_slot(cps.session);
+	/* matched by cb_sequence find_client_with_session */
+	put_session_client(cps.session);
 	dprintk("%s: done, status = %u\n", __func__, ntohl(status));
 	return rpc_success;
 }
-- 
1.7.2.3