Re: [PATCH RFC 1/3] fs/lock: add new callback, lm_expire_lock, to lock_manager_operations

[dai.ngo@xxxxxxxxxx]

On 2/3/22 10:41 AM, Chuck Lever III wrote:
> > On Jan 28, 2022, at 2:39 PM, Dai Ngo <dai.ngo@xxxxxxxxxx> wrote:
> >
> > Add new callback, lm_expire_lock, to lock_manager_operations to allow
> > the lock manager to take appropriate action to resolve the lock conflict
> > if possible. The callback takes 1 argument, the file_lock of the blocker
> > and returns true if the conflict was resolved else returns false. Note
> > that the lock manager has to be able to resolve the conflict while
> > the spinlock flc_lock is held.
> >
> > Lock manager, such as NFSv4 courteous server, uses this callback to
> > resolve conflict by destroying lock owner, or the NFSv4 courtesy client
> > (client that has expired but allowed to maintains its states) that owns
> > the lock.
> This change is nice and simple now. The only issue is that the
> short and long patch descriptions need to be updated to replace
> "lm_expire_lock" with "lm_lock_conflict".

Fix in v11.

Thanks,
-Dai

> > Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx>
> > ---
> > Documentation/filesystems/locking.rst |  2 ++
> > fs/locks.c                            | 14 ++++++++++----
> > include/linux/fs.h                    |  1 +
> > 3 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
> > index d36fe79167b3..57ce0fbc8ab1 100644
> > --- a/Documentation/filesystems/locking.rst
> > +++ b/Documentation/filesystems/locking.rst
> > @@ -439,6 +439,7 @@ prototypes::
> > 	void (*lm_break)(struct file_lock *); /* break_lease callback */
> > 	int (*lm_change)(struct file_lock **, int);
> > 	bool (*lm_breaker_owns_lease)(struct file_lock *);
> > +	bool (*lm_lock_conflict)(struct file_lock *);
> >
> > locking rules:
> >
> > @@ -450,6 +451,7 @@ lm_grant:		no		no			no
> > lm_break:		yes		no			no
> > lm_change		yes		no			no
> > lm_breaker_owns_lease:	no		no			no
> > +lm_lock_conflict:       no		no			no
> > ======================	=============	=================	=========
> >
> > buffer_head
> > diff --git a/fs/locks.c b/fs/locks.c
> > index 0fca9d680978..052b42cc7f25 100644
> > --- a/fs/locks.c
> > +++ b/fs/locks.c
> > @@ -853,10 +853,13 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
> >
> > 	spin_lock(&ctx->flc_lock);
> > 	list_for_each_entry(cfl, &ctx->flc_posix, fl_list) {
> > -		if (posix_locks_conflict(fl, cfl)) {
> > -			locks_copy_conflock(fl, cfl);
> > -			goto out;
> > -		}
> > +		if (!posix_locks_conflict(fl, cfl))
> > +			continue;
> > +		if (cfl->fl_lmops && cfl->fl_lmops->lm_lock_conflict &&
> > +			!cfl->fl_lmops->lm_lock_conflict(cfl))
> > +			continue;
> > +		locks_copy_conflock(fl, cfl);
> > +		goto out;
> > 	}
> > 	fl->fl_type = F_UNLCK;
> > out:
> > @@ -1059,6 +1062,9 @@ 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_lmops && fl->fl_lmops->lm_lock_conflict &&
> > +				!fl->fl_lmops->lm_lock_conflict(fl))
> > +				continue;
> > 			if (conflock)
> > 				locks_copy_conflock(conflock, fl);
> > 			error = -EAGAIN;
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index bbf812ce89a8..21cb7afe2d63 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1068,6 +1068,7 @@ struct lock_manager_operations {
> > 	int (*lm_change)(struct file_lock *, int, struct list_head *);
> > 	void (*lm_setup)(struct file_lock *, void **);
> > 	bool (*lm_breaker_owns_lease)(struct file_lock *);
> > +	bool (*lm_lock_conflict)(struct file_lock *cfl);
> > };
> >
> > struct lock_manager {
> > --
> > 2.9.5
> --
> Chuck Lever