This looks very good to me. A couple of tiny nitpicks below:
> +/*
> + * Commit metadata changes to stable storage. You pay pass NULL for dchild.
> + */
The dchild argument is gone in this version.
> + struct writeback_control wbc = {
> + .sync_mode = WB_SYNC_ALL,
> + .nr_to_write = 0, /* metadata only */
> + };
> + int error = 0;
> +
> + if (!EX_ISSYNC(fhp->fh_export))
> + return 0;
> +
> + if (export_ops->commit_metadata) {
> + error = export_ops->commit_metadata(inode);
> + } else {
> + error = sync_inode(inode, &wbc);
> + }
Maybe move the wbc declaration into the else branch here to keep
variables in the smallest possible scope.
> @@ -1199,7 +1204,7 @@ nfsd_create_setattr(struct svc_rqst *rqstp, struct svc_fh *resfhp,
> if (current_fsuid() != 0)
> iap->ia_valid &= ~(ATTR_UID|ATTR_GID);
> if (iap->ia_valid)
> - return nfsd_setattr(rqstp, resfhp, iap, 0, (time_t)0);
> + return nfsd_setattr(rqstp, resfhp, iap, 0, 0);
While this is a worthwhile cleanup I'd not put it into a patch that is
now entirely unrelated.
> + err = nfsd_create_setattr(rqstp, resfhp, iap);
>
> + /*
> + * nfsd_setattr already committed the child. Transactional filesystems
> + * had a chance to commit changes for both parent and child
> + * simultaneously making the following commit_metadata a noop.
> + */
> + err2 = nfserrno(commit_metadata(fhp));
> + if (err2)
> err = err2;
The if statement above seems rather minindented, possibly due to the
partial use of spaces instead of tabs.
--
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
