On 4/27/22 9:12 AM, J. Bruce Fields wrote:
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.
yes, I should have renamed the function to something like 'is_client_active()'.
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;
I'll change to this:
static inline bool try_to_expire_client(struct nfs4_client *clp)
{
bool ret;
cmpxchg(&clp->cl_state, NFSD4_COURTESY, NFSD4_EXPIRABLE);
return clp->cl_state == NFSD4_EXPIRABLE;
}
and its caller:
if (try_to_expire_client(clp)) {
nn = net_generic(clp->net, nfsd_net_id);
mod_delayed_work(laundry_wq, &nn->laundromat_work, 0);
}
-Dai
--b.
