From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hitoshi Mitake Subject: Re: [PATCH] lib/cleaner_exec.c: get process ID of cleanerd through pipe Date: Sun, 26 Jan 2014 01:46:09 +0900 Message-ID: <87ha8sko1a.wl%mitake.hitoshi@gmail.com> References: <1390397720-2259-1-git-send-email-konishi.ryusuke@lab.ntt.co.jp> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:message-id:from:to:cc:subject:in-reply-to:references :user-agent:mime-version:content-type; bh=yECN9Prjzc1VWcGD29s/vgByxSFQ3W0vmDU0aZ3TH1A=; b=cMppX0knWdDcC1Ymh+B1uXfqyv3i9We9tzj/ZVCxn2U0E8itO7+E0Vr/gqI3RE177S ggthfI5sKV64aI4PeXTggG/6HYTv5k/qO5bjJ10T+IwwBG3sMfDycH5bH5bHYJpUOF+6 CYUoV8g34HcMby3xNMgOhznI7CQz52tuS0vkvxuuxHxx9hTki1SgPDq+hh1AcLVUJJJR Fjlc5Rv9wF5qbAsnZ0L67SEKFC1ZW87e5z4GqswVkySwHNMhmfFXgG2RsH3At+dFpAz5 7kJMvLphskLd06P9HyI/B9jNHdQWORYpSdy0Uqpye7pKs75T1LeVKoUjcXw4fNWSzLeO C//A== In-Reply-To: <1390397720-2259-1-git-send-email-konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org> Sender: linux-nilfs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Ryusuke Konishi Cc: linux-nilfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Hitoshi Mitake , Hitoshi Mitake 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 > Cc: Hitoshi Mitake Looks good to me and sorry for my bug! Reviewed-by: Hitoshi Mitake 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html