On Fri, Dec 05, 2008 at 08:26:44AM -0500, Steve Dickson wrote:
> J. Bruce Fields wrote:
> >
> >> I think it would still be valuable to replace the 'sync' with two
> >> 'fsync's, one of the file, one on the directory.
> >
> > Sure, may as well.--b.
> >
> Something similar to this:
>
> diff -up nfs-utils/utils/statd/sm-notify.c.orig nfs-utils/utils/statd/sm-notify.c
> --- nfs-utils/utils/statd/sm-notify.c.orig 2008-11-17 15:06:13.000000000 -0500
> +++ nfs-utils/utils/statd/sm-notify.c 2008-12-05 08:21:52.000000000 -0500
> @@ -211,12 +211,6 @@ usage: fprintf(stderr,
> backup_hosts(_SM_DIR_PATH, _SM_BAK_PATH);
> get_hosts(_SM_BAK_PATH);
>
> - /* If there are not hosts to notify, just exit */
> - if (!hosts) {
> - nsm_log(LOG_DEBUG, "No hosts to notify; exiting");
> - return 0;
> - }
This was still a huge boot-time win in the common case, so now that
we've committed to it I'd rather not regress. Let's just skip the
sync()s/fsncy()s in the !hosts case--that looks to me like the simplest
correct solution for now.
> -
> /* Get and update the NSM state. This will call sync() */
> nsm_state = nsm_get_state(opt_update_state);
> set_kernel_nsm_state(nsm_state);
> @@ -694,6 +688,7 @@ nsm_get_state(int update)
> {
> char newfile[PATH_MAX];
> int fd, state;
> + DIR *fp;
>
> if ((fd = open(_SM_STATE_PATH, O_RDONLY)) < 0) {
> if (!opt_quiet) {
> @@ -730,13 +725,16 @@ nsm_get_state(int update)
> "Failed to write state to %s", newfile);
> exit(1);
> }
> + fsync(fd);
> close(fd);
> + fp = opendir(_SM_STATE_PATH);
Also, I think you meant to:
fsync(fp);
close(fp);
?
--b.
> if (rename(newfile, _SM_STATE_PATH) < 0) {
> nsm_log(LOG_ERR,
> "Cannot create %s: %m", _SM_STATE_PATH);
> exit(1);
> }
> - sync();
>
> If so, I'll make it happen...
>
> steved.
--
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
