Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: [PATCH 0/8] NFSD: clean up locking.
From:   NeilBrown <neilb@suse.de>
To:     Chuck Lever <chuck.lever@oracle.com>,
        Jeff Layton <jlayton@kernel.org>
Cc:     linux-nfs@vger.kernel.org
Date:   Wed, 06 Jul 2022 14:18:12 +1000
Message-ID: <165708033167.1940.3364591321728458949.stgit@noble.brown>
User-Agent: StGit/1.5
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

This series prepares NFSD to be able to adjust to work with a proposed
patch which allows updates to directories to happen in parallel.
This patch set changes the way directories are locked, so the present
series cleans up some locking in nfsd.

Specifically we remove fh_lock() and fh_unlock().
These functions are problematic for a few reasons.
- they are deliberately idempotent - setting or clearing a flag
  so that a second call does nothing.  This makes locking errors harder,
  but it results in code that looks wrong ...  and maybe sometimes is a
  little bit wrong.
  Early patches clean up several places where this idempotent nature of
  the functions is depended on, and so makes the code clearer.

- they transparently call fh_fill_pre/post_attrs(), including at times
  when this is not necessary.  Having the calls only when necessary is
  marginally more efficient, and arguably makes the code clearer.

nfsd_lookup() currently always locks the directory, though often no lock
is needed.  So a patch in this series reduces this locking.

There is an awkward case that could still be further improved.
NFSv4 open needs to ensure the file is not renamed (or unlinked etc)
between the moment when the open succeeds, and a later moment when a
"lease" is attached to support a delegation.  The handling of this lock
is currently untidy, particularly when creating a file.
It would probably be better to take a lease immediately after
opening the file, and then discarding if after deciding not to provide a
delegation.

I have run fstests and cthon tests on this, but I wouldn't be surprised
if there is a corner case that I've missed.

NeilBrown


---

NeilBrown (8):
      NFSD: drop rqstp arg to do_set_nfs4_acl()
      NFSD: change nfsd_create() to unlock directory before returning.
      NFSD: always drop directory lock in nfsd_unlink()
      NFSD: only call fh_unlock() once in nfsd_link()
      NFSD: reduce locking in nfsd_lookup()
      NFSD: use explicit lock/unlock for directory ops
      NFSD: use (un)lock_inode instead of fh_(un)lock for file operations
      NFSD: discard fh_locked flag and fh_lock/fh_unlock


 fs/nfsd/nfs2acl.c   |   6 +-
 fs/nfsd/nfs3acl.c   |   4 +-
 fs/nfsd/nfs3proc.c  |  21 ++---
 fs/nfsd/nfs4acl.c   |  19 ++---
 fs/nfsd/nfs4proc.c  | 106 +++++++++++++++---------
 fs/nfsd/nfs4state.c |   8 +-
 fs/nfsd/nfsfh.c     |   3 +-
 fs/nfsd/nfsfh.h     |  56 +------------
 fs/nfsd/nfsproc.c   |  14 ++--
 fs/nfsd/vfs.c       | 193 ++++++++++++++++++++++++++------------------
 fs/nfsd/vfs.h       |  19 +++--
 11 files changed, 238 insertions(+), 211 deletions(-)

--
Signature