On Wed, Apr 27, 2022 at 01:52:47AM -0700, Dai Ngo wrote:
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 234e852fcdfa..216bd77a8764 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;
> @@ -152,6 +154,7 @@ static __be32 get_client_locked(struct nfs4_client *clp)
> if (is_client_expired(clp))
> return nfserr_expired;
> atomic_inc(&clp->cl_rpc_users);
> + clp->cl_state = NFSD4_ACTIVE;
> return nfs_ok;
> }
>
> @@ -172,6 +175,7 @@ renew_client_locked(struct nfs4_client *clp)
>
> list_move_tail(&clp->cl_lru, &nn->client_lru);
> clp->cl_time = ktime_get_boottime_seconds();
> + clp->cl_state = NFSD4_ACTIVE;
> }
We shouldn't need that assignment in both places.
The laundromat really shouldn't let a client go to COURTESY while there
are rpc's in process for that client. So, let's just add that check to
the laundromat (see below), and then the assignment in
renew_client_locked becomes unnecessary.
> +static void
> +nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
> + struct laundry_time *lt)
> +{
> + struct list_head *pos, *next;
> + struct nfs4_client *clp;
> + bool cour;
> +
> + INIT_LIST_HEAD(reaplist);
> + spin_lock(&nn->client_lock);
> + list_for_each_safe(pos, next, &nn->client_lru) {
> + clp = list_entry(pos, struct nfs4_client, cl_lru);
> + if (clp->cl_state == NFSD4_EXPIRABLE)
> + goto exp_client;
> + if (!state_expired(lt, clp->cl_time))
> + break;
> + if (!client_has_state_tmp(clp))
> + goto exp_client;
> + cour = (clp->cl_state == NFSD4_COURTESY);
> + if (cour && ktime_get_boottime_seconds() >=
> + (clp->cl_time + NFSD_COURTESY_CLIENT_TIMEOUT)) {
> + goto exp_client;
> + }
> + if (nfs4_anylock_blockers(clp)) {
> +exp_client:
> + if (mark_client_expired_locked(clp))
> + continue;
> + list_add(&clp->cl_lru, reaplist);
> + continue;
> + }
> + if (!cour)
> + cmpxchg(&clp->cl_state, NFSD4_ACTIVE, NFSD4_COURTESY);
So, as in mark_client_expired_locked, we should only be doing this if
clp_cl_rpc_users is 0.
Also, it should just be a simple assignment, the cmpxchg isn't necessary
here.
--b.
