On Wed, Apr 27, 2022 at 01:52:47AM -0700, Dai Ngo wrote:
> + if (!try_to_expire_client(clp)) {
> + nn = net_generic(clp->net, nfsd_net_id);
> + mod_delayed_work(laundry_wq, &nn->laundromat_work, 0);
> + }
...
> +static inline bool try_to_expire_client(struct nfs4_client *clp)
> +{
> + bool ret;
> +
> + ret = NFSD4_ACTIVE ==
> + cmpxchg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE);
> + return ret;
> +}
Hm, this feels a little backwards to me. I'd expect
"try_to_expire_client" to return true on success. Maybe call it
"client_is_expirable()" to make that clear? Or stick with
"try_to_expire_client and make it return 0/-EAGAIN.
If "NFSD4_ACTIVE != cmpxchg(...)" is too confusing, I don't think we
really even need the atomic return of the previous value, I think it
would be OK just to do:
cmpschg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE);
return clp->cl_state == NFSD4_EXPIRABLE;
--b.
