On Thu, Jan 13, 2022 at 12:51:57AM -0800, dai.ngo@xxxxxxxxxx wrote:
>
> On 1/12/22 11:40 AM, J. Bruce Fields wrote:
> >On Mon, Jan 10, 2022 at 10:50:53AM -0800, Dai Ngo wrote:
> >>+ }
> >>+ if (!state_expired(<, clp->cl_time)) {
> >>+ spin_unlock(&clp->cl_cs_lock);
> >> break;
> >>+ }
> >>+ id = 0;
> >>+ spin_lock(&clp->cl_lock);
> >>+ stid = idr_get_next(&clp->cl_stateids, &id);
> >>+ if (stid && !nfs4_anylock_conflict(clp)) {
> >>+ /* client still has states */
> >I'm a little confused by that comment. I think what you just checked is
> >that the client has some state, *and* nobody is waiting for one of its
> >locks. For me, that comment just conufses things.
>
> will remove.
>
> >
> >>+ spin_unlock(&clp->cl_lock);
> >Is nn->client_lock enough to guarantee that the condition you just
> >checked still holds? (Honest question, I'm not sure.)
>
> nfs4_anylock_conflict_locked scans cl_ownerstr_hashtbl which is protected
> by the cl_lock.
That doesn't answer the question. Which, I confess, was muddled (I
should have said "clp->cl_cs_lock", not "nn->client_lock".)
Let me try it a different way. You just checked that the client has
some state, and that nobody is waiting for one of its locks.
After you drop the cl_lock, how do you know that both of those things
are still true?
--b.
