Re: [PATCH RFC v9 2/2] nfsd: Initial implementation of NFSv4 Courteous Server

["J. Bruce Fields" <bfields@fieldses.org>]

On Thu, Jan 13, 2022 at 12:51:57AM -0800, dai.ngo@oracle.com 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(&lt, 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.