From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754837AbbIRPTQ (ORCPT ); Fri, 18 Sep 2015 11:19:16 -0400 Received: from foss.arm.com ([217.140.101.70]:46990 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754806AbbIRPTG (ORCPT ); Fri, 18 Sep 2015 11:19:06 -0400 Date: Fri, 18 Sep 2015 16:18:51 +0100 From: Mark Rutland To: Arnaldo Carvalho de Melo Cc: Adrian Hunter , "linux-kernel@vger.kernel.org" , Ingo Molnar , Jiri Olsa , Peter Zijlstra Subject: Re: [PATCH] perf tools: session: avoid infinite loop Message-ID: <20150918151851.GC31683@leverpostej> References: <1442423929-12253-1-git-send-email-mark.rutland@arm.com> <20150916205454.GI11551@kernel.org> <20150917154152.GD12808@leverpostej> <55FBAA8C.2020703@intel.com> <20150918095112.GA21286@leverpostej> <55FBED85.8020603@intel.com> <20150918133708.GA26468@leverpostej> <20150918150018.GS11551@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150918150018.GS11551@kernel.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 18, 2015 at 04:00:18PM +0100, Arnaldo Carvalho de Melo wrote: > I tried what you suggested, but couldn't reproduce it, I only managed > when I applied the patch below _and_ executed; > > # echo 1 > /proc/sys/kernel/kptr_restrict > > To be on the same page as Mark. Then, yes, I get stuck in: > > #0 0x00007f746a1d8aba in mmap64 () at ../sysdeps/unix/syscall-template.S:81 > #1 0x00000000004be66e in __perf_session__process_events (file_size=232, data_size=, data_offset=, session=0x1ba8590) > at util/session.c:1604 > #2 perf_session__process_events (session=0x1ba8590) at util/session.c:1685 > #3 0x000000000042d97b in process_buildids (rec=0x869ac0 ) at builtin-record.c:364 > #4 __cmd_record (rec=0x869ac0 , argv=, argc=) at builtin-record.c:734 > #5 cmd_record (argc=, argv=, prefix=) at builtin-record.c:1199 > #6 0x0000000000479123 in run_builtin (p=p@entry=0x874a90 , argc=argc@entry=3, argv=argv@entry=0x7ffd92eb5ee0) at perf.c:370 > #7 0x0000000000420aba in handle_internal_command (argv=0x7ffd92eb5ee0, argc=3) at perf.c:429 > #8 run_argv (argv=0x7ffd92eb5c70, argcp=0x7ffd92eb5c7c) at perf.c:473 > #9 main (argc=3, argv=0x7ffd92eb5ee0) at perf.c:588 > (gdb) > > After applying your patch it works, had to tweak the commit log to avoid > starting lines with ---, breaks git scripts, also added a commiter log, > check it, patch is below, after the one I used to not process any > samples. Sorry for the '---' problem, I'll bear that in mind in future. Your commit log looks fine, though I'm slightly confused by the Reported-by line -- did you mean to add that? Mark. > commit dd486ec4aa33cfca2fd912ef501d49909005de79 > Author: Mark Rutland > Date: Wed Sep 16 18:18:49 2015 +0100 > > perf record: Avoid infinite loop at buildid processing with no samples > > If a session contains no events, we can get stuck in an infinite loop in > __perf_session__process_events, with a non-zero file_size and data_offset, but > a zero data_size. > > In this case, we can mmap the entirety of the file (consisting of the file and > attribute headers), and fetch_mmaped_event will correctly refuse to read any > (unmapped and non-existent) event headers. This causes > __perf_session__process_events to unmap the file and retry with the exact same > parameters, getting stuck in an infinite loop. > > This has been observed to result in an exit-time hang when counting > rare/unschedulable events with perf record, and can be triggered artificially > with the script below: > > ---- > #!/bin/sh > printf "REPRO: launching perf\n"; > ./perf record -e software/config=9/ sleep 1 & > PERF_PID=$!; > sleep 0.002; > kill -2 $PERF_PID; > printf "REPRO: waiting for perf (%d) to exit...\n" "$PERF_PID"; > wait $PERF_PID; > printf "REPRO: perf exited\n"; > ---- > > To avoid this, have __perf_session__process_events bail out early when > the file has no data (i.e. it has no events). > > Commiter note: > > I only managed to reproduce this when setting > /proc/sys/kernel/kptr_restrict to '1' and changing the code to > purposefully not process any samples and no synthesized samples, i.e. > kptr_restrict prevents 'record' from synthesizing the kernel mmaps for > vmlinux + modules and since it is a workload started from perf, we don't > synthesize mmap/comm records for existing threads. > > Adrian Hunter managed to reproduce it in his environment tho. > > Reported-by: Arnaldo Carvalho de Melo > Signed-off-by: Mark Rutland > Cc: Adrian Hunter > Cc: Jiri Olsa > Cc: Peter Zijlstra > Link: http://lkml.kernel.org/r/1442423929-12253-1-git-send-email-mark.rutland@arm.com > Signed-off-by: Arnaldo Carvalho de Melo > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > index 8a4537ee9bc3..fc3f7c922f99 100644 > --- a/tools/perf/util/session.c > +++ b/tools/perf/util/session.c > @@ -1580,7 +1580,10 @@ static int __perf_session__process_events(struct perf_session *session, > file_offset = page_offset; > head = data_offset - page_offset; > > - if (data_size && (data_offset + data_size < file_size)) > + if (data_size == 0) > + goto out; > + > + if (data_offset + data_size < file_size) > file_size = data_offset + data_size; > > ui_progress__init(&prog, file_size, "Processing events..."); >