From: Steven Rostedt <rostedt@goodmis.org>
To: Prasad Sodagudi <psodagud@codeaurora.org>
Cc: mingo@redhat.com, keescook@chromium.org,
saiprakash.ranjan@codeaurora.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
gregkh@linuxfoundation.org, anton@enomsg.org, arnd@arndb.de,
catalin.marinas@arm.com, ccross@android.com, jbaron@akamai.com,
jim.cromie@gmail.com, joe@perches.com, joel@joelfernandes.org
Subject: Re: [PATCH] tracing: Add register read and write tracing support
Date: Mon, 28 Sep 2020 10:55:01 -0400 [thread overview]
Message-ID: <20200928105501.7e29df65@oasis.local.home> (raw)
In-Reply-To: <1601253290-400618-2-git-send-email-psodagud@codeaurora.org>
On Sun, 27 Sep 2020 17:34:50 -0700
Prasad Sodagudi <psodagud@codeaurora.org> wrote:
> Add register read/write operations tracing support.
> ftrace events helps trace register read and write
> location details of memory mapped IO registers. Also
> add _no_log variants the writel_relaxed/readl_relaed
> APIs to avoid excessive logging for certain register
> operations.
As mentioned elsewhere, I don't see a reason for "nolog" variants if it
is just to avoid logging too much. You can easily filter on the
recording side.
> --- /dev/null
> +++ b/include/linux/iorw.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *
> + */
> +#ifndef __LOG_IORW_H__
> +#define __LOG_IORW_H__
> +
> +#include <linux/types.h>
> +
> +#if IS_ENABLED(CONFIG_TRACE_RW)
> +void log_write_io(volatile void __iomem *addr);
> +void log_read_io(const volatile void __iomem *addr);
So basically, this is always doing a function call, even when tracing
is not enabled. You may want to turn this into a macro, and use the new
interface I'm about to push:
See https://lore.kernel.org/r/20200925211206.423598568@goodmis.org
Although I'm about to push a v3 (found a config that breaks msr.h)
#if IS_ENABLED(CONFIG_TRACE_RW)
#include <linux/atomic.h>
#include <linux/tracepoint-defs.h>
DECLARE_TRACEPOINT(rwio_write);
DECLARE_TRACEPOINT(rwio_read);
void __log_write_io(volatile void __iomem *addr);
void __log_read_io(const volatile void __iomem *addr);
#define log_write_io(addr) \
if (tracepoint_enabled(rwio_write)
__log_write_io(addr)
#define log_read_io(addr) \
if (tracepoint_enabled(rwio_read)
__log_read_io(addr)
> +#else
> +static inline void log_write_io(volatile void __iomem *addr)
> +{ }
> +static inline void log_read_io(const volatile void __iomem *addr)
> +{ }
> +#endif /* CONFIG_TRACE_RW */
> +
> +#endif /* __LOG_IORW_H__ */
> diff --git a/include/trace/events/rwio.h b/include/trace/events/rwio.h
> new file mode 100644
> index 0000000..b829629
> --- /dev/null
> +++ b/include/trace/events/rwio.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM rwio
> +
> +#if !defined(_TRACE_RWIO_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_RWIO_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(raw_write,
"raw" is too generic. Call this rwio_write.
> +
> + TP_PROTO(unsigned long fn, volatile void __iomem *addr),
> +
> + TP_ARGS(fn, addr),
> +
> + TP_STRUCT__entry(
> + __field(u64, fn)
> + __field(u64, addr)
> + ),
> +
> + TP_fast_assign(
> + __entry->fn = fn;
> + __entry->addr = (u64)addr;
> + ),
> +
> + TP_printk("%pS write addr=%p\n", __entry->fn, __entry->addr)
> +);
> +
> +TRACE_EVENT(raw_read,
And this "rwio_read"
-- Steve
> +
> + TP_PROTO(unsigned long fn, const volatile void __iomem *addr),
> +
> + TP_ARGS(fn, addr),
> +
> + TP_STRUCT__entry(
> + __field(u64, fn)
> + __field(u64, addr)
> + ),
> +
> + TP_fast_assign(
> + __entry->fn = fn;
> + __entry->addr = (u64)addr;
> + ),
> +
> + TP_printk("%pS read addr=%p\n", __entry->fn, __entry->addr)
> +);
> +
> +#endif /* _TRACE_PREEMPTIRQ_H */
> +
next prev parent reply other threads:[~2020-09-28 14:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-28 0:34 [PATCH] Register read and writes tracing Prasad Sodagudi
2020-09-28 0:34 ` [PATCH] tracing: Add register read and write tracing support Prasad Sodagudi
2020-09-28 5:17 ` Greg KH
2020-09-28 5:20 ` Greg KH
2020-09-28 14:55 ` Steven Rostedt [this message]
2020-09-28 5:45 ` [PATCH] Register read and writes tracing Sai Prakash Ranjan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200928105501.7e29df65@oasis.local.home \
--to=rostedt@goodmis.org \
--cc=anton@enomsg.org \
--cc=arnd@arndb.de \
--cc=catalin.marinas@arm.com \
--cc=ccross@android.com \
--cc=gregkh@linuxfoundation.org \
--cc=jbaron@akamai.com \
--cc=jim.cromie@gmail.com \
--cc=joe@perches.com \
--cc=joel@joelfernandes.org \
--cc=keescook@chromium.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=psodagud@codeaurora.org \
--cc=saiprakash.ranjan@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).