Message ID | 1651526367-1522-6-git-send-email-dai.ngo@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | NFSD: Initial implementation of NFSv4 Courteous Server | expand |
> On May 2, 2022, at 5:19 PM, Dai Ngo <dai.ngo@oracle.com> wrote: > > Add 2 new callbacks, lm_lock_expirable and lm_expire_lock, to > lock_manager_operations to allow the lock manager to take appropriate > action to resolve the lock conflict if possible. > > A new field, lm_mod_owner, is also added to lock_manager_operations. > The lm_mod_owner is used by the fs/lock code to make sure the lock > manager module such as nfsd, is not freed while lock conflict is being > resolved. > > lm_lock_expirable checks and returns true to indicate that the lock > conflict can be resolved else return false. This callback must be > called with the flc_lock held so it can not block. > > lm_expire_lock is called to resolve the lock conflict if the returned > value from lm_lock_expirable is true. This callback is called without > the flc_lock held since it's allowed to block. Upon returning from > this callback, the lock conflict should be resolved and the caller is > expected to restart the conflict check from the beginnning of the list. > > 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. > > Reviewed-by: J. Bruce Fields <bfields@fieldses.org> > Signed-off-by: Dai Ngo <dai.ngo@oracle.com> Jeff, May I add your Reviewed-by or Acked-by to 4/7 and 5/7 in this series? > --- > Documentation/filesystems/locking.rst | 4 ++++ > fs/locks.c | 33 ++++++++++++++++++++++++++++++--- > include/linux/fs.h | 3 +++ > 3 files changed, 37 insertions(+), 3 deletions(-) > > diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst > index c26d854275a0..0997a258361a 100644 > --- a/Documentation/filesystems/locking.rst > +++ b/Documentation/filesystems/locking.rst > @@ -428,6 +428,8 @@ 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_expirable)(struct file_lock *); > + void (*lm_expire_lock)(void); > > locking rules: > > @@ -439,6 +441,8 @@ lm_grant: no no no > lm_break: yes no no > lm_change yes no no > lm_breaker_owns_lease: yes no no > +lm_lock_expirable yes no no > +lm_expire_lock no no yes > ====================== ============= ================= ========= > > buffer_head > diff --git a/fs/locks.c b/fs/locks.c > index c369841ef7d1..ca28e0e50e56 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -902,6 +902,8 @@ posix_test_lock(struct file *filp, struct file_lock *fl) > struct file_lock *cfl; > struct file_lock_context *ctx; > struct inode *inode = locks_inode(filp); > + void *owner; > + void (*func)(void); > > ctx = smp_load_acquire(&inode->i_flctx); > if (!ctx || list_empty_careful(&ctx->flc_posix)) { > @@ -909,12 +911,23 @@ posix_test_lock(struct file *filp, struct file_lock *fl) > return; > } > > +retry: > 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_expirable > + && (*cfl->fl_lmops->lm_lock_expirable)(cfl)) { > + owner = cfl->fl_lmops->lm_mod_owner; > + func = cfl->fl_lmops->lm_expire_lock; > + __module_get(owner); > + spin_unlock(&ctx->flc_lock); > + (*func)(); > + module_put(owner); > + goto retry; > } > + locks_copy_conflock(fl, cfl); > + goto out; > } > fl->fl_type = F_UNLCK; > out: > @@ -1088,6 +1101,8 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request, > int error; > bool added = false; > LIST_HEAD(dispose); > + void *owner; > + void (*func)(void); > > ctx = locks_get_lock_context(inode, request->fl_type); > if (!ctx) > @@ -1106,6 +1121,7 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request, > new_fl2 = locks_alloc_lock(); > } > > +retry: > percpu_down_read(&file_rwsem); > spin_lock(&ctx->flc_lock); > /* > @@ -1117,6 +1133,17 @@ 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_expirable > + && (*fl->fl_lmops->lm_lock_expirable)(fl)) { > + owner = fl->fl_lmops->lm_mod_owner; > + func = fl->fl_lmops->lm_expire_lock; > + __module_get(owner); > + spin_unlock(&ctx->flc_lock); > + percpu_up_read(&file_rwsem); > + (*func)(); > + module_put(owner); > + goto retry; > + } > if (conflock) > locks_copy_conflock(conflock, fl); > error = -EAGAIN; > diff --git a/include/linux/fs.h b/include/linux/fs.h > index b8ed7f974fb4..aa6c1bbdb8c4 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -1029,6 +1029,7 @@ struct file_lock_operations { > }; > > struct lock_manager_operations { > + void *lm_mod_owner; > fl_owner_t (*lm_get_owner)(fl_owner_t); > void (*lm_put_owner)(fl_owner_t); > void (*lm_notify)(struct file_lock *); /* unblock callback */ > @@ -1037,6 +1038,8 @@ 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_expirable)(struct file_lock *cfl); > + void (*lm_expire_lock)(void); > }; > > struct lock_manager { > -- > 2.9.5 > -- Chuck Lever
On Mon, 2022-05-02 at 14:19 -0700, Dai Ngo wrote: > Add 2 new callbacks, lm_lock_expirable and lm_expire_lock, to > lock_manager_operations to allow the lock manager to take appropriate > action to resolve the lock conflict if possible. > > A new field, lm_mod_owner, is also added to lock_manager_operations. > The lm_mod_owner is used by the fs/lock code to make sure the lock > manager module such as nfsd, is not freed while lock conflict is being > resolved. > > lm_lock_expirable checks and returns true to indicate that the lock > conflict can be resolved else return false. This callback must be > called with the flc_lock held so it can not block. > > lm_expire_lock is called to resolve the lock conflict if the returned > value from lm_lock_expirable is true. This callback is called without > the flc_lock held since it's allowed to block. Upon returning from > this callback, the lock conflict should be resolved and the caller is > expected to restart the conflict check from the beginnning of the list. > > 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. > > Reviewed-by: J. Bruce Fields <bfields@fieldses.org> > Signed-off-by: Dai Ngo <dai.ngo@oracle.com> > --- > Documentation/filesystems/locking.rst | 4 ++++ > fs/locks.c | 33 ++++++++++++++++++++++++++++++--- > include/linux/fs.h | 3 +++ > 3 files changed, 37 insertions(+), 3 deletions(-) > > diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst > index c26d854275a0..0997a258361a 100644 > --- a/Documentation/filesystems/locking.rst > +++ b/Documentation/filesystems/locking.rst > @@ -428,6 +428,8 @@ 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_expirable)(struct file_lock *); > + void (*lm_expire_lock)(void); > > locking rules: > > @@ -439,6 +441,8 @@ lm_grant: no no no > lm_break: yes no no > lm_change yes no no > lm_breaker_owns_lease: yes no no > +lm_lock_expirable yes no no > +lm_expire_lock no no yes > ====================== ============= ================= ========= > > buffer_head > diff --git a/fs/locks.c b/fs/locks.c > index c369841ef7d1..ca28e0e50e56 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -902,6 +902,8 @@ posix_test_lock(struct file *filp, struct file_lock *fl) > struct file_lock *cfl; > struct file_lock_context *ctx; > struct inode *inode = locks_inode(filp); > + void *owner; > + void (*func)(void); > > ctx = smp_load_acquire(&inode->i_flctx); > if (!ctx || list_empty_careful(&ctx->flc_posix)) { > @@ -909,12 +911,23 @@ posix_test_lock(struct file *filp, struct file_lock *fl) > return; > } > > +retry: > 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_expirable > + && (*cfl->fl_lmops->lm_lock_expirable)(cfl)) { > + owner = cfl->fl_lmops->lm_mod_owner; > + func = cfl->fl_lmops->lm_expire_lock; > + __module_get(owner); > + spin_unlock(&ctx->flc_lock); > + (*func)(); > + module_put(owner); > + goto retry; > } > + locks_copy_conflock(fl, cfl); > + goto out; > } > fl->fl_type = F_UNLCK; > out: > @@ -1088,6 +1101,8 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request, > int error; > bool added = false; > LIST_HEAD(dispose); > + void *owner; > + void (*func)(void); > > ctx = locks_get_lock_context(inode, request->fl_type); > if (!ctx) > @@ -1106,6 +1121,7 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request, > new_fl2 = locks_alloc_lock(); > } > > +retry: > percpu_down_read(&file_rwsem); > spin_lock(&ctx->flc_lock); > /* > @@ -1117,6 +1133,17 @@ 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_expirable > + && (*fl->fl_lmops->lm_lock_expirable)(fl)) { > + owner = fl->fl_lmops->lm_mod_owner; > + func = fl->fl_lmops->lm_expire_lock; > + __module_get(owner); > + spin_unlock(&ctx->flc_lock); > + percpu_up_read(&file_rwsem); > + (*func)(); > + module_put(owner); > + goto retry; > + } > if (conflock) > locks_copy_conflock(conflock, fl); > error = -EAGAIN; > diff --git a/include/linux/fs.h b/include/linux/fs.h > index b8ed7f974fb4..aa6c1bbdb8c4 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -1029,6 +1029,7 @@ struct file_lock_operations { > }; > > struct lock_manager_operations { > + void *lm_mod_owner; > fl_owner_t (*lm_get_owner)(fl_owner_t); > void (*lm_put_owner)(fl_owner_t); > void (*lm_notify)(struct file_lock *); /* unblock callback */ > @@ -1037,6 +1038,8 @@ 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_expirable)(struct file_lock *cfl); > + void (*lm_expire_lock)(void); > }; > > struct lock_manager { Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst index c26d854275a0..0997a258361a 100644 --- a/Documentation/filesystems/locking.rst +++ b/Documentation/filesystems/locking.rst @@ -428,6 +428,8 @@ 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_expirable)(struct file_lock *); + void (*lm_expire_lock)(void); locking rules: @@ -439,6 +441,8 @@ lm_grant: no no no lm_break: yes no no lm_change yes no no lm_breaker_owns_lease: yes no no +lm_lock_expirable yes no no +lm_expire_lock no no yes ====================== ============= ================= ========= buffer_head diff --git a/fs/locks.c b/fs/locks.c index c369841ef7d1..ca28e0e50e56 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -902,6 +902,8 @@ posix_test_lock(struct file *filp, struct file_lock *fl) struct file_lock *cfl; struct file_lock_context *ctx; struct inode *inode = locks_inode(filp); + void *owner; + void (*func)(void); ctx = smp_load_acquire(&inode->i_flctx); if (!ctx || list_empty_careful(&ctx->flc_posix)) { @@ -909,12 +911,23 @@ posix_test_lock(struct file *filp, struct file_lock *fl) return; } +retry: 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_expirable + && (*cfl->fl_lmops->lm_lock_expirable)(cfl)) { + owner = cfl->fl_lmops->lm_mod_owner; + func = cfl->fl_lmops->lm_expire_lock; + __module_get(owner); + spin_unlock(&ctx->flc_lock); + (*func)(); + module_put(owner); + goto retry; } + locks_copy_conflock(fl, cfl); + goto out; } fl->fl_type = F_UNLCK; out: @@ -1088,6 +1101,8 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request, int error; bool added = false; LIST_HEAD(dispose); + void *owner; + void (*func)(void); ctx = locks_get_lock_context(inode, request->fl_type); if (!ctx) @@ -1106,6 +1121,7 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request, new_fl2 = locks_alloc_lock(); } +retry: percpu_down_read(&file_rwsem); spin_lock(&ctx->flc_lock); /* @@ -1117,6 +1133,17 @@ 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_expirable + && (*fl->fl_lmops->lm_lock_expirable)(fl)) { + owner = fl->fl_lmops->lm_mod_owner; + func = fl->fl_lmops->lm_expire_lock; + __module_get(owner); + spin_unlock(&ctx->flc_lock); + percpu_up_read(&file_rwsem); + (*func)(); + module_put(owner); + goto retry; + } if (conflock) locks_copy_conflock(conflock, fl); error = -EAGAIN; diff --git a/include/linux/fs.h b/include/linux/fs.h index b8ed7f974fb4..aa6c1bbdb8c4 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1029,6 +1029,7 @@ struct file_lock_operations { }; struct lock_manager_operations { + void *lm_mod_owner; fl_owner_t (*lm_get_owner)(fl_owner_t); void (*lm_put_owner)(fl_owner_t); void (*lm_notify)(struct file_lock *); /* unblock callback */ @@ -1037,6 +1038,8 @@ 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_expirable)(struct file_lock *cfl); + void (*lm_expire_lock)(void); }; struct lock_manager {