[PATCH RFC 04/12] nfs: concurrent O_CREAT in one NFSv4 directory

[Jonathan Flynn]

Set IOPF_ATOMIC_OPEN_CREATE_SHARED in nfs4_dir_inode_operations, so
that the VFS takes the parent shared rather than exclusive for an
O_CREAT open in an NFSv4 directory.  Creates of different names in one
directory then stop serializing on the parent inode, except on a server
that reports itself case-insensitive or not case-preserving, where
"nfs: tell the VFS when two names can be one entry on the server" has
set SB_I_NAMES_COLLIDE and creates keep the exclusive lock.

This is the only user of "vfs: allow the parent lock to be shared for
O_CREAT", and what it needs from it is the guarantee that the dentry
reaching the atomic-open method for a create is in-lookup.  NFSv4 keeps
the dentry the VFS gives it when O_CREAT is set, and publishes the new
inode on that dentry; because it is in-lookup, the dentry is not in the
main hash, so dropping it is a no-op and publishing it is permitted.
The name enters the hash exactly once, and until it does, a concurrent
lookup or create of the same name finds the in-lookup dentry and waits.

NFS does not use the exclusive parent lock to serialize its own
per-directory state, which is what that patch asks an opting-in
filesystem to promise.  Both pieces of state a create touches are
already under finer locks: the directory's change attribute and the
cache-validity bookkeeping that moves with it are updated under the
directory's own spinlock, and a dentry's directory verifier is stored
under that dentry's lock.  Neither needs the parent lock held
exclusive, or held at all.  The verifier is also ordered so that losing
a race can only cost work: a verifier offered against a directory state
it no longer reflects is declined rather than stored, so the failure
direction is an extra revalidation, never a dentry claiming stale
validity.

Lookup and readdir already run with the parent held shared, and
readdir already instantiates dentries there under that lock:
nfs_prime_dcache() takes an in-lookup dentry with d_alloc_trylock(),
skipping a name another thread holds in-lookup, and invalidates and
retries a hashed dentry that no longer matches the listing.  A create
joins concurrency NFS already handles.

NeilBrown's "nfs: remove d_drop()/d_alloc_parallel() from
nfs_atomic_open()" states that the method has exclusive access to the
dentry in one of two ways: O_CREAT, where the parent is held exclusive,
or a dentry carrying DCACHE_PAR_LOOKUP.  Setting the flag removes the
first of those for open(2), which reaches the method through
lookup_open(); nfsd reaches it through dentry_create(), which still
holds the parent exclusive.  "vfs: allow the parent lock to be shared
for O_CREAT" supplies the second in its place: with the parent held
shared for a create, a hashed negative dentry is rejected and the
lookup retried, so an in-lookup dentry is what reaches the method.
NFS's invalidation of a cached negative for LOOKUP_OPEN and that
rejection do not overlap: nfs_neg_need_reval() returns 0 for
LOOKUP_CREATE before it can reach the LOOKUP_OPEN case that invalidates
a cached negative, so the invalidation that covers a plain open never
fires for a create, and the rejection in "vfs: allow the parent lock to
be shared for O_CREAT" never fires for anything else.  The opener newly
in company with a shared create is one opening the same name without
O_CREAT, and that is the opener the invalidation covers.

The flag goes on the NFSv4 directory operations only.  NFSv2 and NFSv3
reach a different atomic-open path, which has not been analysed or
exercised under a shared parent lock, so it does not get the flag;
adding it later is one line.  Referral and automount directories are
unaffected, since they are given different operations vectors.

Every other directory-modifying operation still takes the parent
exclusive; for unlink that changes when "nfs: remove files with the
parent directory locked shared on NFSv4", later in this series, gives
NFSv4 ->unlink_shared().

The measured effect, from a 7.1.13-based kernel carrying an earlier
revision of both patches, is in "vfs: allow the parent lock to be
shared for O_CREAT".

Signed-off-by: Jonathan Flynn <jonathan.flynn@xxxxxxxxxxxxxxx>
---
 fs/nfs/nfs4proc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index fac8450b65..808bb8f1c1 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -10869,6 +10869,7 @@ static const struct inode_operations nfs4_dir_inode_operations = {
 	.setattr	= nfs_setattr,
 	.listxattr	= nfs4_listxattr,
 	.fileattr_get	= nfs_fileattr_get,
+	.iop_flags	= IOPF_ATOMIC_OPEN_CREATE_SHARED,
 };
 
 static const struct inode_operations nfs4_file_inode_operations = {
-- 
2.43.0