* Re: perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute [not found] <CAGjB_Bp0haKX+YPC5117yK440mTK1ZW3WBubt616hbKaQNxd6w@mail.gmail.com> @ 2017-07-01 14:47 ` Arnaldo Carvalho de Melo 2017-07-05 16:22 ` Arun Kalyanasundaram 0 siblings, 1 reply; 7+ messages in thread From: Arnaldo Carvalho de Melo @ 2017-07-01 14:47 UTC (permalink / raw) To: Arun Kalyanasundaram Cc: tzanussi, linux-kernel, David Carrillo-Cisneros, acme Em Fri, Jun 30, 2017 at 03:40:57PM -0700, Arun Kalyanasundaram escreveu: > The handlers in the python script generated from "perf script" have an > attribute: common_pid. This attribute contains the tid of the process > instead of its pid. I would like to know if this is the expected behavior. > There are no other attributes in the Python handler that provide the pid > and knowing the process id is useful to be able to group all samples that > belong to the same process that generated different threads. Humm, you have: def process_event(param_dict): event_attr = param_dict["attr"] sample = param_dict["sample"] raw_buf = param_dict["raw_buf"] comm = param_dict["comm"] name = param_dict["ev_name"] And then, on sample you have (from a recent python script for processing Intel PT samples): def print_common_start(comm, sample, name): ts = sample["time"] cpu = sample["cpu"] pid = sample["pid"] tid = sample["tid"] print "%16s %5u/%-5u [%03u] %9u.%09u %7s:" % (comm, pid, tid, cpu, ts / 1000000000, ts %1000000000, name), - Arnaldo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute 2017-07-01 14:47 ` perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute Arnaldo Carvalho de Melo @ 2017-07-05 16:22 ` Arun Kalyanasundaram 2017-07-05 19:25 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 7+ messages in thread From: Arun Kalyanasundaram @ 2017-07-05 16:22 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Tom Zanussi, linux-kernel, David Carrillo-Cisneros, acme 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. Best, - Arun On Sat, Jul 1, 2017 at 7:47 AM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote: > Em Fri, Jun 30, 2017 at 03:40:57PM -0700, Arun Kalyanasundaram escreveu: >> The handlers in the python script generated from "perf script" have an >> attribute: common_pid. This attribute contains the tid of the process >> instead of its pid. I would like to know if this is the expected behavior. >> There are no other attributes in the Python handler that provide the pid >> and knowing the process id is useful to be able to group all samples that >> belong to the same process that generated different threads. > > Humm, you have: > > def process_event(param_dict): > event_attr = param_dict["attr"] > sample = param_dict["sample"] > raw_buf = param_dict["raw_buf"] > comm = param_dict["comm"] > name = param_dict["ev_name"] > > And then, on sample you have (from a recent python script for processing > Intel PT samples): > > def print_common_start(comm, sample, name): > ts = sample["time"] > cpu = sample["cpu"] > pid = sample["pid"] > tid = sample["tid"] > print "%16s %5u/%-5u [%03u] %9u.%09u %7s:" % (comm, pid, tid, cpu, ts / 1000000000, ts %1000000000, name), > > - Arnaldo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute 2017-07-05 16:22 ` Arun Kalyanasundaram @ 2017-07-05 19:25 ` Arnaldo Carvalho de Melo 2017-07-05 19:26 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 7+ messages in thread From: Arnaldo Carvalho de Melo @ 2017-07-05 19:25 UTC (permalink / raw) To: Arun Kalyanasundaram Cc: Arnaldo Carvalho de Melo, Tom Zanussi, linux-kernel, David Carrillo-Cisneros 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? - Arnaldo > Best, > - Arun > > > On Sat, Jul 1, 2017 at 7:47 AM, Arnaldo Carvalho de Melo > <acme@redhat.com> wrote: > > Em Fri, Jun 30, 2017 at 03:40:57PM -0700, Arun Kalyanasundaram escreveu: > >> The handlers in the python script generated from "perf script" have an > >> attribute: common_pid. This attribute contains the tid of the process > >> instead of its pid. I would like to know if this is the expected behavior. > >> There are no other attributes in the Python handler that provide the pid > >> and knowing the process id is useful to be able to group all samples that > >> belong to the same process that generated different threads. > > > > Humm, you have: > > > > def process_event(param_dict): > > event_attr = param_dict["attr"] > > sample = param_dict["sample"] > > raw_buf = param_dict["raw_buf"] > > comm = param_dict["comm"] > > name = param_dict["ev_name"] > > > > And then, on sample you have (from a recent python script for processing > > Intel PT samples): > > > > def print_common_start(comm, sample, name): > > ts = sample["time"] > > cpu = sample["cpu"] > > pid = sample["pid"] > > tid = sample["tid"] > > print "%16s %5u/%-5u [%03u] %9u.%09u %7s:" % (comm, pid, tid, cpu, ts / 1000000000, ts %1000000000, name), > > > > - Arnaldo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute 2017-07-05 19:25 ` Arnaldo Carvalho de Melo @ 2017-07-05 19:26 ` Arnaldo Carvalho de Melo 2017-07-05 20:41 ` Arun Kalyanasundaram 0 siblings, 1 reply; 7+ messages in thread From: Arnaldo Carvalho de Melo @ 2017-07-05 19:26 UTC (permalink / raw) To: Arun Kalyanasundaram Cc: Arnaldo Carvalho de Melo, Tom Zanussi, linux-kernel, David Carrillo-Cisneros 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute 2017-07-05 19:26 ` Arnaldo Carvalho de Melo @ 2017-07-05 20:41 ` Arun Kalyanasundaram 2017-07-05 23:51 ` Arun Kalyanasundaram 0 siblings, 1 reply; 7+ messages in thread From: Arun Kalyanasundaram @ 2017-07-05 20:41 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Arnaldo Carvalho de Melo, Tom Zanussi, linux-kernel, David Carrillo-Cisneros 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 <acme@kernel.org> 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute 2017-07-05 20:41 ` Arun Kalyanasundaram @ 2017-07-05 23:51 ` Arun Kalyanasundaram 2017-07-07 15:18 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 7+ messages in thread From: Arun Kalyanasundaram @ 2017-07-05 23:51 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Arnaldo Carvalho de Melo, Tom Zanussi, linux-kernel, David Carrillo-Cisneros Arnaldo: So, I think what you are suggesting is, we should check the signature of the hook to determine if it has an additional attribute and only then should we provide the dict. May be something like this: PyObject* custom_dict = PyObject_GetAttrString(handler, "other_fields_dict"); if (custom_dict) //Add dict to PyTuple Do you think this would be a better approach? Thank you, - Arun On Wed, Jul 5, 2017 at 1:41 PM, Arun Kalyanasundaram <arunkaly@google.com> wrote: > 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 > <acme@kernel.org> 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute 2017-07-05 23:51 ` Arun Kalyanasundaram @ 2017-07-07 15:18 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 7+ messages in thread From: Arnaldo Carvalho de Melo @ 2017-07-07 15:18 UTC (permalink / raw) To: Arun Kalyanasundaram Cc: Arnaldo Carvalho de Melo, Tom Zanussi, linux-kernel, David Carrillo-Cisneros Em Wed, Jul 05, 2017 at 04:51:26PM -0700, Arun Kalyanasundaram escreveu: > Arnaldo: > So, I think what you are suggesting is, we should check the signature > of the hook to determine if it has an additional attribute and only > then should we provide the dict. May be something like this: > > PyObject* custom_dict = PyObject_GetAttrString(handler, "other_fields_dict"); > if (custom_dict) > //Add dict to PyTuple > > Do you think this would be a better approach? I think that we should provide as much as possible the common info about a sample at all the sample processing routines, be it the initial, tracepoint specific one, the one for non-tracepoint samples and the trace_unhandled one. So scripts not using this optional argument will work with older versions of the perf tool as well as with new ones that support it. One could say that to use a script that wants this optional argument, a new enough version of the tool is required. So yeah, I think this is a better approach. The details of how to best do this method signature signing need to be figured out, of course. Thanks for working on this! - Arnaldo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-07-07 15:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CAGjB_Bp0haKX+YPC5117yK440mTK1ZW3WBubt616hbKaQNxd6w@mail.gmail.com> 2017-07-01 14:47 ` perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute Arnaldo Carvalho de Melo 2017-07-05 16:22 ` Arun Kalyanasundaram 2017-07-05 19:25 ` Arnaldo Carvalho de Melo 2017-07-05 19:26 ` Arnaldo Carvalho de Melo 2017-07-05 20:41 ` Arun Kalyanasundaram 2017-07-05 23:51 ` Arun Kalyanasundaram 2017-07-07 15:18 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).