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

[dai.ngo@oracle.com]

On 1/13/22 7:42 AM, J. Bruce Fields wrote:
> 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?

After we drop the lock, if the client now has no state then it just
remains in memory until the courtesy client timeout expires then we
get rid of it.

For the race condition of lock conflict, we use the client->cl_cs_lock
to synchronize the laundromat and and lm_lock_conflict/nfsd4_fl_lock_conflict.
If the locking thread acquires the cl_cs_lock before the laundromat does
then the thread will be blocked and laundromat detects there is blocker
and expires the client. If the laundromat acquires the cl_cs_lock first
then NFSD4_COURTESY_CLIENT is set and nfsd4_fl_lock_conflict detects
this flag and sets the client to NFSD4_DESTROY_COURTESY_CLIENT.

-Dai