From: Maynard J. <may...@us...> - 2012-06-25 13:59:03
|
On 06/25/2012 05:02 AM, Paul Guo wrote: >> Paul, >> This patch fixes the problem of 'operf -p <PID>' not always >> collecting all of the sample data it should, which you've seen by profiling >> 'top'. Can you give it a try and le me know if you still have any >> problems? > > This patch works. operf can collect samples with both default sampling frequency > and much lower sampling frequency now. Thanks for testing. I committed the patch to the perf-events branch now. -Maynard > > Thanks, > Paul > >> >> Thanks. >> >> -Maynard >> >> ---------------------------------------------------------------- >> >> [PATCH] Handle EINTR of operf read of sample data pipe >> >> When profiling an application with operf's "--pid" option, >> the user must stop profiling using ctrl-C. A signal >> handler was setup for the operf process that reads >> sample data from a pipe (written to by the operf record >> process), but when that handler was triggered, the >> operf read process was ending before reading the >> rest of the sample data from the pipe. In cases >> where lots of sample data has already been received, >> the little bit that gets left in the pipe may not be >> noticeable. But for situations where the samples >> are few and far between, this problem can, on some >> platforms, result in no profiling data being received >> at all. >> >> The fix for the problem is to detect an EINTR when trying >> to read the pipe and go back and read it again. >> >> Another symptom of unnecessary stopping due to EINTR is >> possible when opening the sample data files to store >> the converted sample data. This is a very small window, >> and I have not actually seen this symptom, but with this >> patch, I plug up that window as well. >> >> Signed-off-by: Maynard Johnson <may...@us...> >> --- >> libperf_events/operf_counter.cpp | 21 +++++++++++++++++++-- >> libperf_events/operf_mangling.cpp | 4 ++++ >> pe_profiling/operf.cpp | 3 ++- >> 3 files changed, 25 insertions(+), 3 deletions(-) >> >> diff --git a/libperf_events/operf_counter.cpp b/libperf_events/operf_counter.cpp >> index 627a1a5..364e3ac 100644 >> --- a/libperf_events/operf_counter.cpp >> +++ b/libperf_events/operf_counter.cpp >> @@ -528,19 +528,36 @@ int operf_read::_get_one_perf_event(event_t * event) >> char * evt = (char *)event; >> ssize_t num_read; >> perf_event_header * header = (perf_event_header *)event; >> + >> + /* A signal handler was setup for the operf_read process to handle interrupts >> + * (i.e., from ctrl-C), so the read syscalls below may get interrupted. But the >> + * operf_read process should ignore the interrupt and continue processing >> + * until there's no more data to read or until the parent operf process >> + * forces us to stop. So we must try the read operation again if it was >> + * interrupted. >> + */ >> +again: >> errno = 0; >> if ((num_read = read(sample_data_fd, header, pe_header_size)) < 0) { >> cverb << vdebug << "Read 1 of sample data pipe returned with " << strerror(errno) << endl; >> - return -1; >> + if (errno == EINTR) >> + goto again; >> + else >> + return -1; >> } else if (num_read == 0) { >> return -1; >> } >> evt += pe_header_size; >> if (!header->size) >> return -1; >> + >> +again2: >> if ((num_read = read(sample_data_fd, evt, header->size - pe_header_size)) < 0) { >> cverb << vdebug << "Read 2 of sample data pipe returned with " << strerror(errno) << endl; >> - return -1; >> + if (errno == EINTR) >> + goto again2; >> + else >> + return -1; >> } else if (num_read == 0) { >> return -1; >> } >> diff --git a/libperf_events/operf_mangling.cpp b/libperf_events/operf_mangling.cpp >> index 5165807..df173d8 100644 >> --- a/libperf_events/operf_mangling.cpp >> +++ b/libperf_events/operf_mangling.cpp >> @@ -163,6 +163,10 @@ retry: >> } >> goto retry; >> } >> + if (err == EINTR) { >> + cverb << vsfile << "operf: open of " << mangled << " was interrupted. Trying again." << endl; >> + goto retry; >> + } >> >> cerr << "operf: open of " << mangled << " failed: " << strerror(err) << endl; >> goto out; >> diff --git a/pe_profiling/operf.cpp b/pe_profiling/operf.cpp >> index 9d3eea7..0cf3778 100644 >> --- a/pe_profiling/operf.cpp >> +++ b/pe_profiling/operf.cpp >> @@ -859,7 +859,8 @@ static void convert_sample_data(void) >> cverb << vdebug << "Successfully read header info for sample data " << endl; >> if (operfRead.is_valid()) { >> try { >> - operfRead.convertPerfData(); >> + int num = operfRead.convertPerfData(); >> + cverb << vdebug << "operf_read: Total bytes received from operf_record process: " << dec << num << endl; >> } catch (runtime_error e) { >> cerr << "Caught runtime error from operf_read::convertPerfData" << endl; >> cerr << e.what() << endl; >> > |