I wasn't entirely sure if we should modify the signature of the python hooks_ as this would make existing scripts incompatible. So the patch only adds sample->pid to the event_fields_dict param in trace_unhandled().
On Wed, Jul 5, 2017 at 12:26 PM, Arnaldo Carvalho de Melo <[email protected]> wrote: > Em Wed, Jul 05, 2017 at 04:25:45PM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Wed, Jul 05, 2017 at 09:22:07AM -0700, Arun Kalyanasundaram escreveu: >> > Hi Arnaldo, >> > >> > Thank you for your reply. >> > I actually meant tracepoint event handlers: def >> > trace_unhandled(event_name, context, event_fields_dict) >> > The dict parameter contains an attribute "common_pid" which is >> > actually the "tid" of the thread. There are no other attributes that >> > contain the actual pid of the process. So, I was wondering if this is >> > something intentional? If not I can share a patch to fix this. >> >> Yeah there is a problem in: >> >> tools/perf/util/scripting-engines/trace-event-python.c >> >> static void python_process_event(union perf_event *event, >> struct perf_sample *sample, >> struct perf_evsel *evsel, >> struct addr_location *al) >> { >> struct tables *tables = &tables_global; >> >> switch (evsel->attr.type) { >> case PERF_TYPE_TRACEPOINT: >> python_process_tracepoint(sample, evsel, al); >> break; >> /* Reserve for future process_hw/sw/raw APIs */ >> default: >> if (tables->db_export_mode) >> db_export__sample(&tables->dbe, event, sample, >> evsel, al); >> else >> python_process_general_event(sample, evsel, al); >> } >> } >> >> The python_process_tracepoint() thing predates >> python_process_general_event(), and doesn't adds the dict with all the >> perf_sample entries that python_process_general_event() passes to the >> python method :-\ >> >> Both the per-tracepoint python hooks _and_ trace_unhandled() should get >> that dict, is that what your patch does? > > Well, for performance reasons I think perhaps we could take a look at > the signature of the python hook and provide the dictionary only if it > is in it? > > - Arnaldo
