|
»
»
[PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
|
View:
New views
13 Messages
—
Rating Filter:
Alert me
|
 |
[PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by William Cohen
::
Rate this Message:
The ia64 handles things a bit differently than other architectures. It starts up
a number of children processes to control perfmon on each of the processors in the machine. However, these children processes do not close the io. The results in children processes still being connected to the console. There are more details about this problem at: https://bugzilla.redhat.com/show_bug.cgi?id=518480 I have made a pass at a patch to fix this particular problem and I am looking for feedback on the patch: 2009-09-01 William Cohen <wcohen@...> * daemon/opd_perfmon.c: Fix ia64 output to allow spawning daemon. Signed-off-by: William Cohen <wcohen@...> -Will diff -up oprofile-0.9.4/daemon/init.c.orig oprofile-0.9.4/daemon/init.c diff -up oprofile-0.9.4/daemon/opd_perfmon.c.orig oprofile-0.9.4/daemon/opd_perfmon.c --- oprofile-0.9.4/daemon/opd_perfmon.c.orig 2009-09-01 10:45:52.000000000 -0400 +++ oprofile-0.9.4/daemon/opd_perfmon.c 2009-09-01 11:42:19.000000000 -0400 @@ -141,7 +141,6 @@ static void child_sigusr2(int val __attr static void child_sigterm(int val __attribute__((unused))) { - printf("Child received SIGTERM, killing parent.\n"); kill(getppid(), SIGTERM); } @@ -156,8 +155,7 @@ static void set_affinity(size_t cpu) int err = sched_setaffinity(getpid(), sizeof(set), &set); if (err == -1) { - fprintf(stderr, "Failed to set affinity: %s\n", - strerror(errno)); + perror("Failed to set affinity: "); exit(EXIT_FAILURE); } } @@ -214,8 +212,7 @@ static void create_context(struct child err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); if (err == -1) { - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", - strerror(errno)); + perror("CREATE_CONTEXT failed: "); exit(EXIT_FAILURE); } @@ -304,8 +301,7 @@ static void notify_parent(struct child * if (ret == sizeof(size_t)) break; if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to write child pipe with %s\n", - strerror(errno)); + perror("Failed to write child pipe with "); exit(EXIT_FAILURE); } } @@ -333,6 +329,11 @@ static void run_child(size_t cpu) notify_parent(self, cpu); + /* Close IO to allow this child process to be run as a daemon */ + close(0); + close(1); + close(2); + for (;;) { sigset_t sigmask; sigfillset(&sigmask); @@ -341,15 +342,11 @@ static void run_child(size_t cpu) sigdelset(&sigmask, SIGTERM); if (self->sigusr1) { - printf("PFM_START on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_start_child(self->ctx_fd); self->sigusr1 = 0; } if (self->sigusr2) { - printf("PFM_STOP on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_stop_child(self->ctx_fd); self->sigusr2 = 0; } @@ -368,13 +365,10 @@ static void wait_for_child(struct child if (ret == sizeof(size_t)) break; if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to read child pipe with %s\n", - strerror(errno)); + perror("Failed to read child pipe with "); exit(EXIT_FAILURE); } } - printf("Perfmon child up on CPU%d\n", (int)tmp); - fflush(stdout); close(child->up_pipe[0]); close(child->up_pipe[1]); @@ -422,21 +416,18 @@ void perfmon_init(void) int ret; if (pipe(children[i].up_pipe)) { - perror("Couldn't create child pipe.\n"); + perror("Couldn't create child pipe: "); exit(EXIT_FAILURE); } ret = fork(); if (ret == -1) { - fprintf(stderr, "Couldn't fork perfmon child.\n"); + perror("Couldn't fork perfmon child: "); exit(EXIT_FAILURE); } else if (ret == 0) { - printf("Running perfmon child on CPU%d.\n", (int)i); - fflush(stdout); run_child(i); } else { children[i].pid = ret; - printf("Waiting on CPU%d\n", (int)i); wait_for_child(&children[i]); } } ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by Maynard Johnson
::
Rate this Message:
William Cohen wrote:
> The ia64 handles things a bit differently than other architectures. It starts up > a number of children processes to control perfmon on each of the processors in > the machine. However, these children processes do not close the io. The results > in children processes still being connected to the console. There are more > details about this problem at: > > https://bugzilla.redhat.com/show_bug.cgi?id=518480 > > I have made a pass at a patch to fix this particular problem and I am looking > for feedback on the patch: > > 2009-09-01 William Cohen <wcohen@...> > > * daemon/opd_perfmon.c: Fix ia64 output to allow spawning daemon. > > > Signed-off-by: William Cohen <wcohen@...> > > -Will > > > > diff -up oprofile-0.9.4/daemon/init.c.orig oprofile-0.9.4/daemon/init.c > diff -up oprofile-0.9.4/daemon/opd_perfmon.c.orig oprofile-0.9.4/daemon/opd_perfmon.c > --- oprofile-0.9.4/daemon/opd_perfmon.c.orig 2009-09-01 10:45:52.000000000 -0400 > +++ oprofile-0.9.4/daemon/opd_perfmon.c 2009-09-01 11:42:19.000000000 -0400 > @@ -141,7 +141,6 @@ static void child_sigusr2(int val __attr > > static void child_sigterm(int val __attribute__((unused))) > { > - printf("Child received SIGTERM, killing parent.\n"); > kill(getppid(), SIGTERM); > } > > @@ -156,8 +155,7 @@ static void set_affinity(size_t cpu) > int err = sched_setaffinity(getpid(), sizeof(set), &set); > > if (err == -1) { > - fprintf(stderr, "Failed to set affinity: %s\n", > - strerror(errno)); > + perror("Failed to set affinity: "); > exit(EXIT_FAILURE); > } > } > @@ -214,8 +212,7 @@ static void create_context(struct child > > err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); > if (err == -1) { > - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", > - strerror(errno)); > + perror("CREATE_CONTEXT failed: "); > exit(EXIT_FAILURE); > } > > @@ -304,8 +301,7 @@ static void notify_parent(struct child * > if (ret == sizeof(size_t)) > break; > if (ret < 0 && errno != EINTR) { > - fprintf(stderr, "Failed to write child pipe with %s\n", > - strerror(errno)); > + perror("Failed to write child pipe with "); > exit(EXIT_FAILURE); > } > } > @@ -333,6 +329,11 @@ static void run_child(size_t cpu) > > notify_parent(self, cpu); > > + /* Close IO to allow this child process to be run as a daemon */ > + close(0); > + close(1); > + close(2); > + > for (;;) { > sigset_t sigmask; > sigfillset(&sigmask); > @@ -341,15 +342,11 @@ static void run_child(size_t cpu) > sigdelset(&sigmask, SIGTERM); > > if (self->sigusr1) { > - printf("PFM_START on CPU%d\n", (int)cpu); > - fflush(stdout); > perfmon_start_child(self->ctx_fd); -Maynard > self->sigusr1 = 0; > } > > if (self->sigusr2) { > - printf("PFM_STOP on CPU%d\n", (int)cpu); > - fflush(stdout); > perfmon_stop_child(self->ctx_fd); > self->sigusr2 = 0; > } > @@ -368,13 +365,10 @@ static void wait_for_child(struct child > if (ret == sizeof(size_t)) > break; > if (ret < 0 && errno != EINTR) { > - fprintf(stderr, "Failed to read child pipe with %s\n", > - strerror(errno)); > + perror("Failed to read child pipe with "); > exit(EXIT_FAILURE); > } > } > - printf("Perfmon child up on CPU%d\n", (int)tmp); > - fflush(stdout); > > close(child->up_pipe[0]); > close(child->up_pipe[1]); > @@ -422,21 +416,18 @@ void perfmon_init(void) > int ret; > > if (pipe(children[i].up_pipe)) { > - perror("Couldn't create child pipe.\n"); > + perror("Couldn't create child pipe: "); > exit(EXIT_FAILURE); > } > > ret = fork(); > if (ret == -1) { > - fprintf(stderr, "Couldn't fork perfmon child.\n"); > + perror("Couldn't fork perfmon child: "); > exit(EXIT_FAILURE); > } else if (ret == 0) { > - printf("Running perfmon child on CPU%d.\n", (int)i); > - fflush(stdout); > run_child(i); > } else { > children[i].pid = ret; > - printf("Waiting on CPU%d\n", (int)i); > wait_for_child(&children[i]); > } > } > > > > > ------------------------------------------------------------------------ > > ------------------------------------------------------------------------------ > Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day > trial. Simplify your report design, integration and deployment - and focus on > what you do best, core application coding. Discover what's new with > Crystal Reports now. http://p.sf.net/sfu/bobj-july > > > ------------------------------------------------------------------------ > > _______________________________________________ > oprofile-list mailing list > oprofile-list@... > https://lists.sourceforge.net/lists/listinfo/oprofile-list ------------------------------------------------------------------------------ Come build with us! The BlackBerry® Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9-12, 2009. Register now! http://p.sf.net/sfu/devconf _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by Maynard Johnson
::
Rate this Message:
Maynard Johnson wrote:
> William Cohen wrote: >> The ia64 handles things a bit differently than other architectures. It starts up >> a number of children processes to control perfmon on each of the processors in >> the machine. However, these children processes do not close the io. The results >> in children processes still being connected to the console. There are more >> details about this problem at: Will, I was going through the list, checking for unfinished business on patch submissions, and I found this one which you've not yet responded to my comments. I noticed you re-posted this exact patch on Oct 6, so I'm guessing you didn't see my original reply with comments. So I'm re-sending it. I'm seriously considering a bug-fix release (mainly to address the regressions introduced in 0.9.5), but I'm open to including other bug fixes. Please reply as soon as possible, as I'd like to get the bug-fix release put out soon. Thanks. -Maynard >> >> https://bugzilla.redhat.com/show_bug.cgi?id=518480 >> >> I have made a pass at a patch to fix this particular problem and I am looking >> for feedback on the patch: > I've never used the perfmon version of oprofile, so my feedback is probably not that helpful. But I'll give you my 2 cents worth anyway. :-) >> 2009-09-01 William Cohen <wcohen@...> >> >> * daemon/opd_perfmon.c: Fix ia64 output to allow spawning daemon. >> >> >> Signed-off-by: William Cohen <wcohen@...> >> >> -Will >> >> >> >> diff -up oprofile-0.9.4/daemon/init.c.orig oprofile-0.9.4/daemon/init.c >> diff -up oprofile-0.9.4/daemon/opd_perfmon.c.orig oprofile-0.9.4/daemon/opd_perfmon.c >> --- oprofile-0.9.4/daemon/opd_perfmon.c.orig 2009-09-01 10:45:52.000000000 -0400 >> +++ oprofile-0.9.4/daemon/opd_perfmon.c 2009-09-01 11:42:19.000000000 -0400 >> @@ -141,7 +141,6 @@ static void child_sigusr2(int val __attr >> >> static void child_sigterm(int val __attribute__((unused))) >> { >> - printf("Child received SIGTERM, killing parent.\n"); >> kill(getppid(), SIGTERM); >> } >> >> @@ -156,8 +155,7 @@ static void set_affinity(size_t cpu) >> int err = sched_setaffinity(getpid(), sizeof(set), &set); >> >> if (err == -1) { >> - fprintf(stderr, "Failed to set affinity: %s\n", >> - strerror(errno)); >> + perror("Failed to set affinity: "); > Just a nit, but the ":" is redundant, since perror will add the semicolon to the end of the message you pass it. >> exit(EXIT_FAILURE); >> } >> } >> @@ -214,8 +212,7 @@ static void create_context(struct child >> >> err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); >> if (err == -1) { >> - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", >> - strerror(errno)); >> + perror("CREATE_CONTEXT failed: "); >> exit(EXIT_FAILURE); >> } >> >> @@ -304,8 +301,7 @@ static void notify_parent(struct child * >> if (ret == sizeof(size_t)) >> break; >> if (ret < 0 && errno != EINTR) { >> - fprintf(stderr, "Failed to write child pipe with %s\n", >> - strerror(errno)); >> + perror("Failed to write child pipe with "); >> exit(EXIT_FAILURE); >> } >> } >> @@ -333,6 +329,11 @@ static void run_child(size_t cpu) >> >> notify_parent(self, cpu); >> >> + /* Close IO to allow this child process to be run as a daemon */ >> + close(0); >> + close(1); >> + close(2); >> + >> for (;;) { >> sigset_t sigmask; >> sigfillset(&sigmask); >> @@ -341,15 +342,11 @@ static void run_child(size_t cpu) >> sigdelset(&sigmask, SIGTERM); >> >> if (self->sigusr1) { >> - printf("PFM_START on CPU%d\n", (int)cpu); >> - fflush(stdout); >> perfmon_start_child(self->ctx_fd); > Maybe I'm missing something here, but if there's a problem running perfmon_start_child, perror is called; however, at this point stderr is closed so the user gets no message displayed anywhere that they can see. Should stderr be redirected to a log? > > -Maynard >> self->sigusr1 = 0; >> } >> >> if (self->sigusr2) { >> - printf("PFM_STOP on CPU%d\n", (int)cpu); >> - fflush(stdout); >> perfmon_stop_child(self->ctx_fd); >> self->sigusr2 = 0; >> } >> @@ -368,13 +365,10 @@ static void wait_for_child(struct child >> if (ret == sizeof(size_t)) >> break; >> if (ret < 0 && errno != EINTR) { >> - fprintf(stderr, "Failed to read child pipe with %s\n", >> - strerror(errno)); >> + perror("Failed to read child pipe with "); >> exit(EXIT_FAILURE); >> } >> } >> - printf("Perfmon child up on CPU%d\n", (int)tmp); >> - fflush(stdout); >> >> close(child->up_pipe[0]); >> close(child->up_pipe[1]); >> @@ -422,21 +416,18 @@ void perfmon_init(void) >> int ret; >> >> if (pipe(children[i].up_pipe)) { >> - perror("Couldn't create child pipe.\n"); >> + perror("Couldn't create child pipe: "); >> exit(EXIT_FAILURE); >> } >> >> ret = fork(); >> if (ret == -1) { >> - fprintf(stderr, "Couldn't fork perfmon child.\n"); >> + perror("Couldn't fork perfmon child: "); >> exit(EXIT_FAILURE); >> } else if (ret == 0) { >> - printf("Running perfmon child on CPU%d.\n", (int)i); >> - fflush(stdout); >> run_child(i); >> } else { >> children[i].pid = ret; >> - printf("Waiting on CPU%d\n", (int)i); >> wait_for_child(&children[i]); >> } >> } >> >> > >> >> ------------------------------------------------------------------------ >> >> ------------------------------------------------------------------------------ >> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day >> trial. Simplify your report design, integration and deployment - and focus on >> what you do best, core application coding. Discover what's new with >> Crystal Reports now. http://p.sf.net/sfu/bobj-july >> >> >> ------------------------------------------------------------------------ >> >> _______________________________________________ >> oprofile-list mailing list >> oprofile-list@... >> https://lists.sourceforge.net/lists/listinfo/oprofile-list > > > ------------------------------------------------------------------------------ > Come build with us! The BlackBerry® Developer Conference in SF, CA > is the only developer event you need to attend this year. Jumpstart your > developing skills, take BlackBerry mobile applications to market and stay > ahead of the curve. Join us from November 9-12, 2009. Register now! > http://p.sf.net/sfu/devconf > _______________________________________________ > oprofile-list mailing list > oprofile-list@... > https://lists.sourceforge.net/lists/listinfo/oprofile-list ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by William Cohen
::
Rate this Message:
Maynard Johnson wrote:
> William Cohen wrote: >> The ia64 handles things a bit differently than other architectures. It starts up >> a number of children processes to control perfmon on each of the processors in >> the machine. However, these children processes do not close the io. The results >> in children processes still being connected to the console. There are more >> details about this problem at: >> >> https://bugzilla.redhat.com/show_bug.cgi?id=518480 >> >> I have made a pass at a patch to fix this particular problem and I am looking >> for feedback on the patch: > I've never used the perfmon version of oprofile, so my feedback is probably not that helpful. But I'll give you my 2 cents worth anyway. :-) Hi Maynard, Found this earlier email and commented on it rather than the resent one. Thanks for looking over the patch. I know that few people use the ia64, but fixing this will make people that have scripts to test things out on all the architectures happy. Your comments are helpful. >> 2009-09-01 William Cohen <wcohen@...> >> >> * daemon/opd_perfmon.c: Fix ia64 output to allow spawning daemon. >> >> >> Signed-off-by: William Cohen <wcohen@...> >> >> -Will >> >> >> >> diff -up oprofile-0.9.4/daemon/init.c.orig oprofile-0.9.4/daemon/init.c >> diff -up oprofile-0.9.4/daemon/opd_perfmon.c.orig oprofile-0.9.4/daemon/opd_perfmon.c >> --- oprofile-0.9.4/daemon/opd_perfmon.c.orig 2009-09-01 10:45:52.000000000 -0400 >> +++ oprofile-0.9.4/daemon/opd_perfmon.c 2009-09-01 11:42:19.000000000 -0400 >> @@ -141,7 +141,6 @@ static void child_sigusr2(int val __attr >> >> static void child_sigterm(int val __attribute__((unused))) >> { >> - printf("Child received SIGTERM, killing parent.\n"); >> kill(getppid(), SIGTERM); >> } >> >> @@ -156,8 +155,7 @@ static void set_affinity(size_t cpu) >> int err = sched_setaffinity(getpid(), sizeof(set), &set); >> >> if (err == -1) { >> - fprintf(stderr, "Failed to set affinity: %s\n", >> - strerror(errno)); >> + perror("Failed to set affinity: "); > Just a nit, but the ":" is redundant, since perror will add the semicolon to the end of the message you pass it. Okay, fixed the perror() in the patch. >> exit(EXIT_FAILURE); >> } >> } >> @@ -214,8 +212,7 @@ static void create_context(struct child >> >> err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); >> if (err == -1) { >> - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", >> - strerror(errno)); >> + perror("CREATE_CONTEXT failed: "); >> exit(EXIT_FAILURE); >> } >> >> @@ -304,8 +301,7 @@ static void notify_parent(struct child * >> if (ret == sizeof(size_t)) >> break; >> if (ret < 0 && errno != EINTR) { >> - fprintf(stderr, "Failed to write child pipe with %s\n", >> - strerror(errno)); >> + perror("Failed to write child pipe with "); >> exit(EXIT_FAILURE); >> } >> } >> @@ -333,6 +329,11 @@ static void run_child(size_t cpu) >> >> notify_parent(self, cpu); >> >> + /* Close IO to allow this child process to be run as a daemon */ >> + close(0); >> + close(1); >> + close(2); >> + >> for (;;) { >> sigset_t sigmask; >> sigfillset(&sigmask); >> @@ -341,15 +342,11 @@ static void run_child(size_t cpu) >> sigdelset(&sigmask, SIGTERM); >> >> if (self->sigusr1) { >> - printf("PFM_START on CPU%d\n", (int)cpu); >> - fflush(stdout); >> perfmon_start_child(self->ctx_fd); > Maybe I'm missing something here, but if there's a problem running perfmon_start_child, perror is called; however, at this point stderr is closed so the user gets no message displayed anywhere that they can see. Should stderr be redirected to a log? > Hmmm, you are probably right here. I will take a look at this today and send a revised patch. -Will > -Maynard >> self->sigusr1 = 0; >> } >> >> if (self->sigusr2) { >> - printf("PFM_STOP on CPU%d\n", (int)cpu); >> - fflush(stdout); >> perfmon_stop_child(self->ctx_fd); >> self->sigusr2 = 0; >> } >> @@ -368,13 +365,10 @@ static void wait_for_child(struct child >> if (ret == sizeof(size_t)) >> break; >> if (ret < 0 && errno != EINTR) { >> - fprintf(stderr, "Failed to read child pipe with %s\n", >> - strerror(errno)); >> + perror("Failed to read child pipe with "); >> exit(EXIT_FAILURE); >> } >> } >> - printf("Perfmon child up on CPU%d\n", (int)tmp); >> - fflush(stdout); >> >> close(child->up_pipe[0]); >> close(child->up_pipe[1]); >> @@ -422,21 +416,18 @@ void perfmon_init(void) >> int ret; >> >> if (pipe(children[i].up_pipe)) { >> - perror("Couldn't create child pipe.\n"); >> + perror("Couldn't create child pipe: "); >> exit(EXIT_FAILURE); >> } >> >> ret = fork(); >> if (ret == -1) { >> - fprintf(stderr, "Couldn't fork perfmon child.\n"); >> + perror("Couldn't fork perfmon child: "); >> exit(EXIT_FAILURE); >> } else if (ret == 0) { >> - printf("Running perfmon child on CPU%d.\n", (int)i); >> - fflush(stdout); >> run_child(i); >> } else { >> children[i].pid = ret; >> - printf("Waiting on CPU%d\n", (int)i); >> wait_for_child(&children[i]); >> } >> } >> >> > >> >> ------------------------------------------------------------------------ >> >> ------------------------------------------------------------------------------ >> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day >> trial. Simplify your report design, integration and deployment - and focus on >> what you do best, core application coding. Discover what's new with >> Crystal Reports now. http://p.sf.net/sfu/bobj-july >> >> >> ------------------------------------------------------------------------ >> >> _______________________________________________ >> oprofile-list mailing list >> oprofile-list@... >> https://lists.sourceforge.net/lists/listinfo/oprofile-list > ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by Maynard Johnson
::
Rate this Message:
William Cohen wrote:
> Maynard Johnson wrote: >> William Cohen wrote: >>> The ia64 handles things a bit differently than other architectures. It starts up >>> a number of children processes to control perfmon on each of the processors in >>> the machine. However, these children processes do not close the io. The results >>> in children processes still being connected to the console. There are more >>> details about this problem at: Will, I'm getting pretty close to being ready to put out a 0.9.6 bug-fix release. If you'd like this fix included, please send me a revised patch as soon as you can. Thanks. -Maynard >>> >>> https://bugzilla.redhat.com/show_bug.cgi?id=518480 >>> >>> I have made a pass at a patch to fix this particular problem and I am looking >>> for feedback on the patch: >> I've never used the perfmon version of oprofile, so my feedback is probably not that helpful. But I'll give you my 2 cents worth anyway. :-) > > Hi Maynard, > > Found this earlier email and commented on it rather than the resent one. Thanks > for looking over the patch. I know that few people use the ia64, but fixing this > will make people that have scripts to test things out on all the architectures > happy. Your comments are helpful. > >>> 2009-09-01 William Cohen <wcohen@...> >>> >>> * daemon/opd_perfmon.c: Fix ia64 output to allow spawning daemon. >>> >>> >>> Signed-off-by: William Cohen <wcohen@...> >>> >>> -Will >>> >>> >>> >>> diff -up oprofile-0.9.4/daemon/init.c.orig oprofile-0.9.4/daemon/init.c >>> diff -up oprofile-0.9.4/daemon/opd_perfmon.c.orig oprofile-0.9.4/daemon/opd_perfmon.c >>> --- oprofile-0.9.4/daemon/opd_perfmon.c.orig 2009-09-01 10:45:52.000000000 -0400 >>> +++ oprofile-0.9.4/daemon/opd_perfmon.c 2009-09-01 11:42:19.000000000 -0400 >>> @@ -141,7 +141,6 @@ static void child_sigusr2(int val __attr >>> >>> static void child_sigterm(int val __attribute__((unused))) >>> { >>> - printf("Child received SIGTERM, killing parent.\n"); >>> kill(getppid(), SIGTERM); >>> } >>> >>> @@ -156,8 +155,7 @@ static void set_affinity(size_t cpu) >>> int err = sched_setaffinity(getpid(), sizeof(set), &set); >>> >>> if (err == -1) { >>> - fprintf(stderr, "Failed to set affinity: %s\n", >>> - strerror(errno)); >>> + perror("Failed to set affinity: "); >> Just a nit, but the ":" is redundant, since perror will add the semicolon to the end of the message you pass it. > > Okay, fixed the perror() in the patch. > >>> exit(EXIT_FAILURE); >>> } >>> } >>> @@ -214,8 +212,7 @@ static void create_context(struct child >>> >>> err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); >>> if (err == -1) { >>> - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", >>> - strerror(errno)); >>> + perror("CREATE_CONTEXT failed: "); >>> exit(EXIT_FAILURE); >>> } >>> >>> @@ -304,8 +301,7 @@ static void notify_parent(struct child * >>> if (ret == sizeof(size_t)) >>> break; >>> if (ret < 0 && errno != EINTR) { >>> - fprintf(stderr, "Failed to write child pipe with %s\n", >>> - strerror(errno)); >>> + perror("Failed to write child pipe with "); >>> exit(EXIT_FAILURE); >>> } >>> } >>> @@ -333,6 +329,11 @@ static void run_child(size_t cpu) >>> >>> notify_parent(self, cpu); >>> >>> + /* Close IO to allow this child process to be run as a daemon */ >>> + close(0); >>> + close(1); >>> + close(2); >>> + >>> for (;;) { >>> sigset_t sigmask; >>> sigfillset(&sigmask); >>> @@ -341,15 +342,11 @@ static void run_child(size_t cpu) >>> sigdelset(&sigmask, SIGTERM); >>> >>> if (self->sigusr1) { >>> - printf("PFM_START on CPU%d\n", (int)cpu); >>> - fflush(stdout); >>> perfmon_start_child(self->ctx_fd); >> Maybe I'm missing something here, but if there's a problem running perfmon_start_child, perror is called; however, at this point stderr is closed so the user gets no message displayed anywhere that they can see. Should stderr be redirected to a log? >> > > Hmmm, you are probably right here. I will take a look at this today and send a > revised patch. > > -Will > >> -Maynard >>> self->sigusr1 = 0; >>> } >>> >>> if (self->sigusr2) { >>> - printf("PFM_STOP on CPU%d\n", (int)cpu); >>> - fflush(stdout); >>> perfmon_stop_child(self->ctx_fd); >>> self->sigusr2 = 0; >>> } >>> @@ -368,13 +365,10 @@ static void wait_for_child(struct child >>> if (ret == sizeof(size_t)) >>> break; >>> if (ret < 0 && errno != EINTR) { >>> - fprintf(stderr, "Failed to read child pipe with %s\n", >>> - strerror(errno)); >>> + perror("Failed to read child pipe with "); >>> exit(EXIT_FAILURE); >>> } >>> } >>> - printf("Perfmon child up on CPU%d\n", (int)tmp); >>> - fflush(stdout); >>> >>> close(child->up_pipe[0]); >>> close(child->up_pipe[1]); >>> @@ -422,21 +416,18 @@ void perfmon_init(void) >>> int ret; >>> >>> if (pipe(children[i].up_pipe)) { >>> - perror("Couldn't create child pipe.\n"); >>> + perror("Couldn't create child pipe: "); >>> exit(EXIT_FAILURE); >>> } >>> >>> ret = fork(); >>> if (ret == -1) { >>> - fprintf(stderr, "Couldn't fork perfmon child.\n"); >>> + perror("Couldn't fork perfmon child: "); >>> exit(EXIT_FAILURE); >>> } else if (ret == 0) { >>> - printf("Running perfmon child on CPU%d.\n", (int)i); >>> - fflush(stdout); >>> run_child(i); >>> } else { >>> children[i].pid = ret; >>> - printf("Waiting on CPU%d\n", (int)i); >>> wait_for_child(&children[i]); >>> } >>> } >>> >>> >>> ------------------------------------------------------------------------ >>> >>> ------------------------------------------------------------------------------ >>> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day >>> trial. Simplify your report design, integration and deployment - and focus on >>> what you do best, core application coding. Discover what's new with >>> Crystal Reports now. http://p.sf.net/sfu/bobj-july >>> >>> >>> ------------------------------------------------------------------------ >>> >>> _______________________________________________ >>> oprofile-list mailing list >>> oprofile-list@... >>> https://lists.sourceforge.net/lists/listinfo/oprofile-list > ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by William Cohen
::
Rate this Message:
Maynard Johnson wrote:
> William Cohen wrote: >> Maynard Johnson wrote: >>> William Cohen wrote: >>>> The ia64 handles things a bit differently than other architectures. It starts up >>>> a number of children processes to control perfmon on each of the processors in >>>> the machine. However, these children processes do not close the io. The results >>>> in children processes still being connected to the console. There are more >>>> details about this problem at: > Will, I'm getting pretty close to being ready to put out a 0.9.6 bug-fix release. If you'd like this fix included, please send me a revised patch as soon as you can. > > Thanks. > -Maynard I made another pass on this. Signed-off-by: William Cohen <wcohen@...> This allows the daemon to be fully in the background and is a step in the right direction. However, I noticed that the parent process can still be hung in parent process' wait_for_child() read() when the child exits before communicating in the pipe. Any suggestion on how to handle the case where the child exits before communicating on the pipe would be appreciated. -Will ? compile ? ltmain.sh ? daemon/opd_perfmon.c.new ? daemon/opd_perfmon.i ? daemon/opd_perfmon.s Index: daemon/opd_perfmon.c =================================================================== RCS file: /cvsroot/oprofile/oprofile/daemon/opd_perfmon.c,v retrieving revision 1.20 diff -u -r1.20 opd_perfmon.c --- daemon/opd_perfmon.c 11 Jan 2008 17:49:24 -0000 1.20 +++ daemon/opd_perfmon.c 29 Oct 2009 01:44:26 -0000 @@ -30,6 +30,8 @@ #include <stdlib.h> #include <string.h> #include <errno.h> +#include <sys/types.h> +#include <sys/stat.h> #ifdef HAVE_SCHED_SETAFFINITY #include <sched.h> #endif @@ -141,7 +143,6 @@ static void child_sigterm(int val __attribute__((unused))) { - printf("Child received SIGTERM, killing parent.\n"); kill(getppid(), SIGTERM); } @@ -149,15 +150,15 @@ static void set_affinity(size_t cpu) { cpu_set_t set; + int err; CPU_ZERO(&set); CPU_SET(cpu, &set); - int err = sched_setaffinity(getpid(), sizeof(set), &set); + err = sched_setaffinity(getpid(), sizeof(set), &set); if (err == -1) { - fprintf(stderr, "Failed to set affinity: %s\n", - strerror(errno)); + perror("Failed to set affinity"); exit(EXIT_FAILURE); } } @@ -178,7 +179,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGUSR1, &act, NULL)) { - perror("oprofiled: install of SIGUSR1 handler failed: "); + perror("oprofiled: install of SIGUSR1 handler failed"); exit(EXIT_FAILURE); } @@ -187,7 +188,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGUSR2, &act, NULL)) { - perror("oprofiled: install of SIGUSR2 handler failed: "); + perror("oprofiled: install of SIGUSR2 handler failed"); exit(EXIT_FAILURE); } @@ -196,7 +197,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGTERM, &act, NULL)) { - perror("oprofiled: install of SIGTERM handler failed: "); + perror("oprofiled: install of SIGTERM handler failed"); exit(EXIT_FAILURE); } } @@ -214,8 +215,7 @@ err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); if (err == -1) { - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", - strerror(errno)); + perror("CREATE_CONTEXT failed"); exit(EXIT_FAILURE); } @@ -268,13 +268,13 @@ err = perfmonctl(self->ctx_fd, PFM_WRITE_PMCS, pc, i); if (err == -1) { - perror("Couldn't write PMCs: "); + perror("Couldn't write PMCs"); exit(EXIT_FAILURE); } err = perfmonctl(self->ctx_fd, PFM_WRITE_PMDS, pd, i); if (err == -1) { - perror("Couldn't write PMDs: "); + perror("Couldn't write PMDs"); exit(EXIT_FAILURE); } } @@ -290,7 +290,7 @@ err = perfmonctl(self->ctx_fd, PFM_LOAD_CONTEXT, &load_args, 1); if (err == -1) { - perror("Couldn't load context: "); + perror("Couldn't load context"); exit(EXIT_FAILURE); } } @@ -304,8 +304,7 @@ if (ret == sizeof(size_t)) break; if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to write child pipe with %s\n", - strerror(errno)); + perror("Failed to write child pipe:"); exit(EXIT_FAILURE); } } @@ -321,6 +320,13 @@ self->sigusr2 = 0; self->sigterm = 0; + umask(0); + /* Change directory to allow directory to be removed */ + if (chdir("/") < 0) { + perror("Unable to chdir to \"/\""); + exit(EXIT_FAILURE); + } + setup_signals(); set_affinity(cpu); @@ -333,6 +339,11 @@ notify_parent(self, cpu); + /* Redirect standard files to /dev/null */ + freopen( "/dev/null", "r", stdin); + freopen( "/dev/null", "w", stdout); + freopen( "/dev/null", "w", stderr); + for (;;) { sigset_t sigmask; sigfillset(&sigmask); @@ -341,15 +352,11 @@ sigdelset(&sigmask, SIGTERM); if (self->sigusr1) { - printf("PFM_START on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_start_child(self->ctx_fd); self->sigusr1 = 0; } if (self->sigusr2) { - printf("PFM_STOP on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_stop_child(self->ctx_fd); self->sigusr2 = 0; } @@ -368,8 +375,7 @@ if (ret == sizeof(size_t)) break; if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to read child pipe with %s\n", - strerror(errno)); + perror("Failed to read child pipe"); exit(EXIT_FAILURE); } } @@ -422,17 +428,15 @@ int ret; if (pipe(children[i].up_pipe)) { - perror("Couldn't create child pipe.\n"); + perror("Couldn't create child pipe."); exit(EXIT_FAILURE); } ret = fork(); if (ret == -1) { - fprintf(stderr, "Couldn't fork perfmon child.\n"); + perror("Couldn't fork perfmon child."); exit(EXIT_FAILURE); } else if (ret == 0) { - printf("Running perfmon child on CPU%d.\n", (int)i); - fflush(stdout); run_child(i); } else { children[i].pid = ret; ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by William Cohen
::
Rate this Message:
William Cohen wrote:
> Maynard Johnson wrote: >> William Cohen wrote: >>> Maynard Johnson wrote: >>>> William Cohen wrote: >>>>> The ia64 handles things a bit differently than other architectures. It starts up >>>>> a number of children processes to control perfmon on each of the processors in >>>>> the machine. However, these children processes do not close the io. The results >>>>> in children processes still being connected to the console. There are more >>>>> details about this problem at: >> Will, I'm getting pretty close to being ready to put out a 0.9.6 bug-fix release. If you'd like this fix included, please send me a revised patch as soon as you can. >> >> Thanks. >> -Maynard > > Hi Maynard, > > I made another pass on this. > > Signed-off-by: William Cohen <wcohen@...> > > This allows the daemon to be fully in the background and is a step in the right > direction. However, I noticed that the parent process can still be hung in > parent process' wait_for_child() read() when the child exits before > communicating in the pipe. Any suggestion on how to handle the case where the > child exits before communicating on the pipe would be appreciated. > > -Will ? compile ? ltmain.sh ? daemon/opd_perfmon.c.new ? daemon/opd_perfmon.i ? daemon/opd_perfmon.s Index: daemon/opd_perfmon.c =================================================================== RCS file: /cvsroot/oprofile/oprofile/daemon/opd_perfmon.c,v retrieving revision 1.20 diff -u -r1.20 opd_perfmon.c --- daemon/opd_perfmon.c 11 Jan 2008 17:49:24 -0000 1.20 +++ daemon/opd_perfmon.c 29 Oct 2009 04:08:16 -0000 @@ -30,6 +30,8 @@ #include <stdlib.h> #include <string.h> #include <errno.h> +#include <sys/types.h> +#include <sys/stat.h> #ifdef HAVE_SCHED_SETAFFINITY #include <sched.h> #endif @@ -98,7 +100,6 @@ static void perfmon_start_child(int ctx_fd) { if (perfmonctl(ctx_fd, PFM_START, 0, 0) == -1) { - perror("Couldn't start perfmon: "); exit(EXIT_FAILURE); } } @@ -107,7 +108,6 @@ static void perfmon_stop_child(int ctx_fd) { if (perfmonctl(ctx_fd, PFM_STOP, 0, 0) == -1) { - perror("Couldn't stop perfmon: "); exit(EXIT_FAILURE); } } @@ -141,7 +141,6 @@ static void child_sigterm(int val __attribute__((unused))) { - printf("Child received SIGTERM, killing parent.\n"); kill(getppid(), SIGTERM); } @@ -149,15 +148,15 @@ static void set_affinity(size_t cpu) { cpu_set_t set; + int err; CPU_ZERO(&set); CPU_SET(cpu, &set); - int err = sched_setaffinity(getpid(), sizeof(set), &set); + err = sched_setaffinity(getpid(), sizeof(set), &set); if (err == -1) { - fprintf(stderr, "Failed to set affinity: %s\n", - strerror(errno)); + perror("Failed to set affinity"); exit(EXIT_FAILURE); } } @@ -178,7 +177,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGUSR1, &act, NULL)) { - perror("oprofiled: install of SIGUSR1 handler failed: "); + perror("oprofiled: install of SIGUSR1 handler failed"); exit(EXIT_FAILURE); } @@ -187,7 +186,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGUSR2, &act, NULL)) { - perror("oprofiled: install of SIGUSR2 handler failed: "); + perror("oprofiled: install of SIGUSR2 handler failed"); exit(EXIT_FAILURE); } @@ -196,7 +195,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGTERM, &act, NULL)) { - perror("oprofiled: install of SIGTERM handler failed: "); + perror("oprofiled: install of SIGTERM handler failed"); exit(EXIT_FAILURE); } } @@ -214,8 +213,7 @@ err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); if (err == -1) { - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", - strerror(errno)); + perror("CREATE_CONTEXT failed"); exit(EXIT_FAILURE); } @@ -268,13 +266,13 @@ err = perfmonctl(self->ctx_fd, PFM_WRITE_PMCS, pc, i); if (err == -1) { - perror("Couldn't write PMCs: "); + perror("Couldn't write PMCs"); exit(EXIT_FAILURE); } err = perfmonctl(self->ctx_fd, PFM_WRITE_PMDS, pd, i); if (err == -1) { - perror("Couldn't write PMDs: "); + perror("Couldn't write PMDs"); exit(EXIT_FAILURE); } } @@ -290,7 +288,7 @@ err = perfmonctl(self->ctx_fd, PFM_LOAD_CONTEXT, &load_args, 1); if (err == -1) { - perror("Couldn't load context: "); + perror("Couldn't load context"); exit(EXIT_FAILURE); } } @@ -304,8 +302,7 @@ if (ret == sizeof(size_t)) break; if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to write child pipe with %s\n", - strerror(errno)); + perror("Failed to write child pipe:"); exit(EXIT_FAILURE); } } @@ -321,6 +318,13 @@ self->sigusr2 = 0; self->sigterm = 0; + umask(0); + /* Change directory to allow directory to be removed */ + if (chdir("/") < 0) { + perror("Unable to chdir to \"/\""); + exit(EXIT_FAILURE); + } + setup_signals(); set_affinity(cpu); @@ -333,6 +337,11 @@ notify_parent(self, cpu); + /* Redirect standard files to /dev/null */ + freopen( "/dev/null", "r", stdin); + freopen( "/dev/null", "w", stdout); + freopen( "/dev/null", "w", stderr); + for (;;) { sigset_t sigmask; sigfillset(&sigmask); @@ -341,15 +350,11 @@ sigdelset(&sigmask, SIGTERM); if (self->sigusr1) { - printf("PFM_START on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_start_child(self->ctx_fd); self->sigusr1 = 0; } if (self->sigusr2) { - printf("PFM_STOP on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_stop_child(self->ctx_fd); self->sigusr2 = 0; } @@ -368,8 +373,7 @@ if (ret == sizeof(size_t)) break; if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to read child pipe with %s\n", - strerror(errno)); + perror("Failed to read child pipe"); exit(EXIT_FAILURE); } } @@ -422,17 +426,15 @@ int ret; if (pipe(children[i].up_pipe)) { - perror("Couldn't create child pipe.\n"); + perror("Couldn't create child pipe."); exit(EXIT_FAILURE); } ret = fork(); if (ret == -1) { - fprintf(stderr, "Couldn't fork perfmon child.\n"); + perror("Couldn't fork perfmon child."); exit(EXIT_FAILURE); } else if (ret == 0) { - printf("Running perfmon child on CPU%d.\n", (int)i); - fflush(stdout); run_child(i); } else { children[i].pid = ret; ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by William Cohen
::
Rate this Message:
William Cohen wrote:
> William Cohen wrote: >> Maynard Johnson wrote: >>> William Cohen wrote: >>>> Maynard Johnson wrote: >>>>> William Cohen wrote: >>>>>> The ia64 handles things a bit differently than other architectures. It starts up >>>>>> a number of children processes to control perfmon on each of the processors in >>>>>> the machine. However, these children processes do not close the io. The results >>>>>> in children processes still being connected to the console. There are more >>>>>> details about this problem at: >>> Will, I'm getting pretty close to being ready to put out a 0.9.6 bug-fix release. If you'd like this fix included, please send me a revised patch as soon as you can. >>> >>> Thanks. >>> -Maynard >> Hi Maynard, >> >> I made another pass on this. >> >> Signed-off-by: William Cohen <wcohen@...> >> >> This allows the daemon to be fully in the background and is a step in the right >> direction. However, I noticed that the parent process can still be hung in >> parent process' wait_for_child() read() when the child exits before >> communicating in the pipe. Any suggestion on how to handle the case where the >> child exits before communicating on the pipe would be appreciated. >> >> -Will > > Slight tweak to the patch to output that can't be seen. -Will I made a further revisions to the patch to correctly handle the case where the child exits by making sure that the pipe is properly closed so that the parent read() doesn't hang. Also make sure that the kill only sent to child pids that actually exist. I have tested to make sure that a new oprofiled doesn't hang if there is an existing oprofiled is running. -Will ? compile ? ltmain.sh ? daemon/opd_perfmon.c.new ? daemon/opd_perfmon.i ? daemon/opd_perfmon.s Index: daemon/opd_perfmon.c =================================================================== RCS file: /cvsroot/oprofile/oprofile/daemon/opd_perfmon.c,v retrieving revision 1.20 diff -u -r1.20 opd_perfmon.c --- daemon/opd_perfmon.c 11 Jan 2008 17:49:24 -0000 1.20 +++ daemon/opd_perfmon.c 29 Oct 2009 15:26:53 -0000 @@ -30,6 +30,8 @@ #include <stdlib.h> #include <string.h> #include <errno.h> +#include <sys/types.h> +#include <sys/stat.h> #ifdef HAVE_SCHED_SETAFFINITY #include <sched.h> #endif @@ -98,7 +100,6 @@ static void perfmon_start_child(int ctx_fd) { if (perfmonctl(ctx_fd, PFM_START, 0, 0) == -1) { - perror("Couldn't start perfmon: "); exit(EXIT_FAILURE); } } @@ -107,7 +108,6 @@ static void perfmon_stop_child(int ctx_fd) { if (perfmonctl(ctx_fd, PFM_STOP, 0, 0) == -1) { - perror("Couldn't stop perfmon: "); exit(EXIT_FAILURE); } } @@ -141,7 +141,6 @@ static void child_sigterm(int val __attribute__((unused))) { - printf("Child received SIGTERM, killing parent.\n"); kill(getppid(), SIGTERM); } @@ -149,15 +148,15 @@ static void set_affinity(size_t cpu) { cpu_set_t set; + int err; CPU_ZERO(&set); CPU_SET(cpu, &set); - int err = sched_setaffinity(getpid(), sizeof(set), &set); + err = sched_setaffinity(getpid(), sizeof(set), &set); if (err == -1) { - fprintf(stderr, "Failed to set affinity: %s\n", - strerror(errno)); + perror("Failed to set affinity"); exit(EXIT_FAILURE); } } @@ -178,7 +177,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGUSR1, &act, NULL)) { - perror("oprofiled: install of SIGUSR1 handler failed: "); + perror("oprofiled: install of SIGUSR1 handler failed"); exit(EXIT_FAILURE); } @@ -187,7 +186,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGUSR2, &act, NULL)) { - perror("oprofiled: install of SIGUSR2 handler failed: "); + perror("oprofiled: install of SIGUSR2 handler failed"); exit(EXIT_FAILURE); } @@ -196,7 +195,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGTERM, &act, NULL)) { - perror("oprofiled: install of SIGTERM handler failed: "); + perror("oprofiled: install of SIGTERM handler failed"); exit(EXIT_FAILURE); } } @@ -214,8 +213,7 @@ err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); if (err == -1) { - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", - strerror(errno)); + perror("CREATE_CONTEXT failed"); exit(EXIT_FAILURE); } @@ -268,13 +266,13 @@ err = perfmonctl(self->ctx_fd, PFM_WRITE_PMCS, pc, i); if (err == -1) { - perror("Couldn't write PMCs: "); + perror("Couldn't write PMCs"); exit(EXIT_FAILURE); } err = perfmonctl(self->ctx_fd, PFM_WRITE_PMDS, pd, i); if (err == -1) { - perror("Couldn't write PMDs: "); + perror("Couldn't write PMDs"); exit(EXIT_FAILURE); } } @@ -290,7 +288,7 @@ err = perfmonctl(self->ctx_fd, PFM_LOAD_CONTEXT, &load_args, 1); if (err == -1) { - perror("Couldn't load context: "); + perror("Couldn't load context"); exit(EXIT_FAILURE); } } @@ -304,13 +302,17 @@ if (ret == sizeof(size_t)) break; if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to write child pipe with %s\n", - strerror(errno)); + perror("Failed to write child pipe:"); exit(EXIT_FAILURE); } } } +static struct child * inner_child; +void close_pipe(void) +{ + close(inner_child->up_pipe[1]); +} static void run_child(size_t cpu) { @@ -321,6 +323,19 @@ self->sigusr2 = 0; self->sigterm = 0; + inner_child = self; + if (atexit(close_pipe)){ + close_pipe(); + exit(EXIT_FAILURE); + } + + umask(0); + /* Change directory to allow directory to be removed */ + if (chdir("/") < 0) { + perror("Unable to chdir to \"/\""); + exit(EXIT_FAILURE); + } + setup_signals(); set_affinity(cpu); @@ -333,6 +348,11 @@ notify_parent(self, cpu); + /* Redirect standard files to /dev/null */ + freopen( "/dev/null", "r", stdin); + freopen( "/dev/null", "w", stdout); + freopen( "/dev/null", "w", stderr); + for (;;) { sigset_t sigmask; sigfillset(&sigmask); @@ -341,15 +361,11 @@ sigdelset(&sigmask, SIGTERM); if (self->sigusr1) { - printf("PFM_START on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_start_child(self->ctx_fd); self->sigusr1 = 0; } if (self->sigusr2) { - printf("PFM_STOP on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_stop_child(self->ctx_fd); self->sigusr2 = 0; } @@ -367,9 +383,8 @@ ret = read(child->up_pipe[0], &tmp, sizeof(size_t)); if (ret == sizeof(size_t)) break; - if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to read child pipe with %s\n", - strerror(errno)); + if ((ret < 0 && errno != EINTR) || ret == 0 ) { + perror("Failed to read child pipe"); exit(EXIT_FAILURE); } } @@ -377,7 +392,6 @@ fflush(stdout); close(child->up_pipe[0]); - close(child->up_pipe[1]); } static struct child* xen_ctx; @@ -417,25 +431,26 @@ nr_cpus = nr; children = xmalloc(sizeof(struct child) * nr_cpus); + bzero(children, sizeof(struct child) * nr_cpus); for (i = 0; i < nr_cpus; ++i) { int ret; if (pipe(children[i].up_pipe)) { - perror("Couldn't create child pipe.\n"); + perror("Couldn't create child pipe"); exit(EXIT_FAILURE); } ret = fork(); if (ret == -1) { - fprintf(stderr, "Couldn't fork perfmon child.\n"); + perror("Couldn't fork perfmon child"); exit(EXIT_FAILURE); } else if (ret == 0) { - printf("Running perfmon child on CPU%d.\n", (int)i); - fflush(stdout); + close(children[i].up_pipe[0]); run_child(i); } else { children[i].pid = ret; + close(children[i].up_pipe[1]); printf("Waiting on CPU%d\n", (int)i); wait_for_child(&children[i]); } @@ -454,8 +469,12 @@ return; for (i = 0; i < nr_cpus; ++i) { - kill(children[i].pid, SIGKILL); - waitpid(children[i].pid, NULL, 0); + if (children[i].pid) { + int c_pid = children[i].pid; + children[i].pid = 0; + kill(c_pid, SIGKILL); + waitpid(c_pid, NULL, 0); + } } } ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by Maynard Johnson
::
Rate this Message:
William Cohen wrote:
> William Cohen wrote: >> Maynard Johnson wrote: >>> William Cohen wrote: >>>> Maynard Johnson wrote: >>>>> William Cohen wrote: >>>>>> The ia64 handles things a bit differently than other architectures. It starts up >>>>>> a number of children processes to control perfmon on each of the processors in >>>>>> the machine. However, these children processes do not close the io. The results >>>>>> in children processes still being connected to the console. There are more >>>>>> details about this problem at: >>> Will, I'm getting pretty close to being ready to put out a 0.9.6 bug-fix release. If you'd like this fix included, please send me a revised patch as soon as you can. >>> >>> Thanks. >>> -Maynard >> Hi Maynard, >> >> I made another pass on this. >> >> Signed-off-by: William Cohen <wcohen@...> >> >> This allows the daemon to be fully in the background and is a step in the right >> direction. However, I noticed that the parent process can still be hung in >> parent process' wait_for_child() read() when the child exits before >> communicating in the pipe. Any suggestion on how to handle the case where the >> child exits before communicating on the pipe would be appreciated. As far as I can tell, this patch addresses the original issue of the child processes still having open fd's for stdout, etc. One comment I have is that there are two cases where the child may exit with error after std out/err/in are closed, so you get no error information. As a way to address this issue, as well as the potential for the parent hanging in wait_for_child(), you could (and probably should) establish a signal handler for SIGCHLD. The child could exit with a specific error code to indicate the cause of failure, and the handler could interpret the code and print a message to the log. Additionally, the handler could set a per-child variable to indicate the child has exited. In the forever loop in wait_for_child(), you could check that variable and bail out if the child has died. As I mentioned earlier, I want to roll out a bug-fix release, mainly to address the 0.9.5 regressions. I'll be out next week, so I want to put out 0.9.6-rc1 this week yet. So the question to you is do you want this patch included in that bug-fix release? Or would you be able to put together a new, improved patch quickly (i.e., by EOD) that you'd like me to wait for? -Maynard >> >> -Will > > Slight tweak to the patch to output that can't be seen. -Will > > Index: daemon/opd_perfmon.c > =================================================================== > RCS file: /cvsroot/oprofile/oprofile/daemon/opd_perfmon.c,v > retrieving revision 1.20 > diff -u -r1.20 opd_perfmon.c > --- daemon/opd_perfmon.c 11 Jan 2008 17:49:24 -0000 1.20 > +++ daemon/opd_perfmon.c 29 Oct 2009 04:08:16 -0000 > @@ -30,6 +30,8 @@ > #include <stdlib.h> > #include <string.h> > #include <errno.h> > +#include <sys/types.h> > +#include <sys/stat.h> > #ifdef HAVE_SCHED_SETAFFINITY > #include <sched.h> > #endif > @@ -98,7 +100,6 @@ > static void perfmon_start_child(int ctx_fd) > { > if (perfmonctl(ctx_fd, PFM_START, 0, 0) == -1) { > - perror("Couldn't start perfmon: "); > exit(EXIT_FAILURE); > } > } > @@ -107,7 +108,6 @@ > static void perfmon_stop_child(int ctx_fd) > { > if (perfmonctl(ctx_fd, PFM_STOP, 0, 0) == -1) { > - perror("Couldn't stop perfmon: "); > exit(EXIT_FAILURE); > } > } > @@ -141,7 +141,6 @@ > > static void child_sigterm(int val __attribute__((unused))) > { > - printf("Child received SIGTERM, killing parent.\n"); > kill(getppid(), SIGTERM); > } > > @@ -149,15 +148,15 @@ > static void set_affinity(size_t cpu) > { > cpu_set_t set; > + int err; > > CPU_ZERO(&set); > CPU_SET(cpu, &set); > > - int err = sched_setaffinity(getpid(), sizeof(set), &set); > + err = sched_setaffinity(getpid(), sizeof(set), &set); > > if (err == -1) { > - fprintf(stderr, "Failed to set affinity: %s\n", > - strerror(errno)); > + perror("Failed to set affinity"); > exit(EXIT_FAILURE); > } > } > @@ -178,7 +177,7 @@ > sigemptyset(&act.sa_mask); > > if (sigaction(SIGUSR1, &act, NULL)) { > - perror("oprofiled: install of SIGUSR1 handler failed: "); > + perror("oprofiled: install of SIGUSR1 handler failed"); > exit(EXIT_FAILURE); > } > > @@ -187,7 +186,7 @@ > sigemptyset(&act.sa_mask); > > if (sigaction(SIGUSR2, &act, NULL)) { > - perror("oprofiled: install of SIGUSR2 handler failed: "); > + perror("oprofiled: install of SIGUSR2 handler failed"); > exit(EXIT_FAILURE); > } > > @@ -196,7 +195,7 @@ > sigemptyset(&act.sa_mask); > > if (sigaction(SIGTERM, &act, NULL)) { > - perror("oprofiled: install of SIGTERM handler failed: "); > + perror("oprofiled: install of SIGTERM handler failed"); > exit(EXIT_FAILURE); > } > } > @@ -214,8 +213,7 @@ > > err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); > if (err == -1) { > - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", > - strerror(errno)); > + perror("CREATE_CONTEXT failed"); > exit(EXIT_FAILURE); > } > > @@ -268,13 +266,13 @@ > > err = perfmonctl(self->ctx_fd, PFM_WRITE_PMCS, pc, i); > if (err == -1) { > - perror("Couldn't write PMCs: "); > + perror("Couldn't write PMCs"); > exit(EXIT_FAILURE); > } > > err = perfmonctl(self->ctx_fd, PFM_WRITE_PMDS, pd, i); > if (err == -1) { > - perror("Couldn't write PMDs: "); > + perror("Couldn't write PMDs"); > exit(EXIT_FAILURE); > } > } > @@ -290,7 +288,7 @@ > > err = perfmonctl(self->ctx_fd, PFM_LOAD_CONTEXT, &load_args, 1); > if (err == -1) { > - perror("Couldn't load context: "); > + perror("Couldn't load context"); > exit(EXIT_FAILURE); > } > } > @@ -304,8 +302,7 @@ > if (ret == sizeof(size_t)) > break; > if (ret < 0 && errno != EINTR) { > - fprintf(stderr, "Failed to write child pipe with %s\n", > - strerror(errno)); > + perror("Failed to write child pipe:"); > exit(EXIT_FAILURE); > } > } > @@ -321,6 +318,13 @@ > self->sigusr2 = 0; > self->sigterm = 0; > > + umask(0); > + /* Change directory to allow directory to be removed */ > + if (chdir("/") < 0) { > + perror("Unable to chdir to \"/\""); > + exit(EXIT_FAILURE); > + } > + > setup_signals(); > > set_affinity(cpu); > @@ -333,6 +337,11 @@ > > notify_parent(self, cpu); > > + /* Redirect standard files to /dev/null */ > + freopen( "/dev/null", "r", stdin); > + freopen( "/dev/null", "w", stdout); > + freopen( "/dev/null", "w", stderr); > + > for (;;) { > sigset_t sigmask; > sigfillset(&sigmask); > @@ -341,15 +350,11 @@ > sigdelset(&sigmask, SIGTERM); > > if (self->sigusr1) { > - printf("PFM_START on CPU%d\n", (int)cpu); > - fflush(stdout); > perfmon_start_child(self->ctx_fd); > self->sigusr1 = 0; > } > > if (self->sigusr2) { > - printf("PFM_STOP on CPU%d\n", (int)cpu); > - fflush(stdout); > perfmon_stop_child(self->ctx_fd); > self->sigusr2 = 0; > } > @@ -368,8 +373,7 @@ > if (ret == sizeof(size_t)) > break; > if (ret < 0 && errno != EINTR) { > - fprintf(stderr, "Failed to read child pipe with %s\n", > - strerror(errno)); > + perror("Failed to read child pipe"); > exit(EXIT_FAILURE); > } > } > @@ -422,17 +426,15 @@ > int ret; > > if (pipe(children[i].up_pipe)) { > - perror("Couldn't create child pipe.\n"); > + perror("Couldn't create child pipe."); > exit(EXIT_FAILURE); > } > > ret = fork(); > if (ret == -1) { > - fprintf(stderr, "Couldn't fork perfmon child.\n"); > + perror("Couldn't fork perfmon child."); > exit(EXIT_FAILURE); > } else if (ret == 0) { > - printf("Running perfmon child on CPU%d.\n", (int)i); > - fflush(stdout); > run_child(i); > } else { > children[i].pid = ret; ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by William Cohen
::
Rate this Message:
Maynard Johnson wrote:
> William Cohen wrote: >> William Cohen wrote: >>> Maynard Johnson wrote: >>>> William Cohen wrote: >>>>> Maynard Johnson wrote: >>>>>> William Cohen wrote: >>>>>>> The ia64 handles things a bit differently than other >>>>>>> architectures. It starts up >>>>>>> a number of children processes to control perfmon on each of the >>>>>>> processors in >>>>>>> the machine. However, these children processes do not close the >>>>>>> io. The results >>>>>>> in children processes still being connected to the console. >>>>>>> There are more >>>>>>> details about this problem at: >>>> Will, I'm getting pretty close to being ready to put out a 0.9.6 >>>> bug-fix release. If you'd like this fix included, please send me a >>>> revised patch as soon as you can. >>>> >>>> Thanks. >>>> -Maynard >>> Hi Maynard, >>> >>> I made another pass on this. >>> >>> Signed-off-by: William Cohen <wcohen@...> >>> >>> This allows the daemon to be fully in the background and is a step in >>> the right >>> direction. However, I noticed that the parent process can still be >>> hung in >>> parent process' wait_for_child() read() when the child exits before >>> communicating in the pipe. Any suggestion on how to handle the case >>> where the >>> child exits before communicating on the pipe would be appreciated. > This is a separate (and not a new) problem, unrelated to this patch -- > right? As far as I can tell, this patch addresses the original issue of > the child processes still having open fd's for stdout, etc. Yes, you could consider there being two separate issues. The original issue was not allowing the daemon to be a daemon. When looking through the code I and testing it that there was the second problem of possible hanging by the parent if the child exits on its own accord. Earlier versions of the patch only address the first problem. This version addresses the second problem making sure that the parent doesn't hang if there is a premature exit of a child process during setup. > One comment > I have is that there are two cases where the child may exit with error > after std out/err/in are closed, so you get no error information. As a > way to address this issue, as well as the potential for the parent > hanging in wait_for_child(), you could (and probably should) establish a > signal handler for SIGCHLD. The child could exit with a specific error > code to indicate the cause of failure, and the handler could interpret > the code and print a message to the log. Additionally, the handler > could set a per-child variable to indicate the child has exited. In the > forever loop in wait_for_child(), you could check that variable and bail > out if the child has died. Thanks for the analysis and the suggestion about using SIGCHLD. wait_for_child doesn't actually loop forever. It just waits for the message back on the pipe from the child process indicating that the child is set up to start and top perfmon. > > As I mentioned earlier, I want to roll out a bug-fix release, mainly to > address the 0.9.5 regressions. I'll be out next week, so I want to put > out 0.9.6-rc1 this week yet. So the question to you is do you want this > patch included in that bug-fix release? Or would you be able to put > together a new, improved patch quickly (i.e., by EOD) that you'd like me > to wait for? > > -Maynard I am not sure that I will be able to get a revised patch in by EOD, so I think I would go with the current patch for 0.9.6-rc1. -Will ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by William Cohen
::
Rate this Message:
William Cohen wrote:
> Maynard Johnson wrote: >> William Cohen wrote: >>> William Cohen wrote: >>>> Maynard Johnson wrote: >>>>> William Cohen wrote: >>>>>> Maynard Johnson wrote: >>>>>>> William Cohen wrote: >>>>>>>> The ia64 handles things a bit differently than other >>>>>>>> architectures. It starts up >>>>>>>> a number of children processes to control perfmon on each of the >>>>>>>> processors in >>>>>>>> the machine. However, these children processes do not close the >>>>>>>> io. The results >>>>>>>> in children processes still being connected to the console. >>>>>>>> There are more >>>>>>>> details about this problem at: >>>>> Will, I'm getting pretty close to being ready to put out a 0.9.6 >>>>> bug-fix release. If you'd like this fix included, please send me a >>>>> revised patch as soon as you can. >>>>> >>>>> Thanks. >>>>> -Maynard >>>> Hi Maynard, >>>> >>>> I made another pass on this. >>>> >>>> Signed-off-by: William Cohen <wcohen@...> >>>> >>>> This allows the daemon to be fully in the background and is a step in >>>> the right >>>> direction. However, I noticed that the parent process can still be >>>> hung in >>>> parent process' wait_for_child() read() when the child exits before >>>> communicating in the pipe. Any suggestion on how to handle the case >>>> where the >>>> child exits before communicating on the pipe would be appreciated. >> This is a separate (and not a new) problem, unrelated to this patch -- >> right? As far as I can tell, this patch addresses the original issue of >> the child processes still having open fd's for stdout, etc. > > Yes, you could consider there being two separate issues. The original issue was > not allowing the daemon to be a daemon. When looking through the code I and > testing it that there was the second problem of possible hanging by the parent > if the child exits on its own accord. Earlier versions of the patch only address > the first problem. This version addresses the second problem making sure that > the parent doesn't hang if there is a premature exit of a child process during > setup. > > >> One comment >> I have is that there are two cases where the child may exit with error >> after std out/err/in are closed, so you get no error information. As a >> way to address this issue, as well as the potential for the parent >> hanging in wait_for_child(), you could (and probably should) establish a >> signal handler for SIGCHLD. The child could exit with a specific error >> code to indicate the cause of failure, and the handler could interpret >> the code and print a message to the log. Additionally, the handler >> could set a per-child variable to indicate the child has exited. In the >> forever loop in wait_for_child(), you could check that variable and bail >> out if the child has died. > > Thanks for the analysis and the suggestion about using SIGCHLD. wait_for_child > doesn't actually loop forever. It just waits for the message back on the pipe > from the child process indicating that the child is set up to start and top perfmon. > > >> As I mentioned earlier, I want to roll out a bug-fix release, mainly to >> address the 0.9.5 regressions. I'll be out next week, so I want to put >> out 0.9.6-rc1 this week yet. So the question to you is do you want this >> patch included in that bug-fix release? Or would you be able to put >> together a new, improved patch quickly (i.e., by EOD) that you'd like me >> to wait for? >> >> -Maynard > > I am not sure that I will be able to get a revised patch in by EOD, so I think I > would go with the current patch for 0.9.6-rc1. > > -Will I am not sure that the SIGCHLD will work in this particular case because the of the daemonization (fork and setsid()) after the creation of the processes to control perfmon on each processor. -Will ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by William Cohen
::
Rate this Message:
William Cohen wrote:
> > I am not sure that the SIGCHLD will work in this particular case because the of > the daemonization (fork and setsid()) after the creation of the processes to > control perfmon on each processor. > > -Will Because the oprofiled daemon is a sibling rather than a parent, I took a different approach than SIGCHLD, The code checks the results of the kill() to determine if a signal was successfully sent. -Will ? compile ? ltmain.sh ? daemon/opd_perfmon.c.new ? daemon/opd_perfmon.i ? daemon/opd_perfmon.s Index: daemon/opd_perfmon.c =================================================================== RCS file: /cvsroot/oprofile/oprofile/daemon/opd_perfmon.c,v retrieving revision 1.20 diff -u -r1.20 opd_perfmon.c --- daemon/opd_perfmon.c 11 Jan 2008 17:49:24 -0000 1.20 +++ daemon/opd_perfmon.c 29 Oct 2009 19:33:19 -0000 @@ -30,6 +30,8 @@ #include <stdlib.h> #include <string.h> #include <errno.h> +#include <sys/types.h> +#include <sys/stat.h> #ifdef HAVE_SCHED_SETAFFINITY #include <sched.h> #endif @@ -98,7 +100,6 @@ static void perfmon_start_child(int ctx_fd) { if (perfmonctl(ctx_fd, PFM_START, 0, 0) == -1) { - perror("Couldn't start perfmon: "); exit(EXIT_FAILURE); } } @@ -107,7 +108,6 @@ static void perfmon_stop_child(int ctx_fd) { if (perfmonctl(ctx_fd, PFM_STOP, 0, 0) == -1) { - perror("Couldn't stop perfmon: "); exit(EXIT_FAILURE); } } @@ -141,7 +141,6 @@ static void child_sigterm(int val __attribute__((unused))) { - printf("Child received SIGTERM, killing parent.\n"); kill(getppid(), SIGTERM); } @@ -149,15 +148,15 @@ static void set_affinity(size_t cpu) { cpu_set_t set; + int err; CPU_ZERO(&set); CPU_SET(cpu, &set); - int err = sched_setaffinity(getpid(), sizeof(set), &set); + err = sched_setaffinity(getpid(), sizeof(set), &set); if (err == -1) { - fprintf(stderr, "Failed to set affinity: %s\n", - strerror(errno)); + perror("Failed to set affinity"); exit(EXIT_FAILURE); } } @@ -178,7 +177,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGUSR1, &act, NULL)) { - perror("oprofiled: install of SIGUSR1 handler failed: "); + perror("oprofiled: install of SIGUSR1 handler failed"); exit(EXIT_FAILURE); } @@ -187,7 +186,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGUSR2, &act, NULL)) { - perror("oprofiled: install of SIGUSR2 handler failed: "); + perror("oprofiled: install of SIGUSR2 handler failed"); exit(EXIT_FAILURE); } @@ -196,7 +195,7 @@ sigemptyset(&act.sa_mask); if (sigaction(SIGTERM, &act, NULL)) { - perror("oprofiled: install of SIGTERM handler failed: "); + perror("oprofiled: install of SIGTERM handler failed"); exit(EXIT_FAILURE); } } @@ -214,8 +213,7 @@ err = perfmonctl(0, PFM_CREATE_CONTEXT, &ctx, 1); if (err == -1) { - fprintf(stderr, "CREATE_CONTEXT failed: %s\n", - strerror(errno)); + perror("CREATE_CONTEXT failed"); exit(EXIT_FAILURE); } @@ -268,13 +266,13 @@ err = perfmonctl(self->ctx_fd, PFM_WRITE_PMCS, pc, i); if (err == -1) { - perror("Couldn't write PMCs: "); + perror("Couldn't write PMCs"); exit(EXIT_FAILURE); } err = perfmonctl(self->ctx_fd, PFM_WRITE_PMDS, pd, i); if (err == -1) { - perror("Couldn't write PMDs: "); + perror("Couldn't write PMDs"); exit(EXIT_FAILURE); } } @@ -290,7 +288,7 @@ err = perfmonctl(self->ctx_fd, PFM_LOAD_CONTEXT, &load_args, 1); if (err == -1) { - perror("Couldn't load context: "); + perror("Couldn't load context"); exit(EXIT_FAILURE); } } @@ -304,13 +302,17 @@ if (ret == sizeof(size_t)) break; if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to write child pipe with %s\n", - strerror(errno)); + perror("Failed to write child pipe:"); exit(EXIT_FAILURE); } } } +static struct child * inner_child; +void close_pipe(void) +{ + close(inner_child->up_pipe[1]); +} static void run_child(size_t cpu) { @@ -321,6 +323,19 @@ self->sigusr2 = 0; self->sigterm = 0; + inner_child = self; + if (atexit(close_pipe)){ + close_pipe(); + exit(EXIT_FAILURE); + } + + umask(0); + /* Change directory to allow directory to be removed */ + if (chdir("/") < 0) { + perror("Unable to chdir to \"/\""); + exit(EXIT_FAILURE); + } + setup_signals(); set_affinity(cpu); @@ -333,6 +348,11 @@ notify_parent(self, cpu); + /* Redirect standard files to /dev/null */ + freopen( "/dev/null", "r", stdin); + freopen( "/dev/null", "w", stdout); + freopen( "/dev/null", "w", stderr); + for (;;) { sigset_t sigmask; sigfillset(&sigmask); @@ -341,15 +361,11 @@ sigdelset(&sigmask, SIGTERM); if (self->sigusr1) { - printf("PFM_START on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_start_child(self->ctx_fd); self->sigusr1 = 0; } if (self->sigusr2) { - printf("PFM_STOP on CPU%d\n", (int)cpu); - fflush(stdout); perfmon_stop_child(self->ctx_fd); self->sigusr2 = 0; } @@ -367,9 +383,8 @@ ret = read(child->up_pipe[0], &tmp, sizeof(size_t)); if (ret == sizeof(size_t)) break; - if (ret < 0 && errno != EINTR) { - fprintf(stderr, "Failed to read child pipe with %s\n", - strerror(errno)); + if ((ret < 0 && errno != EINTR) || ret == 0 ) { + perror("Failed to read child pipe"); exit(EXIT_FAILURE); } } @@ -377,7 +392,6 @@ fflush(stdout); close(child->up_pipe[0]); - close(child->up_pipe[1]); } static struct child* xen_ctx; @@ -417,25 +431,26 @@ nr_cpus = nr; children = xmalloc(sizeof(struct child) * nr_cpus); + bzero(children, sizeof(struct child) * nr_cpus); for (i = 0; i < nr_cpus; ++i) { int ret; if (pipe(children[i].up_pipe)) { - perror("Couldn't create child pipe.\n"); + perror("Couldn't create child pipe"); exit(EXIT_FAILURE); } ret = fork(); if (ret == -1) { - fprintf(stderr, "Couldn't fork perfmon child.\n"); + perror("Couldn't fork perfmon child"); exit(EXIT_FAILURE); } else if (ret == 0) { - printf("Running perfmon child on CPU%d.\n", (int)i); - fflush(stdout); + close(children[i].up_pipe[0]); run_child(i); } else { children[i].pid = ret; + close(children[i].up_pipe[1]); printf("Waiting on CPU%d\n", (int)i); wait_for_child(&children[i]); } @@ -454,8 +469,12 @@ return; for (i = 0; i < nr_cpus; ++i) { - kill(children[i].pid, SIGKILL); - waitpid(children[i].pid, NULL, 0); + if (children[i].pid) { + int c_pid = children[i].pid; + children[i].pid = 0; + if (kill(c_pid, SIGKILL)==0) + waitpid(c_pid, NULL, 0); + } } } @@ -472,8 +491,12 @@ return; } - for (i = 0; i < nr_cpus; ++i) - kill(children[i].pid, SIGUSR1); + for (i = 0; i < nr_cpus; ++i) { + if (kill(children[i].pid, SIGUSR1)) { + perror("Unable to start perfmon"); + exit(EXIT_FAILURE); + } + } } @@ -490,7 +513,10 @@ } for (i = 0; i < nr_cpus; ++i) - kill(children[i].pid, SIGUSR2); + if (kill(children[i].pid, SIGUSR2)) { + perror("Unable to stop perfmon"); + exit(EXIT_FAILURE); + } } #endif /* __ia64__ */ ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
|
|
Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
by Maynard Johnson
::
Rate this Message:
William Cohen wrote:
> William Cohen wrote: > >> I am not sure that the SIGCHLD will work in this particular case because the of >> the daemonization (fork and setsid()) after the creation of the processes to >> control perfmon on each processor. >> >> -Will > > Because the oprofiled daemon is a sibling rather than a parent, I took a > different approach than SIGCHLD, The code checks the results of the kill() to > determine if a signal was successfully sent. thanks. -Maynard > > -Will > ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ oprofile-list mailing list oprofile-list@... https://lists.sourceforge.net/lists/listinfo/oprofile-list |
| Free embeddable forum powered by Nabble | Forum Help |
