On 4/25/22 11:51 AM, J. Bruce Fields wrote:
On Sat, Apr 23, 2022 at 11:44:09AM -0700, Dai Ngo wrote:
This patch provides courteous server support for delegation only.
Only expired client with delegation but no conflict and no open
or lock state is allowed to be in COURTESY state.
Delegation conflict with COURTESY client is resolved by setting it
to EXPIRABLE, queue work for the laundromat and return delay to
caller. Conflict is resolved when the laudromat runs and expires
the EXIRABLE client while the NFS client retries the OPEN request.
Local thread request that gets conflict is doing the retry in
__break_lease.
Client in COURTESY state is allowed to reconnect and continues to
have access to its state while client in EXPIRABLE state is not.
To prevent 2 clients with the same name are on the conf_name_tree,
nfsd4_setclientid is modified to expire confirmed client in EXPIRABLE
state.
The spinlock cl_cs_lock is used to handle race conditions between
conflict resolver, nfsd_break_deleg_cb, and the COURTESY client
doing reconnect.
find_in_sessionid_hashtbl, find_confirmed_client_by_name and
find_confirmed_client are modified to activate COURTESY client by
setting it to ACTIVE state and skip client in EXPIRABLE state.
Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx>
---
fs/nfsd/nfs4state.c | 129 +++++++++++++++++++++++++++++++++++++++++++++-------
fs/nfsd/nfsd.h | 1 +
fs/nfsd/state.h | 63 +++++++++++++++++++++++++
3 files changed, 177 insertions(+), 16 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 234e852fcdfa..fea5e24e7d94 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -125,6 +125,8 @@ static void free_session(struct nfsd4_session *);
static const struct nfsd4_callback_ops nfsd4_cb_recall_ops;
static const struct nfsd4_callback_ops nfsd4_cb_notify_lock_ops;
+static struct workqueue_struct *laundry_wq;
+
static bool is_session_dead(struct nfsd4_session *ses)
{
return ses->se_flags & NFS4_SESSION_DEAD;
@@ -690,6 +692,14 @@ static unsigned int file_hashval(struct svc_fh *fh)
static struct hlist_head file_hashtbl[FILE_HASH_SIZE];
+static inline void
+run_laundromat(struct nfsd_net *nn, bool wait)
+{
+ mod_delayed_work(laundry_wq, &nn->laundromat_work, 0);
+ if (wait)
+ flush_workqueue(laundry_wq);
+}
Let's keep those two things separate. The "wait" argument isn't
self-documenting when reading the calling code.
we can use enum to spell out the intention.
And the
mod_delayed_work call isn't always needed before flush_workqueue.
ok. I'll keep them separate.
+
static void
__nfs4_file_get_access(struct nfs4_file *fp, u32 access)
{
@@ -1939,6 +1949,11 @@ find_in_sessionid_hashtbl(struct nfs4_sessionid *sessionid, struct net *net,
session = __find_in_sessionid_hashtbl(sessionid, net);
if (!session)
goto out;
+ if (!try_to_activate_client(session->se_client)) {
+ /* client is EXPIRABLE */
+ session = NULL;
+ goto out;
No, we definitely don't want to do this. As I said before, an
"expirable client" should be treated in every way exactly like any
regular active client. Literally the only difference is that the
laundromat can try to expire it.
Do you mean leave the state as EXPIRABLE but allow the callers
to use the client as an ACTIVE client:
static inline bool try_to_expire_client(struct nfs4_client *clp)
{
bool ret;
spin_lock(&clp->cl_cs_lock);
if (clp->cl_state == NFSD4_EXPIRABLE) {
spin_unlock(&clp->cl_cs_lock);
return false; <<<====== was true
}
ret = NFSD4_COURTESY ==
cmpxchg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE);
spin_unlock(&clp->cl_cs_lock);
return ret;
}
-Dai
And then all this code becomes unnecessary:
@@ -702,4 +727,42 @@ extern void nfsd4_client_record_remove(struct nfs4_client *clp);
extern int nfsd4_client_record_check(struct nfs4_client *clp);
extern void nfsd4_record_grace_done(struct nfsd_net *nn);
+static inline bool try_to_expire_client(struct nfs4_client *clp)
+{
+ bool ret;
+
+ spin_lock(&clp->cl_cs_lock);
+ if (clp->cl_state == NFSD4_EXPIRABLE) {
+ spin_unlock(&clp->cl_cs_lock);
+ return true;
+ }
+ ret = NFSD4_COURTESY ==
+ cmpxchg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE);
+ spin_unlock(&clp->cl_cs_lock);
+ return ret;
+}
+
+static inline bool try_to_activate_client(struct nfs4_client *clp)
+{
+ bool ret;
+ struct nfsd_net *nn = net_generic(clp->net, nfsd_net_id);
+
+ /* sync with laundromat */
+ lockdep_assert_held(&nn->client_lock);
+
+ /* sync with try_to_expire_client */
+ spin_lock(&clp->cl_cs_lock);
+ if (clp->cl_state == NFSD4_ACTIVE) {
+ spin_unlock(&clp->cl_cs_lock);
+ return true;
+ }
+ if (clp->cl_state == NFSD4_EXPIRABLE) {
+ spin_unlock(&clp->cl_cs_lock);
+ return false;
+ }
+ ret = NFSD4_COURTESY ==
+ cmpxchg(&clp->cl_state, NFSD4_COURTESY, NFSD4_ACTIVE);
+ spin_unlock(&clp->cl_cs_lock);
+ return ret;
+}
--b.
