2010-11-11 02:11:22

by Andy Adamson

[permalink] [raw]
Subject: [PATCH 0/3] Fix session draining and back channel processing


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



2010-11-11 02:13:37

by Andy Adamson

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

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, [email protected] 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2010-11-11 02:11:23

by Andy Adamson

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

From: Andy Adamson <[email protected]>

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 <[email protected]>
---
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


2010-11-11 02:11:23

by Andy Adamson

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

From: Andy Adamson <[email protected]>

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 <[email protected]>
---
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


2010-11-14 12:08:42

by Benny Halevy

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

Signed-off-by: Benny Halevy <[email protected]>
---
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


2010-11-12 08:13:33

by Andy Adamson

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

On Thu, Nov 11, 2010 at 8:22 AM, Benny Halevy <[email protected]> wrote:
> On 2010-11-11 16:10, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2010-11-11 13:22:16

by Benny Halevy

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

On 2010-11-11 16:10, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> 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 <[email protected]>
> ---
> 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

2010-11-11 02:11:22

by Andy Adamson

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

From: Andy Adamson <[email protected]>

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 <[email protected]>
---
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


2010-11-12 10:08:20

by Benny Halevy

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

On 2010-11-12 10:13, William A. (Andy) Adamson wrote:
> On Thu, Nov 11, 2010 at 8:22 AM, Benny Halevy <[email protected]> wrote:
>> On 2010-11-11 16:10, [email protected] wrote:
>>> From: Andy Adamson <[email protected]>
>>>
>>> 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 <[email protected]>
>>> ---
>>> 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>