From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
To: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>,
linux-kernel@vger.kernel.org,
Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH] perf stat: Do not show stats if workload fails
Date: Thu, 26 Dec 2013 11:15:07 -0300 [thread overview]
Message-ID: <20131226141507.GD30980@ghostprotocols.net> (raw)
In-Reply-To: <52BC390A.2000504@gmail.com>
Em Thu, Dec 26, 2013 at 09:11:22AM -0500, David Ahern escreveu:
> On 12/24/13, 8:30 AM, Arnaldo Carvalho de Melo wrote:
> >Em Tue, Dec 24, 2013 at 09:53:42AM -0300, Arnaldo Carvalho de Melo escreveu:
> >>>The thing to check is perf_evlist__{prepare,start}_workload notification
> >>>errors using SIGUSR1, that we need to check for in the caller, and emit
> >>>the message, no?
> >Something like this:
> >
> >1. We tell perf_evlist__prepare_workload that we want a signal if execvp
> >fails, it will be a SIGUSR1
> >
> >2. We catch that signal in 'stat' and check that we got a signal, only
> >problem so far with this signal maze is that we're getting a SIGCHLD
> >while I was expecting a SIGUSR1... I.e. the "if (signr != -1) test
> >really should be if (signr == SIGUSR1), but I'm getting a SIGCHLD there
> >and the elves are tugging me away...
>
> Did the elves release you?
>
> There are all kinds of failure paths with the workload functions. In
> the end I was focusing on perf-stat actually checking the rc on the
> start_workload function. If it fails, then write() failed and
> something happened to the workload process. In that case don't show
> the counters.
>
> Handling the other error paths with appropriate messages will take
> additional effort - as you are finding. ;-)
Right, but I can't apply that patch, as it makes 'perf stat
whatever-workload' to fail, as I realized when doing a demo to someone
interested in using perf ;-\
So for now I'm not applying that one.
Ah, at this point elves are everywhere, dammit! ;-)
- Arnaldo
next prev parent reply other threads:[~2013-12-26 14:15 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-20 5:52 [PATCH] perf stat: Do not show stats if workload fails David Ahern
2013-12-20 7:57 ` Ingo Molnar
2013-12-23 19:37 ` Arnaldo Carvalho de Melo
2013-12-24 0:29 ` David Ahern
2013-12-24 12:53 ` Arnaldo Carvalho de Melo
2013-12-24 13:30 ` Arnaldo Carvalho de Melo
2013-12-26 14:11 ` David Ahern
2013-12-26 14:15 ` Arnaldo Carvalho de Melo [this message]
2013-12-26 14:18 ` David Ahern
2013-12-26 14:26 ` Arnaldo Carvalho de Melo
2013-12-26 14:55 ` Arnaldo Carvalho de Melo
2014-01-02 14:44 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131226141507.GD30980@ghostprotocols.net \
--to=acme@ghostprotocols.net \
--cc=dsahern@gmail.com \
--cc=eranian@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox