diff mbox series

[2/9] perf report: Ignore SIGPIPE for srcline

Message ID 20221215192817.2734573-3-namhyung@kernel.org
State New
Headers
Series perf report: Improve srcline sort performance (v1) |

Commit Message

Namhyung Kim Dec. 15, 2022, 7:28 p.m. UTC 
  It can get SIGPIPE when it uses an external addr2line process and the
process was terminated unexpectedly.  Let's ignore the signal and move
on to the next sample.  The sample will get the default srcline value
anyway.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andi Kleen Dec. 16, 2022, 7:24 a.m. UTC | #1 
On 12/15/2022 9:28 PM, Namhyung Kim wrote:
> It can get SIGPIPE when it uses an external addr2line process and the
> process was terminated unexpectedly.  Let's ignore the signal and move
> on to the next sample.  The sample will get the default srcline value
> anyway.


That's a bit dangerous -- if perf report output is piped to something 
else you really want to stop on SIGPIPE.

You would need to find a way to distinguish those cases.

-Andi
Namhyung Kim Dec. 16, 2022, 6:08 p.m. UTC | #2 
Hi Andi,

On Thu, Dec 15, 2022 at 11:25 PM Andi Kleen <ak@linux.intel.com> wrote:
>
>
> On 12/15/2022 9:28 PM, Namhyung Kim wrote:
> > It can get SIGPIPE when it uses an external addr2line process and the
> > process was terminated unexpectedly.  Let's ignore the signal and move
> > on to the next sample.  The sample will get the default srcline value
> > anyway.
>
>
> That's a bit dangerous -- if perf report output is piped to something
> else you really want to stop on SIGPIPE.

Maybe we can handle the pipe write errors gracefully, but it'd require
more changes in many places.

>
> You would need to find a way to distinguish those cases.

Hmm.. ok.  I guess we can just drop this for now.  With checking
the .debug_line section, problematic cases should be gone mostly.

Thanks,
Namhyung
Arnaldo Carvalho de Melo Dec. 20, 2022, 6:38 p.m. UTC | #3 
Em Fri, Dec 16, 2022 at 10:08:50AM -0800, Namhyung Kim escreveu:
> Hi Andi,
> 
> On Thu, Dec 15, 2022 at 11:25 PM Andi Kleen <ak@linux.intel.com> wrote:
> >
> >
> > On 12/15/2022 9:28 PM, Namhyung Kim wrote:
> > > It can get SIGPIPE when it uses an external addr2line process and the
> > > process was terminated unexpectedly.  Let's ignore the signal and move
> > > on to the next sample.  The sample will get the default srcline value
> > > anyway.
> >
> >
> > That's a bit dangerous -- if perf report output is piped to something
> > else you really want to stop on SIGPIPE.
> 
> Maybe we can handle the pipe write errors gracefully, but it'd require
> more changes in many places.
> 
> >
> > You would need to find a way to distinguish those cases.
> 
> Hmm.. ok.  I guess we can just drop this for now.  With checking
> the .debug_line section, problematic cases should be gone mostly.

So just skip this one, ok, I'll cherry pick the rest. Done.

- Arnaldo
diff mbox series

Patch

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2ee2ecca208e..d98112f173b0 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -949,6 +949,7 @@  static int __cmd_report(struct report *rep)
 	struct perf_data *data = session->data;
 
 	signal(SIGINT, sig_handler);
+	signal(SIGPIPE, SIG_IGN);
 
 	if (rep->cpu_list) {
 		ret = perf_session__cpu_bitmap(session, rep->cpu_list,