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

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

On Thu, Jun 03, 2021 at 02:14:38PM -0400, Dai Ngo wrote:
> @@ -6875,7 +6947,12 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	case -EAGAIN:		/* conflock holds conflicting lock */
>  		status = nfserr_denied;
>  		dprintk("NFSD: nfsd4_lock: conflicting lock found!\n");
> -		nfs4_set_lock_denied(conflock, &lock->lk_denied);
> +
> +		/* try again if conflict with courtesy client  */
> +		if (nfs4_set_lock_denied(conflock, &lock->lk_denied) == -EAGAIN && !retried) {
> +			retried = true;
> +			goto again;
> +		}

Ugh, apologies, this was my idea, but I just noticed it only handles conflicts
from other NFSv4 clients.  The conflicting lock could just as well come from
NLM or a local process.  So we need cooperation from the common locks.c code.

I'm not sure what to suggest....

Maybe something like:

@@ -1159,6 +1159,7 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request,
        }
 
        percpu_down_read(&file_rwsem);
+retry:
        spin_lock(&ctx->flc_lock);
        /*
         * New lock request. Walk all POSIX locks and look for conflicts. If
@@ -1169,6 +1170,11 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request,
                list_for_each_entry(fl, &ctx->flc_posix, fl_list) {
                        if (!posix_locks_conflict(request, fl))
                                continue;
+                       if (fl->fl_lops->fl_expire_lock(fl, 1)) {
+                               spin_unlock(&ctx->flc_lock);
+                               fl->fl_lops->fl_expire_locks(fl, 0);
+                               goto retry;
+                       }
                        if (conflock)
                                locks_copy_conflock(conflock, fl);
                        error = -EAGAIN;


where ->fl_expire_lock is a new lock callback with second argument "check"
where:

	check = 1 means: just check whether this lock could be freed
	check = 0 means: go ahead and free this lock if you can

--b.