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.
Signed-off-by: Dai Ngo <[email protected]>
---
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
> On Jan 28, 2022, at 2:39 PM, Dai Ngo <[email protected]> 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".
> Signed-off-by: Dai Ngo <[email protected]>
> ---
> 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
On 2/3/22 10:41 AM, Chuck Lever III wrote:
>
>> On Jan 28, 2022, at 2:39 PM, Dai Ngo <[email protected]> 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 <[email protected]>
>> ---
>> 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
>
>
>
On 2/3/22 2:50 PM, Jeff Layton wrote:
> On Fri, 2022-01-28 at 11:39 -0800, Dai Ngo 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.
>>
>> Signed-off-by: Dai Ngo <[email protected]>
>> ---
>> 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;
> The naming of this op is a little misleading. We already know that there
> is a lock confict in this case. The question is whether it's resolvable
> by expiring a tardy client. That said, I don't have a better name to
> suggest at the moment.
I will leave it as is for now.
>
> A comment about what this function actually tells us would be nice here.
will do in v11.
Thanks,
-Dai
>
>> 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 {
> Acked-by: Jeff Layton <[email protected]>
>
On Fri, 2022-01-28 at 11:39 -0800, Dai Ngo 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.
>
> Signed-off-by: Dai Ngo <[email protected]>
> ---
> 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;
The naming of this op is a little misleading. We already know that there
is a lock confict in this case. The question is whether it's resolvable
by expiring a tardy client. That said, I don't have a better name to
suggest at the moment.
A comment about what this function actually tells us would be nice here.
> 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 {
Acked-by: Jeff Layton <[email protected]>