The following patches fix bugs in and do some minor cleanup of the
server 4.1 sessions code. I'm thinking of submitting them for 2.6.37.
They come from efforts to prepare some patches that address:
http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#Trunking
http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#BIND_CONN_TO_SESSION
which aren't yet ready to merge. (They may still be ready in time for
2.6.37, but I'm not sure.)
--b.
On 2010-10-21 18:20, J. Bruce Fields wrote:
> From: J. Bruce Fields <[email protected]>
>
> The backchannel should be associated with a session, it isn't really
> global to the client.
>
> We do, however, want a pointer global to the client which tracks which
> session we're currently using for client-based callbacks.
>
> This is a first step in that direction; for now, just reshuffling of
> code with no significant change in behavior.
>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 29 ++++++++++++++---------------
> fs/nfsd/state.h | 1 +
> 2 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 7f12828..db5d8c8 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -771,6 +771,19 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
> free_session(&new->se_ref);
> return nfserr_jukebox;
> }
> + if (!clp->cl_cb_session && (cses->flags & SESSION4_BACK_CHAN)) {
> + struct sockaddr *sa = svc_addr(rqstp);
> +
> + clp->cl_cb_session = new;
> + clp->cl_cb_conn.cb_xprt = rqstp->rq_xprt;
> + svc_xprt_get(rqstp->rq_xprt);
> + rpc_copy_addr((struct sockaddr *)&clp->cl_cb_conn.cb_addr, sa);
> + clp->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
> + clp->cl_cb_conn.cb_minorversion = 1;
> + clp->cl_cb_conn.cb_prog = cses->callback_prog;
> + clp->cl_cb_seq_nr = 1;
> + nfsd4_probe_callback(clp, &clp->cl_cb_conn);
> + }
else
cses->flags &= ~SESSION4_BACK_CHAN;
We need that for returning the right value in csr_flags
Benny
> return nfs_ok;
> }
>
> @@ -1045,7 +1058,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir,
> clp->cl_flavor = rqstp->rq_flavor;
> copy_cred(&clp->cl_cred, &rqstp->rq_cred);
> gen_confirm(clp);
> -
> + clp->cl_cb_session = NULL;
> return clp;
> }
>
> @@ -1515,20 +1528,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
>
> cs_slot->sl_seqid++; /* from 0 to 1 */
> move_to_confirmed(unconf);
> -
> - if (cr_ses->flags & SESSION4_BACK_CHAN) {
> - unconf->cl_cb_conn.cb_xprt = rqstp->rq_xprt;
> - svc_xprt_get(rqstp->rq_xprt);
> - rpc_copy_addr(
> - (struct sockaddr *)&unconf->cl_cb_conn.cb_addr,
> - sa);
> - unconf->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
> - unconf->cl_cb_conn.cb_minorversion =
> - cstate->minorversion;
> - unconf->cl_cb_conn.cb_prog = cr_ses->callback_prog;
> - unconf->cl_cb_seq_nr = 1;
> - nfsd4_probe_callback(unconf, &unconf->cl_cb_conn);
> - }
> conf = unconf;
> } else {
> status = nfserr_stale_clientid;
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 8d5e237..6e63c1d 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -234,6 +234,7 @@ struct nfs4_client {
> u32 cl_cb_ident;
> atomic_t cl_cb_set;
> struct nfsd4_callback cl_cb_null;
> + struct nfsd4_session *cl_cb_session;
>
> /* for all client information that callback code might need: */
> spinlock_t cl_lock;
In the sessions backchannel case, we don't need a krb5 principal name
for the client; we use the already-created forechannel credentials
instead.
Some cleanup, while we're there: make it clearer which code here is 4.0-
or sessions- specific.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4callback.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 67bcd2c..143da2e 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -481,22 +481,24 @@ int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *conn)
};
struct rpc_create_args args = {
.net = &init_net,
- .protocol = XPRT_TRANSPORT_TCP,
.address = (struct sockaddr *) &conn->cb_addr,
.addrsize = conn->cb_addrlen,
.timeout = &timeparms,
.program = &cb_program,
- .prognumber = conn->cb_prog,
.version = 0,
.authflavor = clp->cl_flavor,
.flags = (RPC_CLNT_CREATE_NOPING | RPC_CLNT_CREATE_QUIET),
- .client_name = clp->cl_principal,
};
struct rpc_clnt *client;
- if (!clp->cl_principal && (clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
- return -EINVAL;
- if (clp->cl_minorversion) {
+ if (clp->cl_minorversion == 0) {
+ if (!clp->cl_principal && (clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
+ return -EINVAL;
+ args.client_name = clp->cl_principal;
+ args.prognumber = conn->cb_prog,
+ args.protocol = XPRT_TRANSPORT_TCP;
+ clp->cl_cb_ident = conn->cb_ident;
+ } else {
args.bc_xprt = conn->cb_xprt;
args.prognumber = clp->cl_cb_session->se_cb_prog;
args.protocol = XPRT_TRANSPORT_BC_TCP;
@@ -508,7 +510,6 @@ int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *conn)
PTR_ERR(client));
return PTR_ERR(client);
}
- clp->cl_cb_ident = conn->cb_ident;
clp->cl_cb_client = client;
return 0;
--
1.7.1
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 02c23b7..7f12828 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1510,7 +1510,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
if (status) {
/* an unconfirmed replay returns misordered */
status = nfserr_seq_misordered;
- goto out_cache;
+ goto out;
}
cs_slot->sl_seqid++; /* from 0 to 1 */
@@ -1549,7 +1549,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
NFS4_MAX_SESSIONID_LEN);
cr_ses->seqid = cs_slot->sl_seqid;
-out_cache:
/* cache solo and embedded create sessions under the state lock */
nfsd4_cache_create_session(cr_ses, cs_slot, status);
out:
--
1.7.1
On 2010-10-27 19:26, Benny Halevy wrote:
> On 2010-10-21 18:20, J. Bruce Fields wrote:
>> From: J. Bruce Fields <[email protected]>
>>
>> The backchannel should be associated with a session, it isn't really
>> global to the client.
>>
>> We do, however, want a pointer global to the client which tracks which
>> session we're currently using for client-based callbacks.
>>
>> This is a first step in that direction; for now, just reshuffling of
>> code with no significant change in behavior.
>>
>> Signed-off-by: J. Bruce Fields <[email protected]>
>> ---
>> fs/nfsd/nfs4state.c | 29 ++++++++++++++---------------
>> fs/nfsd/state.h | 1 +
>> 2 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 7f12828..db5d8c8 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -771,6 +771,19 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
>> free_session(&new->se_ref);
>> return nfserr_jukebox;
>> }
>> + if (!clp->cl_cb_session && (cses->flags & SESSION4_BACK_CHAN)) {
>> + struct sockaddr *sa = svc_addr(rqstp);
>> +
>> + clp->cl_cb_session = new;
>> + clp->cl_cb_conn.cb_xprt = rqstp->rq_xprt;
>> + svc_xprt_get(rqstp->rq_xprt);
>> + rpc_copy_addr((struct sockaddr *)&clp->cl_cb_conn.cb_addr, sa);
>> + clp->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
>> + clp->cl_cb_conn.cb_minorversion = 1;
>> + clp->cl_cb_conn.cb_prog = cses->callback_prog;
>> + clp->cl_cb_seq_nr = 1;
>> + nfsd4_probe_callback(clp, &clp->cl_cb_conn);
>> + }
>
> else
> cses->flags &= ~SESSION4_BACK_CHAN;
>
> We need that for returning the right value in csr_flags
Though after "nfsd4: track backchannel connections",
we mark the connection correctly.
Too late to worry about bisectibility now :)
Benny
>
> Benny
>
>> return nfs_ok;
>> }
>>
>> @@ -1045,7 +1058,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir,
>> clp->cl_flavor = rqstp->rq_flavor;
>> copy_cred(&clp->cl_cred, &rqstp->rq_cred);
>> gen_confirm(clp);
>> -
>> + clp->cl_cb_session = NULL;
>> return clp;
>> }
>>
>> @@ -1515,20 +1528,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
>>
>> cs_slot->sl_seqid++; /* from 0 to 1 */
>> move_to_confirmed(unconf);
>> -
>> - if (cr_ses->flags & SESSION4_BACK_CHAN) {
>> - unconf->cl_cb_conn.cb_xprt = rqstp->rq_xprt;
>> - svc_xprt_get(rqstp->rq_xprt);
>> - rpc_copy_addr(
>> - (struct sockaddr *)&unconf->cl_cb_conn.cb_addr,
>> - sa);
>> - unconf->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
>> - unconf->cl_cb_conn.cb_minorversion =
>> - cstate->minorversion;
>> - unconf->cl_cb_conn.cb_prog = cr_ses->callback_prog;
>> - unconf->cl_cb_seq_nr = 1;
>> - nfsd4_probe_callback(unconf, &unconf->cl_cb_conn);
>> - }
>> conf = unconf;
>> } else {
>> status = nfserr_stale_clientid;
>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
>> index 8d5e237..6e63c1d 100644
>> --- a/fs/nfsd/state.h
>> +++ b/fs/nfsd/state.h
>> @@ -234,6 +234,7 @@ struct nfs4_client {
>> u32 cl_cb_ident;
>> atomic_t cl_cb_set;
>> struct nfsd4_callback cl_cb_null;
>> + struct nfsd4_session *cl_cb_session;
>>
>> /* for all client information that callback code might need: */
>> spinlock_t cl_lock;
> --
> 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
On Wed, Oct 27, 2010 at 07:45:49PM +0200, Benny Halevy wrote:
> On 2010-10-27 19:26, Benny Halevy wrote:
> > On 2010-10-21 18:20, J. Bruce Fields wrote:
> >> From: J. Bruce Fields <[email protected]>
> >>
> >> The backchannel should be associated with a session, it isn't really
> >> global to the client.
> >>
> >> We do, however, want a pointer global to the client which tracks which
> >> session we're currently using for client-based callbacks.
> >>
> >> This is a first step in that direction; for now, just reshuffling of
> >> code with no significant change in behavior.
> >>
> >> Signed-off-by: J. Bruce Fields <[email protected]>
> >> ---
> >> fs/nfsd/nfs4state.c | 29 ++++++++++++++---------------
> >> fs/nfsd/state.h | 1 +
> >> 2 files changed, 15 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> >> index 7f12828..db5d8c8 100644
> >> --- a/fs/nfsd/nfs4state.c
> >> +++ b/fs/nfsd/nfs4state.c
> >> @@ -771,6 +771,19 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
> >> free_session(&new->se_ref);
> >> return nfserr_jukebox;
> >> }
> >> + if (!clp->cl_cb_session && (cses->flags & SESSION4_BACK_CHAN)) {
> >> + struct sockaddr *sa = svc_addr(rqstp);
> >> +
> >> + clp->cl_cb_session = new;
> >> + clp->cl_cb_conn.cb_xprt = rqstp->rq_xprt;
> >> + svc_xprt_get(rqstp->rq_xprt);
> >> + rpc_copy_addr((struct sockaddr *)&clp->cl_cb_conn.cb_addr, sa);
> >> + clp->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
> >> + clp->cl_cb_conn.cb_minorversion = 1;
> >> + clp->cl_cb_conn.cb_prog = cses->callback_prog;
> >> + clp->cl_cb_seq_nr = 1;
> >> + nfsd4_probe_callback(clp, &clp->cl_cb_conn);
> >> + }
> >
> > else
> > cses->flags &= ~SESSION4_BACK_CHAN;
> >
> > We need that for returning the right value in csr_flags
>
> Though after "nfsd4: track backchannel connections",
> we mark the connection correctly.
> Too late to worry about bisectibility now :)
Erp, yes. We should try to coordinate review better, apologies if I
haven't been leaving enough time. Let me know if you catch anything
else.
--b.
The minorversion seems more a property of the client than the callback
channel.
Some time we should probably also enforce consistent minorversion usage
from the client; for now, this is just a cosmetic change.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4callback.c | 8 ++++----
fs/nfsd/nfs4state.c | 12 ++++++++++--
fs/nfsd/state.h | 2 +-
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index d38ee3c..67bcd2c 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -496,7 +496,7 @@ int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *conn)
if (!clp->cl_principal && (clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
return -EINVAL;
- if (conn->cb_minorversion) {
+ if (clp->cl_minorversion) {
args.bc_xprt = conn->cb_xprt;
args.prognumber = clp->cl_cb_session->se_cb_prog;
args.protocol = XPRT_TRANSPORT_BC_TCP;
@@ -620,7 +620,7 @@ static void nfsd4_cb_prepare(struct rpc_task *task, void *calldata)
struct nfsd4_callback *cb = calldata;
struct nfs4_delegation *dp = container_of(cb, struct nfs4_delegation, dl_recall);
struct nfs4_client *clp = dp->dl_client;
- u32 minorversion = clp->cl_cb_conn.cb_minorversion;
+ u32 minorversion = clp->cl_minorversion;
int status = 0;
cb->cb_minorversion = minorversion;
@@ -645,9 +645,9 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
struct nfs4_client *clp = dp->dl_client;
dprintk("%s: minorversion=%d\n", __func__,
- clp->cl_cb_conn.cb_minorversion);
+ clp->cl_minorversion);
- if (clp->cl_cb_conn.cb_minorversion) {
+ if (clp->cl_minorversion) {
/* No need for lock, access serialized in nfsd4_cb_prepare */
++clp->cl_cb_session->se_cb_seq_nr;
clear_bit(0, &clp->cl_cb_slot_busy);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0f2643d..ce0412f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -782,7 +782,6 @@ static struct nfsd4_session *alloc_init_session(struct svc_rqst *rqstp, struct n
svc_xprt_get(rqstp->rq_xprt);
rpc_copy_addr((struct sockaddr *)&clp->cl_cb_conn.cb_addr, sa);
clp->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
- clp->cl_cb_conn.cb_minorversion = 1;
nfsd4_probe_callback(clp);
}
return new;
@@ -1200,7 +1199,6 @@ gen_callback(struct nfs4_client *clp, struct nfsd4_setclientid *se, u32 scopeid)
if (conn->cb_addr.ss_family == AF_INET6)
((struct sockaddr_in6 *)&conn->cb_addr)->sin6_scope_id = scopeid;
- conn->cb_minorversion = 0;
conn->cb_prog = se->se_callback_prog;
conn->cb_ident = se->se_callback_ident;
return;
@@ -1541,6 +1539,11 @@ nfsd4_create_session(struct svc_rqst *rqstp,
}
/*
+ * XXX: we should probably set this at creation time, and check
+ * for consistent minorversion use throughout:
+ */
+ conf->cl_minorversion = 1;
+ /*
* We do not support RDMA or persistent sessions
*/
cr_ses->flags &= ~SESSION4_PERSIST;
@@ -1857,6 +1860,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
goto out;
gen_clid(new);
}
+ /*
+ * XXX: we should probably set this at creation time, and check
+ * for consistent minorversion use throughout:
+ */
+ new->cl_minorversion = 0;
gen_callback(new, setclid, rpc_get_scope_id(sa));
add_to_unconfirmed(new, strhashval);
setclid->se_clientid.cl_boot = new->cl_clientid.cl_boot;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index bbc4d58..39adc27 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -98,7 +98,6 @@ struct nfs4_cb_conn {
size_t cb_addrlen;
u32 cb_prog; /* used only in 4.0 case;
per-session otherwise */
- u32 cb_minorversion;
u32 cb_ident; /* minorversion 0 only */
struct svc_xprt *cb_xprt; /* minorversion 1 only */
};
@@ -227,6 +226,7 @@ struct nfs4_client {
clientid_t cl_clientid; /* generated by server */
nfs4_verifier cl_confirm; /* generated by server */
u32 cl_firststate; /* recovery dir creation */
+ u32 cl_minorversion;
/* for v4.0 and v4.1 callbacks: */
struct nfs4_cb_conn cl_cb_conn;
--
1.7.1
From: J. Bruce Fields <[email protected]>
Currently we don't deal well with a client that has multiple sessions
associated with it (even simultaneously, or serially over the lifetime
of the client).
In particular, we don't attempt to keep the backchannel running after
the original session diseappears.
We will fix that soon.
Once we do that, we need the slot sequence number to be per-session;
otherwise, for example, we cannot correctly handle a case like this:
- All session 1 connections are lost.
- The client creates session 2. We use it for the backchannel
(since it's the only working choice).
- The client gives us a new connection to use with session 1.
- The client destroys session 2.
At this point our only choice is to go back to using session 1. When we
do so we must use the sequence number that is next for session 1. We
therefore need to maintain multiple sequence number streams.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4callback.c | 8 ++++----
fs/nfsd/nfs4state.c | 22 ++++++++++++----------
fs/nfsd/state.h | 2 +-
3 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 78ac779..5df9dda 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -260,7 +260,7 @@ encode_cb_sequence(struct xdr_stream *xdr, struct nfsd4_callback *cb,
WRITE32(OP_CB_SEQUENCE);
WRITEMEM(ses->se_sessionid.data, NFS4_MAX_SESSIONID_LEN);
- WRITE32(cb->cb_clp->cl_cb_seq_nr);
+ WRITE32(ses->se_cb_seq_nr);
WRITE32(0); /* slotid, always 0 */
WRITE32(0); /* highest slotid always 0 */
WRITE32(0); /* cachethis always 0 */
@@ -369,7 +369,7 @@ decode_cb_sequence(struct xdr_stream *xdr, struct nfsd4_callback *cb,
goto out;
}
READ32(dummy);
- if (dummy != cb->cb_clp->cl_cb_seq_nr) {
+ if (dummy != ses->se_cb_seq_nr) {
dprintk("%s Invalid sequence number\n", __func__);
goto out;
}
@@ -643,11 +643,11 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
if (clp->cl_cb_conn.cb_minorversion) {
/* No need for lock, access serialized in nfsd4_cb_prepare */
- ++clp->cl_cb_seq_nr;
+ ++clp->cl_cb_session->se_cb_seq_nr;
clear_bit(0, &clp->cl_cb_slot_busy);
rpc_wake_up_next(&clp->cl_cb_waitq);
dprintk("%s: freed slot, new seqid=%d\n", __func__,
- clp->cl_cb_seq_nr);
+ clp->cl_cb_session->se_cb_seq_nr);
/* We're done looking into the sequence information */
task->tk_msg.rpc_resp = NULL;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c942511..6367c44 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -725,8 +725,7 @@ void free_session(struct kref *kref)
kfree(ses);
}
-
-static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp, struct nfsd4_create_session *cses)
+static struct nfsd4_session *alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp, struct nfsd4_create_session *cses)
{
struct nfsd4_session *new;
struct nfsd4_channel_attrs *fchan = &cses->fore_channel;
@@ -747,7 +746,7 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
new = alloc_session(slotsize, numslots);
if (!new) {
nfsd4_put_drc_mem(slotsize, fchan->maxreqs);
- return nfserr_jukebox;
+ return NULL;
}
init_forechannel_attrs(&new->se_fchannel, fchan, numslots, slotsize);
@@ -756,6 +755,7 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
INIT_LIST_HEAD(&new->se_conns);
+ new->se_cb_seq_nr = 1;
new->se_flags = cses->flags;
kref_init(&new->se_ref);
idx = hash_sessionid(&new->se_sessionid);
@@ -765,9 +765,10 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
spin_unlock(&client_lock);
status = nfsd4_new_conn(rqstp, new);
+ /* whoops: benny points out, status is ignored! (err, or bogus) */
if (status) {
free_session(&new->se_ref);
- return nfserr_jukebox;
+ return NULL;
}
if (!clp->cl_cb_session && (cses->flags & SESSION4_BACK_CHAN)) {
struct sockaddr *sa = svc_addr(rqstp);
@@ -779,10 +780,9 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
clp->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
clp->cl_cb_conn.cb_minorversion = 1;
clp->cl_cb_conn.cb_prog = cses->callback_prog;
- clp->cl_cb_seq_nr = 1;
nfsd4_probe_callback(clp, &clp->cl_cb_conn);
}
- return nfs_ok;
+ return new;
}
/* caller must hold client_lock */
@@ -1485,6 +1485,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
{
struct sockaddr *sa = svc_addr(rqstp);
struct nfs4_client *conf, *unconf;
+ struct nfsd4_session *new;
struct nfsd4_clid_slot *cs_slot = NULL;
int status = 0;
@@ -1538,11 +1539,12 @@ nfsd4_create_session(struct svc_rqst *rqstp,
cr_ses->flags &= ~SESSION4_PERSIST;
cr_ses->flags &= ~SESSION4_RDMA;
- status = alloc_init_session(rqstp, conf, cr_ses);
- if (status)
+ status = nfserr_jukebox;
+ new = alloc_init_session(rqstp, conf, cr_ses);
+ if (!new)
goto out;
-
- memcpy(cr_ses->sessionid.data, conf->cl_cb_session->se_sessionid.data,
+ status = nfs_ok;
+ memcpy(cr_ses->sessionid.data, new->se_sessionid.data,
NFS4_MAX_SESSIONID_LEN);
cr_ses->seqid = cs_slot->sl_seqid;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index cdce26a..7f5b267 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -172,6 +172,7 @@ struct nfsd4_session {
struct nfsd4_channel_attrs se_fchannel;
struct nfsd4_channel_attrs se_bchannel;
struct list_head se_conns;
+ u32 se_cb_seq_nr;
struct nfsd4_slot *se_slots[]; /* forward channel slots */
};
@@ -249,7 +250,6 @@ struct nfs4_client {
/* for nfs41 callbacks */
/* We currently support a single back channel with a single slot */
unsigned long cl_cb_slot_busy;
- u32 cl_cb_seq_nr;
struct rpc_wait_queue cl_cb_waitq; /* backchannel callers may */
/* wait here for slots */
};
--
1.7.1
From: J. Bruce Fields <[email protected]>
The backchannel should be associated with a session, it isn't really
global to the client.
We do, however, want a pointer global to the client which tracks which
session we're currently using for client-based callbacks.
This is a first step in that direction; for now, just reshuffling of
code with no significant change in behavior.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 29 ++++++++++++++---------------
fs/nfsd/state.h | 1 +
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7f12828..db5d8c8 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -771,6 +771,19 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
free_session(&new->se_ref);
return nfserr_jukebox;
}
+ if (!clp->cl_cb_session && (cses->flags & SESSION4_BACK_CHAN)) {
+ struct sockaddr *sa = svc_addr(rqstp);
+
+ clp->cl_cb_session = new;
+ clp->cl_cb_conn.cb_xprt = rqstp->rq_xprt;
+ svc_xprt_get(rqstp->rq_xprt);
+ rpc_copy_addr((struct sockaddr *)&clp->cl_cb_conn.cb_addr, sa);
+ clp->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
+ clp->cl_cb_conn.cb_minorversion = 1;
+ clp->cl_cb_conn.cb_prog = cses->callback_prog;
+ clp->cl_cb_seq_nr = 1;
+ nfsd4_probe_callback(clp, &clp->cl_cb_conn);
+ }
return nfs_ok;
}
@@ -1045,7 +1058,7 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir,
clp->cl_flavor = rqstp->rq_flavor;
copy_cred(&clp->cl_cred, &rqstp->rq_cred);
gen_confirm(clp);
-
+ clp->cl_cb_session = NULL;
return clp;
}
@@ -1515,20 +1528,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
cs_slot->sl_seqid++; /* from 0 to 1 */
move_to_confirmed(unconf);
-
- if (cr_ses->flags & SESSION4_BACK_CHAN) {
- unconf->cl_cb_conn.cb_xprt = rqstp->rq_xprt;
- svc_xprt_get(rqstp->rq_xprt);
- rpc_copy_addr(
- (struct sockaddr *)&unconf->cl_cb_conn.cb_addr,
- sa);
- unconf->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
- unconf->cl_cb_conn.cb_minorversion =
- cstate->minorversion;
- unconf->cl_cb_conn.cb_prog = cr_ses->callback_prog;
- unconf->cl_cb_seq_nr = 1;
- nfsd4_probe_callback(unconf, &unconf->cl_cb_conn);
- }
conf = unconf;
} else {
status = nfserr_stale_clientid;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 8d5e237..6e63c1d 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -234,6 +234,7 @@ struct nfs4_client {
u32 cl_cb_ident;
atomic_t cl_cb_set;
struct nfsd4_callback cl_cb_null;
+ struct nfsd4_session *cl_cb_session;
/* for all client information that callback code might need: */
spinlock_t cl_lock;
--
1.7.1
On Thu, Oct 21, 2010 at 12:20:07PM -0400, J. Bruce Fields wrote:
> The following patches fix bugs in and do some minor cleanup of the
> server 4.1 sessions code. I'm thinking of submitting them for 2.6.37.
Also, one obvious screwup that I just noticed.
--b.
commit 3f87b9f3ea2517c79aa274fa56df7a17f8e29585
Author: J. Bruce Fields <[email protected]>
Date: Thu Oct 21 17:17:31 2010 -0400
nfsd4: fix connection allocation in sequence()
We're doing an allocation under a spinlock, and ignoring the
possibility of allocation failure.
A better fix wouldn't require an unnecessary allocation in the common
case, but we'll leave that for later.
Signed-off-by: J. Bruce Fields <[email protected]>
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ce0412f..d4aa1b5 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1628,33 +1628,25 @@ out:
return status;
}
-static struct nfsd4_conn *__nfsd4_find_conn(struct svc_rqst *r, struct nfsd4_session *s)
+static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, struct nfsd4_session *s)
{
struct nfsd4_conn *c;
list_for_each_entry(c, &s->se_conns, cn_persession) {
- if (c->cn_xprt == r->rq_xprt) {
+ if (c->cn_xprt == xpt) {
return c;
}
}
return NULL;
}
-static void nfsd4_sequence_check_conn(struct svc_rqst *rqstp, struct nfsd4_session *ses)
+static void nfsd4_sequence_check_conn(struct nfsd4_conn *new, struct nfsd4_session *ses)
{
struct nfs4_client *clp = ses->se_client;
- struct nfsd4_conn *c, *new = NULL;
-
- spin_lock(&clp->cl_lock);
- c = __nfsd4_find_conn(rqstp, ses);
- spin_unlock(&clp->cl_lock);
- if (c)
- return;
-
- new = alloc_conn(rqstp, NFS4_CDFC4_FORE);
+ struct nfsd4_conn *c;
spin_lock(&clp->cl_lock);
- c = __nfsd4_find_conn(rqstp, ses);
+ c = __nfsd4_find_conn(new->cn_xprt, ses);
if (c) {
spin_unlock(&clp->cl_lock);
free_conn(new);
@@ -1674,11 +1666,20 @@ nfsd4_sequence(struct svc_rqst *rqstp,
struct nfsd4_compoundres *resp = rqstp->rq_resp;
struct nfsd4_session *session;
struct nfsd4_slot *slot;
+ struct nfsd4_conn *conn;
int status;
if (resp->opcnt != 1)
return nfserr_sequence_pos;
+ /*
+ * Will be either used or freed by nfsd4_sequence_check_conn
+ * below.
+ */
+ conn = alloc_conn(rqstp, NFS4_CDFC4_FORE);
+ if (!conn)
+ return nfserr_jukebox;
+
spin_lock(&client_lock);
status = nfserr_badsession;
session = find_in_sessionid_hashtbl(&seq->sessionid);
@@ -1710,7 +1711,8 @@ nfsd4_sequence(struct svc_rqst *rqstp,
if (status)
goto out;
- nfsd4_sequence_check_conn(rqstp, session);
+ nfsd4_sequence_check_conn(conn, session);
+ conn = NULL;
/* Success! bump slot seqid */
slot->sl_inuse = true;
@@ -1726,6 +1728,7 @@ out:
nfsd4_get_session(cstate->session);
atomic_inc(&session->se_client->cl_refcount);
}
+ kfree(conn);
spin_unlock(&client_lock);
dprintk("%s: return %d\n", __func__, ntohl(status));
return status;
Following rfc 5661, section 18.36.4: "If the session is not successfully
created, then no changes are made to any client records on the server."
We shouldn't be confirming or incrementing the sequence id in this case.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 6367c44..7e817d1 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1487,6 +1487,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
struct nfs4_client *conf, *unconf;
struct nfsd4_session *new;
struct nfsd4_clid_slot *cs_slot = NULL;
+ bool confirm_me = false;
int status = 0;
nfs4_lock_state();
@@ -1509,7 +1510,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
cs_slot->sl_seqid, cr_ses->seqid);
goto out;
}
- cs_slot->sl_seqid++;
} else if (unconf) {
if (!same_creds(&unconf->cl_cred, &rqstp->rq_cred) ||
!rpc_cmp_addr(sa, (struct sockaddr *) &unconf->cl_addr)) {
@@ -1525,8 +1525,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
goto out;
}
- cs_slot->sl_seqid++; /* from 0 to 1 */
- move_to_confirmed(unconf);
+ confirm_me = true;
conf = unconf;
} else {
status = nfserr_stale_clientid;
@@ -1546,10 +1545,13 @@ nfsd4_create_session(struct svc_rqst *rqstp,
status = nfs_ok;
memcpy(cr_ses->sessionid.data, new->se_sessionid.data,
NFS4_MAX_SESSIONID_LEN);
+ cs_slot->sl_seqid++;
cr_ses->seqid = cs_slot->sl_seqid;
/* cache solo and embedded create sessions under the state lock */
nfsd4_cache_create_session(cr_ses, cs_slot, status);
+ if (confirm_me)
+ move_to_confirmed(conf);
out:
nfs4_unlock_state();
dprintk("%s returns %d\n", __func__, ntohl(status));
--
1.7.1
Only one of the nfsd4_callback_probe callers actually cares about
changing the callback information.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4callback.c | 15 ++++++++++-----
fs/nfsd/nfs4state.c | 7 ++++---
fs/nfsd/state.h | 3 ++-
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 140bb36..d38ee3c 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -550,7 +550,7 @@ int set_callback_cred(void)
static struct workqueue_struct *callback_wq;
-void do_probe_callback(struct nfs4_client *clp)
+static void do_probe_callback(struct nfs4_client *clp)
{
struct nfsd4_callback *cb = &clp->cl_cb_null;
@@ -568,17 +568,22 @@ void do_probe_callback(struct nfs4_client *clp)
}
/*
- * Set up the callback client and put a NFSPROC4_CB_NULL on the wire...
+ * Poke the callback thread to process any updates to the callback
+ * parameters, and send a null probe.
*/
-void nfsd4_probe_callback(struct nfs4_client *clp, struct nfs4_cb_conn *conn)
+void nfsd4_probe_callback(struct nfs4_client *clp)
+{
+ set_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_cb_flags);
+ do_probe_callback(clp);
+}
+
+void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *conn)
{
BUG_ON(atomic_read(&clp->cl_cb_set));
spin_lock(&clp->cl_lock);
memcpy(&clp->cl_cb_conn, conn, sizeof(struct nfs4_cb_conn));
- set_bit(NFSD4_CLIENT_CB_UPDATE, &clp->cl_cb_flags);
spin_unlock(&clp->cl_lock);
- do_probe_callback(clp);
}
/*
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 59bc001..2327a8c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -783,7 +783,7 @@ static struct nfsd4_session *alloc_init_session(struct svc_rqst *rqstp, struct n
rpc_copy_addr((struct sockaddr *)&clp->cl_cb_conn.cb_addr, sa);
clp->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
clp->cl_cb_conn.cb_minorversion = 1;
- nfsd4_probe_callback(clp, &clp->cl_cb_conn);
+ nfsd4_probe_callback(clp);
}
return new;
}
@@ -1912,7 +1912,8 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
status = nfserr_clid_inuse;
else {
atomic_set(&conf->cl_cb_set, 0);
- nfsd4_probe_callback(conf, &unconf->cl_cb_conn);
+ nfsd4_change_callback(conf, &unconf->cl_cb_conn);
+ nfsd4_probe_callback(conf);
expire_client(unconf);
status = nfs_ok;
@@ -1946,7 +1947,7 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp,
}
move_to_confirmed(unconf);
conf = unconf;
- nfsd4_probe_callback(conf, &conf->cl_cb_conn);
+ nfsd4_probe_callback(conf);
status = nfs_ok;
}
} else if ((!conf || (conf && !same_verf(&conf->cl_confirm, &confirm)))
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index b3bed36..bbc4d58 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -453,7 +453,8 @@ extern int nfs4_in_grace(void);
extern __be32 nfs4_check_open_reclaim(clientid_t *clid);
extern void nfs4_free_stateowner(struct kref *kref);
extern int set_callback_cred(void);
-extern void nfsd4_probe_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);
+extern void nfsd4_probe_callback(struct nfs4_client *clp);
+extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);
extern void nfsd4_do_callback_rpc(struct work_struct *);
extern void nfsd4_cb_recall(struct nfs4_delegation *dp);
extern int nfsd4_create_callback_queue(void);
--
1.7.1
From: J. Bruce Fields <[email protected]>
We need to keep track of which connections are available for use with
the backchannel, which for the forechannel, and which for both.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7e817d1..c470cb7 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -644,7 +644,7 @@ static void nfsd4_conn_lost(struct svc_xpt_user *u)
spin_unlock(&clp->cl_lock);
}
-static struct nfsd4_conn *alloc_conn(struct svc_rqst *rqstp)
+static struct nfsd4_conn *alloc_conn(struct svc_rqst *rqstp, u32 flags)
{
struct nfsd4_conn *conn;
@@ -653,7 +653,7 @@ static struct nfsd4_conn *alloc_conn(struct svc_rqst *rqstp)
return NULL;
svc_xprt_get(rqstp->rq_xprt);
conn->cn_xprt = rqstp->rq_xprt;
- conn->cn_flags = NFS4_CDFC4_FORE;
+ conn->cn_flags = flags;
INIT_LIST_HEAD(&conn->cn_xpt_user.list);
return conn;
}
@@ -682,8 +682,11 @@ static void nfsd4_register_conn(struct nfsd4_conn *conn)
static __be32 nfsd4_new_conn(struct svc_rqst *rqstp, struct nfsd4_session *ses)
{
struct nfsd4_conn *conn;
+ u32 flags = NFS4_CDFC4_FORE;
- conn = alloc_conn(rqstp);
+ if (ses->se_flags & SESSION4_BACK_CHAN)
+ flags |= NFS4_CDFC4_BACK;
+ conn = alloc_conn(rqstp, flags);
if (!conn)
return nfserr_jukebox;
nfsd4_hash_conn(conn, ses);
@@ -1640,7 +1643,7 @@ static void nfsd4_sequence_check_conn(struct svc_rqst *rqstp, struct nfsd4_sessi
if (c)
return;
- new = alloc_conn(rqstp);
+ new = alloc_conn(rqstp, NFS4_CDFC4_FORE);
spin_lock(&clp->cl_lock);
c = __nfsd4_find_conn(rqstp, ses);
--
1.7.1
From: J. Bruce Fields <[email protected]>
Instead of copying the sessionid, use the new cl_cb_session pointer,
which indicates which session we're using for the backchannel.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4callback.c | 9 +++++----
fs/nfsd/nfs4state.c | 4 +---
fs/nfsd/state.h | 1 -
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index a269dbe..78ac779 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -251,6 +251,7 @@ encode_cb_sequence(struct xdr_stream *xdr, struct nfsd4_callback *cb,
struct nfs4_cb_compound_hdr *hdr)
{
__be32 *p;
+ struct nfsd4_session *ses = cb->cb_clp->cl_cb_session;
if (hdr->minorversion == 0)
return;
@@ -258,7 +259,7 @@ encode_cb_sequence(struct xdr_stream *xdr, struct nfsd4_callback *cb,
RESERVE_SPACE(1 + NFS4_MAX_SESSIONID_LEN + 20);
WRITE32(OP_CB_SEQUENCE);
- WRITEMEM(cb->cb_clp->cl_sessionid.data, NFS4_MAX_SESSIONID_LEN);
+ WRITEMEM(ses->se_sessionid.data, NFS4_MAX_SESSIONID_LEN);
WRITE32(cb->cb_clp->cl_cb_seq_nr);
WRITE32(0); /* slotid, always 0 */
WRITE32(0); /* highest slotid always 0 */
@@ -341,6 +342,7 @@ static int
decode_cb_sequence(struct xdr_stream *xdr, struct nfsd4_callback *cb,
struct rpc_rqst *rqstp)
{
+ struct nfsd4_session *ses = cb->cb_clp->cl_cb_session;
struct nfs4_sessionid id;
int status;
u32 dummy;
@@ -362,8 +364,7 @@ decode_cb_sequence(struct xdr_stream *xdr, struct nfsd4_callback *cb,
READ_BUF(NFS4_MAX_SESSIONID_LEN + 16);
memcpy(id.data, p, NFS4_MAX_SESSIONID_LEN);
p += XDR_QUADLEN(NFS4_MAX_SESSIONID_LEN);
- if (memcmp(id.data, cb->cb_clp->cl_sessionid.data,
- NFS4_MAX_SESSIONID_LEN)) {
+ if (memcmp(id.data, ses->se_sessionid.data, NFS4_MAX_SESSIONID_LEN)) {
dprintk("%s Invalid session id\n", __func__);
goto out;
}
@@ -587,7 +588,7 @@ void nfsd4_probe_callback(struct nfs4_client *clp, struct nfs4_cb_conn *conn)
static int nfsd41_cb_setup_sequence(struct nfs4_client *clp,
struct rpc_task *task)
{
- u32 *ptr = (u32 *)clp->cl_sessionid.data;
+ u32 *ptr = (u32 *)clp->cl_cb_session->se_sessionid.data;
int status = 0;
dprintk("%s: %u:%u:%u:%u\n", __func__,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index db5d8c8..c942511 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -753,8 +753,6 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
new->se_client = clp;
gen_sessionid(new);
- memcpy(clp->cl_sessionid.data, new->se_sessionid.data,
- NFS4_MAX_SESSIONID_LEN);
INIT_LIST_HEAD(&new->se_conns);
@@ -1544,7 +1542,7 @@ nfsd4_create_session(struct svc_rqst *rqstp,
if (status)
goto out;
- memcpy(cr_ses->sessionid.data, conf->cl_sessionid.data,
+ memcpy(cr_ses->sessionid.data, conf->cl_cb_session->se_sessionid.data,
NFS4_MAX_SESSIONID_LEN);
cr_ses->seqid = cs_slot->sl_seqid;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 6e63c1d..cdce26a 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -243,7 +243,6 @@ struct nfs4_client {
struct list_head cl_sessions;
struct nfsd4_clid_slot cl_cs_slot; /* create_session slot */
u32 cl_exchange_flags;
- struct nfs4_sessionid cl_sessionid;
/* number of rpc's in progress over an associated session: */
atomic_t cl_refcount;
--
1.7.1
The callback program is allowed to depend on the session which the
callback is going over.
No change in behavior yet, while we still only do callbacks over a
single session for the lifetime of the client.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4callback.c | 1 +
fs/nfsd/nfs4state.c | 2 +-
fs/nfsd/state.h | 4 +++-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 5df9dda..140bb36 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -498,6 +498,7 @@ int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *conn)
return -EINVAL;
if (conn->cb_minorversion) {
args.bc_xprt = conn->cb_xprt;
+ args.prognumber = clp->cl_cb_session->se_cb_prog;
args.protocol = XPRT_TRANSPORT_BC_TCP;
}
/* Create RPC client */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c470cb7..59bc001 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -760,6 +760,7 @@ static struct nfsd4_session *alloc_init_session(struct svc_rqst *rqstp, struct n
new->se_cb_seq_nr = 1;
new->se_flags = cses->flags;
+ new->se_cb_prog = cses->callback_prog;
kref_init(&new->se_ref);
idx = hash_sessionid(&new->se_sessionid);
spin_lock(&client_lock);
@@ -782,7 +783,6 @@ static struct nfsd4_session *alloc_init_session(struct svc_rqst *rqstp, struct n
rpc_copy_addr((struct sockaddr *)&clp->cl_cb_conn.cb_addr, sa);
clp->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
clp->cl_cb_conn.cb_minorversion = 1;
- clp->cl_cb_conn.cb_prog = cses->callback_prog;
nfsd4_probe_callback(clp, &clp->cl_cb_conn);
}
return new;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 7f5b267..b3bed36 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -96,7 +96,8 @@ struct nfs4_cb_conn {
/* SETCLIENTID info */
struct sockaddr_storage cb_addr;
size_t cb_addrlen;
- u32 cb_prog;
+ u32 cb_prog; /* used only in 4.0 case;
+ per-session otherwise */
u32 cb_minorversion;
u32 cb_ident; /* minorversion 0 only */
struct svc_xprt *cb_xprt; /* minorversion 1 only */
@@ -172,6 +173,7 @@ struct nfsd4_session {
struct nfsd4_channel_attrs se_fchannel;
struct nfsd4_channel_attrs se_bchannel;
struct list_head se_conns;
+ u32 se_cb_prog;
u32 se_cb_seq_nr;
struct nfsd4_slot *se_slots[]; /* forward channel slots */
};
--
1.7.1
Have unhash_client_locked() remove client and associated sessions from
global hashes, but delay further dismantling till free_client().
(After unhash_client_locked(), the only remaining references outside the
destroying thread are from any connections which have xpt_user callbacks
registered.)
This will simplify locking on session destruction.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4state.c | 21 +++++++++++++--------
1 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2327a8c..0f2643d 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -883,6 +883,13 @@ static struct nfs4_client *alloc_client(struct xdr_netobj name)
static inline void
free_client(struct nfs4_client *clp)
{
+ while (!list_empty(&clp->cl_sessions)) {
+ struct nfsd4_session *ses;
+ ses = list_entry(clp->cl_sessions.next, struct nfsd4_session,
+ se_perclnt);
+ list_del(&ses->se_perclnt);
+ nfsd4_put_session(ses);
+ }
if (clp->cl_cred.cr_group_info)
put_group_info(clp->cl_cred.cr_group_info);
kfree(clp->cl_principal);
@@ -909,15 +916,12 @@ release_session_client(struct nfsd4_session *session)
static inline void
unhash_client_locked(struct nfs4_client *clp)
{
+ struct nfsd4_session *ses;
+
mark_client_expired(clp);
list_del(&clp->cl_lru);
- while (!list_empty(&clp->cl_sessions)) {
- struct nfsd4_session *ses;
- ses = list_entry(clp->cl_sessions.next, struct nfsd4_session,
- se_perclnt);
- unhash_session(ses);
- nfsd4_put_session(ses);
- }
+ list_for_each_entry(ses, &clp->cl_sessions, se_perclnt)
+ list_del_init(&ses->se_hash);
}
static void
@@ -1031,6 +1035,8 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir,
if (clp == NULL)
return NULL;
+ INIT_LIST_HEAD(&clp->cl_sessions);
+
princ = svc_gss_principal(rqstp);
if (princ) {
clp->cl_principal = kstrdup(princ, GFP_KERNEL);
@@ -1047,7 +1053,6 @@ static struct nfs4_client *create_client(struct xdr_netobj name, char *recdir,
INIT_LIST_HEAD(&clp->cl_strhash);
INIT_LIST_HEAD(&clp->cl_openowners);
INIT_LIST_HEAD(&clp->cl_delegations);
- INIT_LIST_HEAD(&clp->cl_sessions);
INIT_LIST_HEAD(&clp->cl_lru);
spin_lock_init(&clp->cl_lock);
INIT_WORK(&clp->cl_cb_null.cb_work, nfsd4_do_callback_rpc);
--
1.7.1
On 2010-10-27 19:59, J. Bruce Fields wrote:
> On Wed, Oct 27, 2010 at 07:45:49PM +0200, Benny Halevy wrote:
>> On 2010-10-27 19:26, Benny Halevy wrote:
>>> On 2010-10-21 18:20, J. Bruce Fields wrote:
>>>> From: J. Bruce Fields <[email protected]>
>>>>
>>>> The backchannel should be associated with a session, it isn't really
>>>> global to the client.
>>>>
>>>> We do, however, want a pointer global to the client which tracks which
>>>> session we're currently using for client-based callbacks.
>>>>
>>>> This is a first step in that direction; for now, just reshuffling of
>>>> code with no significant change in behavior.
>>>>
>>>> Signed-off-by: J. Bruce Fields <[email protected]>
>>>> ---
>>>> fs/nfsd/nfs4state.c | 29 ++++++++++++++---------------
>>>> fs/nfsd/state.h | 1 +
>>>> 2 files changed, 15 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>>> index 7f12828..db5d8c8 100644
>>>> --- a/fs/nfsd/nfs4state.c
>>>> +++ b/fs/nfsd/nfs4state.c
>>>> @@ -771,6 +771,19 @@ static __be32 alloc_init_session(struct svc_rqst *rqstp, struct nfs4_client *clp
>>>> free_session(&new->se_ref);
>>>> return nfserr_jukebox;
>>>> }
>>>> + if (!clp->cl_cb_session && (cses->flags & SESSION4_BACK_CHAN)) {
>>>> + struct sockaddr *sa = svc_addr(rqstp);
>>>> +
>>>> + clp->cl_cb_session = new;
>>>> + clp->cl_cb_conn.cb_xprt = rqstp->rq_xprt;
>>>> + svc_xprt_get(rqstp->rq_xprt);
>>>> + rpc_copy_addr((struct sockaddr *)&clp->cl_cb_conn.cb_addr, sa);
>>>> + clp->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
>>>> + clp->cl_cb_conn.cb_minorversion = 1;
>>>> + clp->cl_cb_conn.cb_prog = cses->callback_prog;
>>>> + clp->cl_cb_seq_nr = 1;
>>>> + nfsd4_probe_callback(clp, &clp->cl_cb_conn);
>>>> + }
>>>
>>> else
>>> cses->flags &= ~SESSION4_BACK_CHAN;
>>>
>>> We need that for returning the right value in csr_flags
>>
>> Though after "nfsd4: track backchannel connections",
>> we mark the connection correctly.
>> Too late to worry about bisectibility now :)
>
> Erp, yes. We should try to coordinate review better, apologies if I
> haven't been leaving enough time. Let me know if you catch anything
> else.
My bad. Sorry for being so slow in my response.
Benny
>
> --b.