Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752721AbdGEXvk (ORCPT ); Wed, 5 Jul 2017 19:51:40 -0400 Received: from mail-ua0-f169.google.com ([209.85.217.169]:34873 "EHLO mail-ua0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752040AbdGEXvi (ORCPT ); Wed, 5 Jul 2017 19:51:38 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170701144703.GA5504@redhat.com> <20170705192545.GI27350@kernel.org> <20170705192640.GJ27350@kernel.org> From: Arun Kalyanasundaram Date: Wed, 5 Jul 2017 16:51:26 -0700 Message-ID: Subject: Re: perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute To: Arnaldo Carvalho de Melo Cc: Arnaldo Carvalho de Melo , Tom Zanussi , linux-kernel@vger.kernel.org, David Carrillo-Cisneros Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2871 Lines: 73 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 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 > 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