> In the cases you pass the child dentry, you will now be syncing > things to disk in a slightly different way/order from before. > I think this way is actually much better, but I thought I'd ask > anyway: are you sure that it's OK to sync them at the same time, > rather than directory first, then child (or the other way around, > depending on the case)? The order is defined by when we commit the transactions, if we do the log force on the later one first we already write out the first transaction. Note that in any transaction filesystems the changes that create/mkdir/link/unlink do to parent and child will be in the same transaction anyway, which is kinda the point of adding the transactions to start with. Only for the case where we do a setattr in addition to the primary transaction we'll actually have another transaction to deal with and the above applies. > > nfsd4_sync_rec_dir(void) > > { > > - mutex_lock(&rec_dir.dentry->d_inode->i_mutex); > > - nfsd_sync_dir(rec_dir.dentry); > > - mutex_unlock(&rec_dir.dentry->d_inode->i_mutex); > > + vfs_fsync(NULL, rec_dir.dentry, 0); > > Why do you no longer need to acquire the mutex here? > I see it gets acquired during the ->fsync() call > inside vfs_fsync_range(), but is there a reason > that it needed to be held during the filemap_write_and_wait() > (as is done in the nfsd_sync_dir() code) also? We don't need i_mutex for filemap_write_and_wait, it's just held because someone used the nfsd_sync_dir helper where it doesn't fit very well. > > @@ -415,9 +449,8 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap, > > } > > if (size_change) > > put_write_access(inode); > > - if (!err) > > - if (EX_ISSYNC(fhp->fh_export)) > > - write_inode_now(inode, 1); > > + if (!err && !delay_commit) > > + err = nfserrno(commit_metadata(fhp, NULL)); > > Is this sufficient even if the size has changed? Yes. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html