[prev in list] [next in list] [prev in thread] [next in thread]
List: oprofile-list
Subject: Re: [PATCH] "opcontrol --start-daemon" doesn't work correctly on ia64
From: Maynard Johnson <maynardj () us ! ibm ! com>
Date: 2009-09-18 13:53:21
Message-ID: 4AB390D1.7090502 () us ! ibm ! com
[Download RAW message or body]
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. :-)
>
> 2009-09-01 William Cohen <wcohen@redhat.com>
>
> * daemon/opd_perfmon.c: Fix ia64 output to allow spawning daemon.
>
>
> Signed-off-by: William Cohen <wcohen@redhat.com>
>
> -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@lists.sourceforge.net
> 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@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/oprofile-list
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic