On Fri, Apr 29, 2022 at 03:55:10PM -0400, J. Bruce Fields wrote:
> On Fri, Apr 29, 2022 at 10:21:21AM -0700, dai.ngo@xxxxxxxxxx wrote:
> >
> > On 4/29/22 7:55 AM, J. Bruce Fields wrote:
> > >On Thu, Apr 28, 2022 at 12:06:29AM -0700, Dai Ngo wrote:
> > >>+static bool client_has_state_tmp(struct nfs4_client *clp)
> > >Why the "_tmp"?
> > >
> > >>+{
> > >>+ if (!list_empty(&clp->cl_delegations) &&
> > >>+ !client_has_openowners(clp) &&
> > >>+ list_empty(&clp->async_copies))
> > >I would have expected
> > >
> > > if (!list_empty(&clp->cl_delegations) ||
> > > client_has_openowners(clp) ||
> > > !list_empty(&clp->async_copies))
> >
> > In patch 1, we want to allow *only* clients with non-conflict delegation
> > to be in COURTESY state, not with opens and locks. So for that, we can not
> > use the existing client_has_state (until patch 6), so I just created
> > client_has_state_tmp for it.
>
> Got it, so, I recommend just moving this logic into
> nfs4_anylock_blockers instead, and replacing the call to
> client_has_state_tmp() with a call to client_has_state().
>
> The logic of nfs4_anylock_blockers() is then basically "return true if anyone
> might be waiting on this client; and if the client has some class of
> state that we don't handle yet, just assume it might have someone
> waiting on it."
And, yeah, the end result is probably the same, but this would just make
the patches easier to read.
--b.
