Re: [PATCH RFC v15 06/11] NFSD: Update find_clp_in_name_tree() to handle courtesy clients

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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






[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux