[PATCHv2,nfsd/master,0/7] lockd: dlm: async lock request changes

Message

Alexander Aring Sept. 12, 2023, 9:53 p.m. UTC

Hi,

I sent this as a PATCH now and drop the RFC. I got some review back from
Jeff Layton and hope I was successful to follow it. There are still
issues with lockd and asynchronous lock request but it will at least not
directly crash when somebody is using nfs on top of GFS2. The issues are
related to cancellation of locks/lockd decides to drop nlm_block for
some reasons while pending request is happening.

I did not change more documentation about the asynchronous lock request
functionality to not confuse users. In my opinion there should be more
documentation about what you SHOULD NOT do with this API. Otherwise I
think the documentation is still correct.

I will send a follow up patch to fsdevel to change all users using
IS_SETLKW() to use FL_SLEEP to determine if it's blocking or
non-blocking and send it to fsdevel as it was recommended. This will
just be a grep and replace patch and look what happens.

- Alex

changes since v2:
 - remove B_PENDING_CALLBACK paragraph from commit msg. Was a leftover
   and I forgot to update the commit message.
 - change function name from export_op_support_safe_async_lock()
   to exportfs_lock_op_is_async()
 - change flag name from EXPORT_OP_SAFE_ASYNC_LOCK to
   EXPORT_OP_ASYNC_LOCK
 - add documentation for EXPORT_OP_ASYNC_LOCK to
   Documentation/filesystems/nfs/exporting.rst
 - add newline between function name and return type of
   exportfs_lock_op_is_async()
 - remove f_op->lock() check and mention it in
   Documentation/filesystems/nfs/exporting.rst to only use it with
   filesystems implementing their own ->lock()
 - add kdoc for exportfs_lock_op_is_async()

changes since RFC:

- drop the pending callback flag but introduce "lockd: don't call
  vfs_lock_file() for pending requests", see patch why I need it.
- drop per nlm_block callback mutex
- change async lock request documentation
- use always FL_SLEEP to determine if it's blocking vs non-blocking in
  DLM

Alexander Aring (7):
  lockd: introduce safe async lock op
  lockd: don't call vfs_lock_file() for pending requests
  lockd: fix race in async lock request handling
  lockd: add doc to enable EXPORT_OP_ASYNC_LOCK
  dlm: use fl_owner from lockd
  dlm: use FL_SLEEP to determine blocking vs non-blocking
  dlm: implement EXPORT_OP_ASYNC_LOCK

 Documentation/filesystems/nfs/exporting.rst |  7 ++++
 fs/dlm/plock.c                              | 20 +++--------
 fs/gfs2/export.c                            |  1 +
 fs/lockd/svclock.c                          | 38 ++++++++++++++-------
 fs/locks.c                                  | 12 ++++---
 fs/nfsd/nfs4state.c                         | 10 ++++--
 fs/ocfs2/export.c                           |  1 +
 include/linux/exportfs.h                    | 14 ++++++++
 8 files changed, 67 insertions(+), 36 deletions(-)

Comments

Chuck Lever III Sept. 13, 2023, 1:24 p.m. UTC | #1

> On Sep 12, 2023, at 5:53 PM, Alexander Aring <[e-mail]> wrote:
> 
> Hi,
> 
> I sent this as a PATCH now and drop the RFC. I got some review back from
> Jeff Layton and hope I was successful to follow it. There are still
> issues with lockd and asynchronous lock request but it will at least not
> directly crash when somebody is using nfs on top of GFS2. The issues are
> related to cancellation of locks/lockd decides to drop nlm_block for
> some reasons while pending request is happening.
> 
> I did not change more documentation about the asynchronous lock request
> functionality to not confuse users. In my opinion there should be more
> documentation about what you SHOULD NOT do with this API. Otherwise I
> think the documentation is still correct.
> 
> I will send a follow up patch to fsdevel to change all users using
> IS_SETLKW() to use FL_SLEEP to determine if it's blocking or
> non-blocking and send it to fsdevel as it was recommended. This will
> just be a grep and replace patch and look what happens.
> 
> - Alex
> 
> changes since v2:
> - remove B_PENDING_CALLBACK paragraph from commit msg. Was a leftover
>   and I forgot to update the commit message.
> - change function name from export_op_support_safe_async_lock()
>   to exportfs_lock_op_is_async()
> - change flag name from EXPORT_OP_SAFE_ASYNC_LOCK to
>   EXPORT_OP_ASYNC_LOCK
> - add documentation for EXPORT_OP_ASYNC_LOCK to
>   Documentation/filesystems/nfs/exporting.rst
> - add newline between function name and return type of
>   exportfs_lock_op_is_async()
> - remove f_op->lock() check and mention it in
>   Documentation/filesystems/nfs/exporting.rst to only use it with
>   filesystems implementing their own ->lock()
> - add kdoc for exportfs_lock_op_is_async()
> 
> changes since RFC:
> 
> - drop the pending callback flag but introduce "lockd: don't call
>  vfs_lock_file() for pending requests", see patch why I need it.
> - drop per nlm_block callback mutex
> - change async lock request documentation
> - use always FL_SLEEP to determine if it's blocking vs non-blocking in
>  DLM
> 
> Alexander Aring (7):
>  lockd: introduce safe async lock op
>  lockd: don't call vfs_lock_file() for pending requests
>  lockd: fix race in async lock request handling
>  lockd: add doc to enable EXPORT_OP_ASYNC_LOCK

I've applied these four to the nfsd/lockd tree for v6.7.

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/


>  dlm: use fl_owner from lockd
>  dlm: use FL_SLEEP to determine blocking vs non-blocking
>  dlm: implement EXPORT_OP_ASYNC_LOCK
> 
> Documentation/filesystems/nfs/exporting.rst |  7 ++++
> fs/dlm/plock.c                              | 20 +++--------
> fs/gfs2/export.c                            |  1 +
> fs/lockd/svclock.c                          | 38 ++++++++++++++-------
> fs/locks.c                                  | 12 ++++---
> fs/nfsd/nfs4state.c                         | 10 ++++--
> fs/ocfs2/export.c                           |  1 +
> include/linux/exportfs.h                    | 14 ++++++++
> 8 files changed, 67 insertions(+), 36 deletions(-)
> 
> -- 
> 2.31.1
> 

--
Chuck Lever