At Wed, 22 Jan 2014 22:35:20 +0900, Ryusuke Konishi wrote: > > After applying commit 8ce4b524a1654ece478f54fce772cc0c16e3c559 > "cleanerd: call _exit(2) twice for ensuring not being a session > leader", the mount helper program of nilfs2 no longer gets proper > process ID (pid) of cleaner daemon, and umount.nilfs2 fails to kill > cleanerd. > > This fixes the issue by letting cleanerd print out the pid of spawned > daemon and letting nilfs_launch_cleanerd() function read it through > pipe. > > Signed-off-by: Ryusuke Konishi <[email protected]> > Cc: Hitoshi Mitake <[email protected]>
Looks good to me and sorry for my bug! Reviewed-by: Hitoshi Mitake <[email protected]> Thanks, Hitoshi > --- > lib/cleaner_exec.c | 84 > ++++++++++++++++++++++++++++++++++++++++++---- > man/nilfs_cleanerd.8 | 3 ++ > sbin/cleanerd/cleanerd.c | 3 ++ > 3 files changed, 83 insertions(+), 7 deletions(-) > > diff --git a/lib/cleaner_exec.c b/lib/cleaner_exec.c > index edc55e3..5336c32 100644 > --- a/lib/cleaner_exec.c > +++ b/lib/cleaner_exec.c > @@ -114,6 +114,43 @@ static inline int process_is_alive(pid_t pid) > return (kill(pid, 0) == 0); > } > > +static int receive_pid(int fd, pid_t *ppid) > +{ > + char buf[100]; > + unsigned long pid; > + FILE *fp; > + int ret; > + > + fp = fdopen(fd, "r"); > + if (!fp) { > + nilfs_cleaner_logger( > + LOG_ERR, _("Error: fdopen failed: %m")); > + close(fd); > + goto fail; > + } > + > + /* read process ID of cleanerd through the given fd */ > + while (fgets(buf, sizeof(buf), fp) != NULL) { > + /* > + * fgets() is blocked during no child processes > + * respond, but it will escape returning a NULL value > + * and terminate the loop when all child processes > + * close the given pipe (fd) including call of exit(). > + */ > + ret = sscanf(buf, "NILFS_CLEANERD_PID=%lu", &pid); > + if (ret == 1) { > + *ppid = pid; > + fclose(fp); /* this also closes fd */ > + return 0; > + } > + } > + fclose(fp); > +fail: > + nilfs_cleaner_logger( > + LOG_WARNING, _("Warning: cannot get pid of cleanerd")); > + return -1; > +} > + > int nilfs_launch_cleanerd(const char *device, const char *mntdir, > unsigned long protperiod, pid_t *ppid) > { > @@ -123,6 +160,7 @@ int nilfs_launch_cleanerd(const char *device, const char > *mntdir, > int i = 0; > int ret; > char buf[256]; > + int pipes[2]; > > if (stat(cleanerd, &statbuf) != 0) { > nilfs_cleaner_logger(LOG_ERR, _("Error: %s not found"), > @@ -130,8 +168,16 @@ int nilfs_launch_cleanerd(const char *device, const char > *mntdir, > return -1; > } > > + ret = pipe(pipes); > + if (ret < 0) { > + nilfs_cleaner_logger( > + LOG_ERR, _("Error: failed to create pipe: %m")); > + return -1; > + } > + > ret = fork(); > if (ret == 0) { > + /* child */ > if (setgid(getgid()) < 0) { > nilfs_cleaner_logger( > LOG_ERR, > @@ -159,16 +205,40 @@ int nilfs_launch_cleanerd(const char *device, const > char *mntdir, > sigdelset(&sigs, SIGSEGV); > sigprocmask(SIG_UNBLOCK, &sigs, NULL); > > + close(pipes[0]); > + ret = dup2(pipes[1], STDOUT_FILENO); > + if (ret < 0) { > + nilfs_cleaner_logger( > + LOG_ERR, > + _("Error: failed to duplicate pipe: %m")); > + exit(1); > + } > + close(pipes[1]); > + > execv(cleanerd, (char **)dargs); > exit(1); /* reach only if failed */ > - } else if (ret != -1) { > - *ppid = ret; > - return 0; /* cleanerd started */ > + } else if (ret > 0) { > + /* parent */ > + close(pipes[1]); > + > + /* > + * fork() returns a pid of the child process, but we > + * cannot use it because cleanerd internally fork()s > + * and changes pid. > + */ > + ret = receive_pid(pipes[0], ppid); > + if (ret < 0) > + *ppid = 0; > + /* > + * always return a success code because cleanerd has > + * already started. > + */ > + return 0; > } else { > - int errsv = errno; > - nilfs_cleaner_logger(LOG_ERR, > - _("Error: could not fork: %s"), > - strerror(errsv)); > + nilfs_cleaner_logger( > + LOG_ERR, _("Error: could not fork: %m")); > + close(pipes[0]); > + close(pipes[1]); > } > return -1; > } > diff --git a/man/nilfs_cleanerd.8 b/man/nilfs_cleanerd.8 > index 46caef2..fd1c48a 100644 > --- a/man/nilfs_cleanerd.8 > +++ b/man/nilfs_cleanerd.8 > @@ -24,6 +24,9 @@ users are recommended to invoke \fBnilfs_cleanerd\fP through > \fBmount.nilfs2\fP(8) or \fBmount\fP(8) and shutdown it through > \fBumount.nilfs2\fP(8) or \fBumount\fP(8) in order to avoid state > inconsistencies among administration tools. > +.PP > +\fBnilfs_cleanerd\fP displays its process ID (pid) to standard > +output when it started. > .SH OPTIONS > .TP > \fB\-V\fR, \fB\-\-version\fR > diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c > index 86dfcf7..742ab98 100644 > --- a/sbin/cleanerd/cleanerd.c > +++ b/sbin/cleanerd/cleanerd.c > @@ -719,6 +719,9 @@ static int daemonize(int nochdir, int noclose) > if (!nochdir && (chdir(ROOTDIR) < 0)) > return -1; > > + printf("NILFS_CLEANERD_PID=%lu\n", (unsigned long)getpid()); > + fflush(stdout); > + > if (!noclose) { > close(0); > close(1); > -- > 1.7.9.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
