Re: [PATCH v2 06/14] nfsd: in nfsd4_create_file() let VFS report if file was created.

On Tue, 2026-07-07 at 09:28 +1000, NeilBrown wrote:
> On Tue, 07 Jul 2026, Jeff Layton wrote:
> > On Mon, 2026-07-06 at 08:19 +1000, NeilBrown wrote:
> > > From: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> > > 
> > > nfsd4_create_file() currently assumes that if a lookup failed but then a
> > > create succeeds, then the "create" operation actually created the file.
> > > With atomic_open this may not be the case - some other actor might have
> > > created the file between the lookup and the create.
> > > 
> > > So we move the call to nfsd4_vfs_create() earlier and set ->op_created
> > > based on the FMODE_CREATED flag that it set.  Then use "!  ->op_created"
> > > to trigger nfserr_exist handling.
> > > 
> > > The switch statement is split up into two if() statements.
> > > First we check for the possibility of a successful exclusive
> > > create and set ->op_create to true if appropriate.
> > > Then we check for NFS4_CREATE_UNCHECKED to decide if a
> > > pre-existing file means an error or success.
> > > 
> > > This allows us to combine the two fh_compose() calls to one place.
> > > 
> > > A subtle difference here is that we now must only pass O_EXCL to
> > > dentry_create() for NFS4_CREATE_GUARDED.  For the EXCLUSIVE create modes
> > > we want a successful open even if the file already exists.  We then
> > > check the verifier after the open succeeded to see if it was exclusive.
> > > 
> > 
> > Do we really want a successful open in the EXCLUSIVE cases?
> > 
> > Opens have side effects (notably, that they can cause delegation
> > recalls). If you have two racing clients creating a file, the first
> > gets an open and write delegation and then the second ends up
> > immediately causing a delegrecall for the first, even though it may
> > never touch the file again after the OPEN fails.
> > 
> > I think we may want to reconsider that logic, if possible: Maybe we
> > should keep using O_EXCL in those cases and just re-drive the open
> > without it if it fails and the verifier looks right? That's a bit
> > uglier, but that may cause fewer delegation recalls.
> 
> Thanks for raising this.  My thinking was that next EXCLUSIVE creates
> was weird.
> If the server is re-exporting NFS, then think about what happens to the
> verifier stored on the backend server.
> First the verifier from the intermediate server is stored, 
> then the nfs client on the intermediate server sends a SETATTR to remove it, 
> then the intermediate server sets the verifier from the originating
> client.
> then the originating client removes it.
> 
> So 4 setattrs altogether.
> 
> Also if the intermediate server ever gets a retransmit of the OPEN
> request, it will send an EXCLUSIVE open to the backend server which will
> fail even if the verifier is still correct.
> 
> I agree that recalling a delegation needlessly is not good, but will it
> happen often?  If the app checks for then name before trying the O_EXCL
> open, then the delegation would not get recalled because the OPEN
> wouldn't be tried.  I wonder how often that happens....
> 
> I think setting O_EXCL in the NFS4_CREATE_GUARDED case is important.
> I'm open to discussion around the issues with O_EXCL and the
> NFS4_CREATE_EXCLUSIVE case.
> 
> Maybe doing a lookup first when the exported fs is NFS would be an OK
> compromise.
> 


I guess it shouldn't happen often, since we do check
d_really_is_negative() prior to calling nfsd4_vfs_create(). Given that,
let's go with the scheme you've got here. We can always revisit it if
this turns out to be more of an issue.
-- 
Jeff Layton <[email protected]>