Re: [PATCH 0/8] reexport lock fixes v3

[Chuck Lever III <chuck.lever@oracle.com>]

> On Aug 26, 2021, at 3:05 PM, Anna Schumaker <schumakeranna@gmail.com> wrote:
> 
> On Fri, Aug 20, 2021 at 5:02 PM J. Bruce Fields <bfields@redhat.com> wrote:
>> 
>> From: "J. Bruce Fields" <bfields@redhat.com>
>> 
>> The following fix up some problems that can cause crashes or silently
>> broken lock guarantees in the reexport case.
>> 
>> Note:
>>        - patches 1-5 are server side
>>        - patches 6-7 are client side
>>        - patch 8 affects both
>> 
>> Simplest might be for Trond or Anna to ACK 6-8, if they look OK, and
> 
> They look okay to me. You can add:
>        Acked-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> to all three.
> 
> Anna

Thanks. I've captured Anna's Acks and included 9/8 (posted a few
days ago). See:

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/log/?h=for-next



>> then submit them all through the server.  But those three sets of
>> patches are all independent if you'd rather split them up.
>> 
>> Not fixed:
>>        - Attempts to reclaim locks after a reboot of the reexport
>>          server will fail.  This at least seems like an improvement
>>          over the current situation, which is that they'll succeed even
>>          in cases where they shouldn't.  Complete support for reboot
>>          recovery is a bigger job.
>> 
>>        - NFSv4.1+ lock nofications don't work.  So, clients have to
>>          poll as they do with NFSv4.0, which is suboptimal, but correct
>>          (and an improvement over the current situation, which is a
>>          kernel oops).
>> 
>> So what we have at this point is a suboptimal lock implementation that
>> doesn't support lock recovery.
>> 
>> Another alternative might be to turn off file locking entirely in the
>> re-export case.  I'd rather take the incremental improvement and fix the
>> oopses.
>> 
>> Change since v2:
>>        - keep nlmsvc_file_inode a static inline to address build
>>          failure identified by the kernel test robot
>> Changes since v1:
>>        - Use ENOGRACE instead of returning NFS-specific error from vfs lock
>>          method.
>>        - Take write opens for write locks in the NLM case (as we always
>>          have in the NFSv4 case).
>>        - Don't block NLM threads waiting for blocking locks.
>> 
>> With those changes I'm passing connecthon tests for NFSv3-4.2 reexports
>> of an NFSv4.0 filesystem.
>> 
>> --b.
>> 
>> J. Bruce Fields (8):
>>  lockd: lockd server-side shouldn't set fl_ops
>>  nlm: minor nlm_lookup_file argument change
>>  nlm: minor refactoring
>>  lockd: update nlm_lookup_file reexport comment
>>  Keep read and write fds with each nlm_file
>>  nfs: don't atempt blocking locks on nfs reexports
>>  lockd: don't attempt blocking locks on nfs reexports
>>  nfs: don't allow reexport reclaims
>> 
>> fs/lockd/svc4proc.c         |   6 +-
>> fs/lockd/svclock.c          |  80 ++++++++++++++----------
>> fs/lockd/svcproc.c          |   6 +-
>> fs/lockd/svcsubs.c          | 117 +++++++++++++++++++++++++-----------
>> fs/nfs/export.c             |   2 +-
>> fs/nfs/file.c               |   3 +
>> fs/nfsd/lockd.c             |   8 ++-
>> fs/nfsd/nfs4state.c         |  11 +++-
>> fs/nfsd/nfsproc.c           |   1 +
>> include/linux/errno.h       |   1 +
>> include/linux/exportfs.h    |   2 +
>> include/linux/fs.h          |   1 +
>> include/linux/lockd/bind.h  |   3 +-
>> include/linux/lockd/lockd.h |  11 +++-
>> 14 files changed, 170 insertions(+), 82 deletions(-)
>> 
>> --
>> 2.31.1
>> 

--
Chuck Lever