Re: [PATCH v4 0/3] Allow knfsd to use atomic_open()

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

On 12/4/25 10:05 AM, Benjamin Coddington wrote:
> Hi Chuck, Christian, Al,
> 
> Comments have died down.  I have some review on this one, and quite a lot of
> testing in-house.  What else can I do to get this into linux-next on this
> cycle?
The merge window is open right now, so any new work like this will be
targeted for the next kernel, not v6.19-rc.

I assume that since you sent To: Al/Christian, they would be taking this
through one of the VFS trees; hence my R-b.

If you're going only for linux-next, you can open your own kernel.org
account and set up a git repo there, then Stephen can pull from that
repo into linux-next and fs-next.


> Ben
> 
> On 27 Nov 2025, at 11:02, Benjamin Coddington wrote:
> 
>> We have workloads that will benefit from allowing knfsd to use atomic_open()
>> in the open/create path.  There are two benefits; the first is the original
>> matter of correctness: when knfsd must perform both vfs_create() and
>> vfs_open() in series there can be races or error results that cause the
>> caller to receive unexpected results.  The second benefit is that for some
>> network filesystems, we can reduce the number of remote round-trip
>> operations by using a single atomic_open() path which provides a performance
>> benefit.
>>
>> I've implemented this with the simplest possible change - by modifying
>> dentry_create() which has a single user: knfsd.  The changes cause us to
>> insert ourselves part-way into the previously closed/static atomic_open()
>> path, so I expect VFS folks to have some good ideas about potentially
>> superior approaches.
>>
>> Previous work on commit fb70bf124b05 ("NFSD: Instantiate a struct file when
>> creating a regular NFSv4 file") addressed most of the atomicity issues, but
>> there are still a few gaps on network filesystems.
>>
>> The problem was noticed on a test that did open O_CREAT with mode 0 which
>> will succeed in creating the file but will return -EACCES from vfs_open() -
>> this specific test is mentioned in 3/3 description.
>>
>> Also, Trond notes that independently of the permissions issues, atomic_open
>> also solves races in open(O_CREAT|O_TRUNC). The NFS client now uses it for
>> both NFSv4 and NFSv3 for that reason.  See commit 7c6c5249f061 "NFS: add
>> atomic_open for NFSv3 to handle O_TRUNC correctly."
>>
>> Changes on v4:
>> 	- ensure we pass O_EXCL for NFS4_CREATE_EXCLUSIVE and
>>   NFS4_CREATE_EXCLUSIVE4_1, thanks to Neil Brown
>>
>> Changes on v3:
>> 	- rebased onto v6.18-rc7
>> 	- R-b on 3/3 thanks to Chuck Lever
>>
>> Changes on v2:
>> 	- R-b thanks to Jeff Layton
>> 	- improvements to patch descriptions thanks to Chuck Lever, Neil
>>   Brown, and Trond Myklebust
>> 	- improvements to dentry_create()'s doc comment to clarify dentry
>>   handling thanks to Mike Snitzer
>>
>> Thanks for any additional comment and critique.  gobble gobble
>>
>>
>> Benjamin Coddington (3):
>>   VFS: move dentry_create() from fs/open.c to fs/namei.c
>>   VFS: Prepare atomic_open() for dentry_create()
>>   VFS/knfsd: Teach dentry_create() to use atomic_open()
>>
>>  fs/namei.c         | 86 ++++++++++++++++++++++++++++++++++++++++++----
>>  fs/nfsd/nfs4proc.c | 11 ++++--
>>  fs/open.c          | 41 ----------------------
>>  include/linux/fs.h |  2 +-
>>  4 files changed, 88 insertions(+), 52 deletions(-)
>>
>> -- 
>> 2.50.1


-- 
Chuck Lever