On 3/9/22 2:08 PM, Chuck Lever III wrote:
On Mar 4, 2022, at 7:37 PM, Dai Ngo <dai.ngo@xxxxxxxxxx> wrote: Update find_clp_in_name_tree: . skip client with CLIENT_EXPIRED flag; discarded courtesy client. . if courtesy client was found then clear CLIENT_COURTESY and set CLIENT_RECONNECTED so callers can take appropriate action. Update find_confirmed_client_by_name to discard the courtesy client; set CLIENT_EXPIRED. Update nfsd4_setclientid to expire the confirmed courtesy client to prevent multiple confirmed clients with the same name on the the conf_id_hashtbl list. Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx> --- fs/nfsd/nfs4state.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 3 deletions(-) diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index b16f689f34c3..f42d72a8f5ca 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -1929,6 +1929,34 @@ __find_in_sessionid_hashtbl(struct nfs4_sessionid *sessionid, struct net *net) return NULL; } +static void +nfsd4_discard_courtesy_clnt(struct nfs4_client *clp) +{ + spin_lock(&clp->cl_cs_lock); + set_bit(NFSD4_CLIENT_EXPIRED, &clp->cl_flags); + spin_unlock(&clp->cl_cs_lock); +} + +static bool +nfs4_is_courtesy_client_expired(struct nfs4_client *clp) +{ + clear_bit(NFSD4_CLIENT_RECONNECTED, &clp->cl_flags); + /* need to sync with thread resolving lock/deleg conflict */ + spin_lock(&clp->cl_cs_lock); + if (test_bit(NFSD4_CLIENT_EXPIRED, &clp->cl_flags)) { + spin_unlock(&clp->cl_cs_lock); + return true; + } + /* + * clear CLIENT_COURTESY flag to prevent it from being + * destroyed by thread trying to resolve conflicts. + */ + if (test_and_clear_bit(NFSD4_CLIENT_COURTESY, &clp->cl_flags)) + set_bit(NFSD4_CLIENT_RECONNECTED, &clp->cl_flags); + spin_unlock(&clp->cl_cs_lock); + return false; +} + static struct nfsd4_session * find_in_sessionid_hashtbl(struct nfs4_sessionid *sessionid, struct net *net, __be32 *ret) @@ -2834,8 +2862,11 @@ find_clp_in_name_tree(struct xdr_netobj *name, struct rb_root *root) node = node->rb_left; else if (cmp < 0) node = node->rb_right; - else + else { + if (nfs4_is_courtesy_client_expired(clp)) + return NULL; return clp; + } } return NULL; } @@ -2914,8 +2945,15 @@ static bool clp_used_exchangeid(struct nfs4_client *clp) static struct nfs4_client * find_confirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn) { + struct nfs4_client *clp; + lockdep_assert_held(&nn->client_lock); - return find_clp_in_name_tree(name, &nn->conf_name_tree); + clp = find_clp_in_name_tree(name, &nn->conf_name_tree); + if (clp && test_bit(NFSD4_CLIENT_RECONNECTED, &clp->cl_flags)) { + nfsd4_discard_courtesy_clnt(clp); + clp = NULL; + } + return clp; }2983 static struct nfs4_client * 2984 find_unconfirmed_client_by_name(struct xdr_netobj *name, struct nfsd_net *nn) 2985 { 2986 lockdep_assert_held(&nn->client_lock); 2987 return find_clp_in_name_tree(name, &nn->unconf_name_tree); 2988 } Notice the difference: find_confirmed() does find_clp_in_name_tree(&nn->conf_name_tree); ^^^^ find_unconfirmed() does find_clp_in_name_tree(&nn->unconf_name_tree); ^^^^^^ I don't think we will ever find a client in unconf_name_tree that has CLIENT_RECONNECTED set, will we?
No.
So it seems to me that you can safely move the CLIENT_RECONNECTED test into find_clp_in_name_tree() in all cases, maybe?
nfsd4_setclientid needs to treat the courtesy client a little differently from find_[un]confirmed_client_by_name. It has to expire the courtesy client immediately to prevent multiple confirmed clients with the same name on the the conf_id_hashtbl list which causes problem for the subsequent setclientid_confirm. -Dai
Or think about it this way: is it possible for an unconfirmed client to become a courtesy client? I don't think it is.@@ -4032,12 +4070,19 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, struct nfs4_client *unconf = NULL; __be32 status; struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id); + struct nfs4_client *cclient = NULL; new = create_client(clname, rqstp, &clverifier); if (new == NULL) return nfserr_jukebox; spin_lock(&nn->client_lock); - conf = find_confirmed_client_by_name(&clname, nn); + /* find confirmed client by name */ + conf = find_clp_in_name_tree(&clname, &nn->conf_name_tree); + if (conf && test_bit(NFSD4_CLIENT_RECONNECTED, &conf->cl_flags)) { + cclient = conf; + conf = NULL; + } + if (conf && client_has_state(conf)) { status = nfserr_clid_inuse; if (clp_used_exchangeid(conf)) @@ -4068,7 +4113,11 @@ nfsd4_setclientid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, new = NULL; status = nfs_ok; out: + if (cclient) + unhash_client_locked(cclient); spin_unlock(&nn->client_lock); + if (cclient) + expire_client(cclient); if (new) free_client(new); if (unconf) { -- 2.9.5-- Chuck Lever
