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;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Jeff Layton" <jlayton@kernel.org>
Cc:     "Chuck Lever" <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 6/8] NFSD: use explicit lock/unlock for directory ops
In-reply-to: <ccc3c78c5a6b4b72f6160aeb38ffa36cec94595f.camel@kernel.org>
References: <165708033167.1940.3364591321728458949.stgit@noble.brown>,
 <165708109259.1940.685583862810495747.stgit@noble.brown>,
 <ccc3c78c5a6b4b72f6160aeb38ffa36cec94595f.camel@kernel.org>
Date:   Mon, 18 Jul 2022 09:43:16 +1000
Message-id: <165810139630.25184.17789001759725189604@noble.neil.brown.name>
Precedence: bulk

On Sat, 16 Jul 2022, Jeff Layton wrote:
> On Wed, 2022-07-06 at 14:18 +1000, NeilBrown wrote:
> >  
> > +	fh_fill_pre_attrs(fhp);
> >  	status = nfsd4_vfs_create(fhp, child, open);
> >  	if (status != nfs_ok)
> >  		goto out;
> >  	open->op_created = true;
> > +	fh_fill_post_attrs(fhp);
> >  
> >  	/* A newly created file already has a file size of zero. */
> >  	if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0))
> 
> Should the fh_fill_post_attrs call be done after nfsd_create_setattr
> instead in this function? It seems like we're filling out the post-op
> attr here before we're actually done changing things...

nfsd_create_setattr() only affects the newly created thing, so it should
not be changing any attributes of the directory that it was created in.
So it should not matter for correctness where fh_fill_post_attrs() is
called, as long as it is between nfsd4_vfs_create() and inode_unlock().

I preferred closer to the former.

Thanks,
NeilBrown