From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Rostedt Subject: Re: [PATCH RFC 3/4] drivers: irqchip: pdc: log PDC info in FTRACE Date: Tue, 23 Jan 2018 13:13:26 -0500 Message-ID: <20180123131326.0f4121e7@gandalf.local.home> References: <20180123175656.11942-1-ilina@codeaurora.org> <20180123175656.11942-4-ilina@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: Received: from mail.kernel.org ([198.145.29.99]:38816 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751796AbeAWSN3 (ORCPT ); Tue, 23 Jan 2018 13:13:29 -0500 In-Reply-To: <20180123175656.11942-4-ilina@codeaurora.org> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Lina Iyer Cc: tglx@linutronix.de, jason@lakedaemon.net, marc.zyngier@arm.com, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, sboyd@codeaurora.org, rnayak@codeaurora.org, asathyak@codeaurora.org On Tue, 23 Jan 2018 10:56:55 -0700 Lina Iyer wrote: > From: Archana Sathyakumar Hi Lina and Archana, > > Log key PDC pin configuration in FTRACE. > > Cc: Steven Rostedt > Signed-off-by: Archana Sathyakumar > Signed-off-by: Lina Iyer > --- > drivers/irqchip/qcom-pdc.c | 6 ++++++ > include/trace/events/pdc.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+) > create mode 100644 include/trace/events/pdc.h > > diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c > index 57f1348bd81c..9b626e9f3a29 100644 > --- a/drivers/irqchip/qcom-pdc.c > +++ b/drivers/irqchip/qcom-pdc.c > @@ -26,6 +26,8 @@ > #include > #include > #include > +#define CREATE_TRACE_POINTS > +#include "trace/events/pdc.h" > > #include "qcom-pdc.h" > > @@ -78,6 +80,7 @@ static inline void pdc_enable_intr(struct irq_data *d, bool on) > break; > udelay(5); > } while (1); > + trace_irq_pin_config("enable", (u32)pin_out, (u32)d->hwirq, 0, on); I'm concerned with that string you are passing in. > spin_unlock_irqrestore(&pdc_lock, flags); > WARN_ON(r_enable != enable); > } > @@ -161,6 +164,9 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type) > udelay(5); > } while (1); > > + trace_irq_pin_config("type_config", (u32)pin_out, (u32)d->hwirq, This one too. > + pdc_type, 0); > + > /* > * If type is edge triggered, forward that as Rising edge as PDC > * takes care of converting falling edge to rising edge signal > diff --git a/include/trace/events/pdc.h b/include/trace/events/pdc.h > new file mode 100644 > index 000000000000..bec54c414880 > --- /dev/null > +++ b/include/trace/events/pdc.h > @@ -0,0 +1,50 @@ > +/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#undef TRACE_SYSTEM > +#define TRACE_SYSTEM pdc > + > +#if !defined(_TRACE_PDC_) || defined(TRACE_HEADER_MULTI_READ) > +#define _TRACE_PDC_H_ > + > +#include > + > +TRACE_EVENT(irq_pin_config, > + > + TP_PROTO(char *func, u32 pin, u32 hwirq, u32 type, u32 enable), > + > + TP_ARGS(func, pin, hwirq, type, enable), > + > + TP_STRUCT__entry( > + __field(char *, func) > + __field(u32, pin) > + __field(u32, hwirq) > + __field(u32, type) > + __field(u32, enable) > + ), > + > + TP_fast_assign( > + __entry->pin = pin; > + __entry->func = func; > + __entry->hwirq = hwirq; > + __entry->type = type; > + __entry->enable = enable; > + ), > + > + TP_printk("%s hwirq:%u pin:%u type:%u enable:%u", %s is dereferencing a pointer, which works fine when reading the ASCII translated trace files (/sys/kernel/debug/tracing/trace and friends). But it breaks when we use trace-cmd or perf. Because they record the binary data and they do not have access to memory inside the kernel. What I would recommend is in pdc.h have: #define PDC_ENTRY 1 #define PDC_TYPE_CONFIG 2 Then have the tracepoints do instead of: trace_irq_pin_config("enable", ...); trace_irq_pin_config("type_config", ...); Have them do: trace_irq_pin_config(PDC_ENTRY, ...); trace_irq_pin_config(PDC_TYPE_CONFIG, ...); And then have: TP_PROTO(u32 func, ...) TP_STRUCT__entry( __field(u32, func) [...] TP_fast_assign( __entry->func = func; [...] TP_printk("%s hwirq...", print_symbolic(__entry->func, { PDC_ENTRY, "entry" }, { PDC_TYPE_CONFIG, "type_config" }), [...] And you will get the same effect, with trace-cmd and perf still working as expected. -- Steve > + __entry->func, __entry->pin, __entry->hwirq, __entry->type, > + __entry->enable) > +); > + > +#endif > +#define TRACE_INCLUDE_FILE pdc > +#include