diff mbox series

[PATCHv3,3/3] dynamic_debug: Add a flag for dynamic event tracing

Message ID 3706af20bc64a320ff8f3ff8950738b988f4bdf5.1636452784.git.quic_saipraka@quicinc.com (mailing list archive)
State Superseded
Headers show
Series tracing/rwmmio/arm64: Add support to trace register reads/writes | expand

Commit Message

Sai Prakash Ranjan Nov. 9, 2021, 12:08 p.m. UTC
Debugging a specific driver or subsystem can be a lot easier if we can
trace events specific to that driver or subsystem. This type of
filtering can be achieved using existing dynamic debug library which
provides a way to filter based on files, functions and modules.

Using this, provide an additional flag 'e' to filter event tracing to
specified input.

For example, tracing all MMIO read/write can be overwhelming and of no
use when debugging a specific driver or a subsystem. So switch to
dynamic event tracing for register accesses.

Example: Tracing register accesses for all drivers in drivers/soc/qcom/*
and the trace output is given below:

  # dyndbg="file drivers/soc/qcom/* +e" trace_event=rwmmio
    or
  # echo "file drivers/soc/qcom/* +e" > /sys/kernel/debug/dynamic_debug/control
  # cat /sys/kernel/debug/tracing/trace
    rwmmio_read: rpmh_rsc_probe+0x35c/0x410 readl addr=0xffff80001071000c
    rwmmio_read: rpmh_rsc_probe+0x3d0/0x410 readl addr=0xffff800010710004
    rwmmio_write: rpmh_rsc_probe+0x3b0/0x410 writel addr=0xffff800010710d00 val=0x3
    rwmmio_write: write_tcs_cmd+0x6c/0x78 writel addr=0xffff800010710d30 val=0x10108

Cc: Jason Baron <jbaron@akamai.com>
Signed-off-by: Sai Prakash Ranjan <quic_saipraka@quicinc.com>
---
 include/linux/dynamic_debug.h     |  1 +
 include/linux/mmio-instrumented.h | 29 +++++++++++++++++++++++++++--
 lib/dynamic_debug.c               |  1 +
 3 files changed, 29 insertions(+), 2 deletions(-)

Comments

Steven Rostedt Nov. 9, 2021, 3:49 p.m. UTC | #1
On Tue, 9 Nov 2021 17:38:21 +0530
Sai Prakash Ranjan <quic_saipraka@quicinc.com> wrote:

> Debugging a specific driver or subsystem can be a lot easier if we can
> trace events specific to that driver or subsystem. This type of
> filtering can be achieved using existing dynamic debug library which
> provides a way to filter based on files, functions and modules.
> 
> Using this, provide an additional flag 'e' to filter event tracing to
> specified input.
> 
> For example, tracing all MMIO read/write can be overwhelming and of no
> use when debugging a specific driver or a subsystem. So switch to
> dynamic event tracing for register accesses.
> 
> Example: Tracing register accesses for all drivers in drivers/soc/qcom/*
> and the trace output is given below:
> 
>   # dyndbg="file drivers/soc/qcom/* +e" trace_event=rwmmio
>     or
>   # echo "file drivers/soc/qcom/* +e" > /sys/kernel/debug/dynamic_debug/control
>   # cat /sys/kernel/debug/tracing/trace

FYI, it's best to use /sys/kernel/tracing, as the debug/tracing is only
there for backward compatibility.

>     rwmmio_read: rpmh_rsc_probe+0x35c/0x410 readl addr=0xffff80001071000c
>     rwmmio_read: rpmh_rsc_probe+0x3d0/0x410 readl addr=0xffff800010710004
>     rwmmio_write: rpmh_rsc_probe+0x3b0/0x410 writel addr=0xffff800010710d00 val=0x3
>     rwmmio_write: write_tcs_cmd+0x6c/0x78 writel addr=0xffff800010710d30 val=0x10108

I'd much rather have a module name or something attached to the event that
ca be filtered on via the trace event filters, than having it determined by
some side effect done in another directory.

-- Steve
Sai Prakash Ranjan Nov. 9, 2021, 4:22 p.m. UTC | #2
Hi Steve,

On 11/9/2021 9:19 PM, Steven Rostedt wrote:
> On Tue, 9 Nov 2021 17:38:21 +0530
> Sai Prakash Ranjan <quic_saipraka@quicinc.com> wrote:
>
>> Debugging a specific driver or subsystem can be a lot easier if we can
>> trace events specific to that driver or subsystem. This type of
>> filtering can be achieved using existing dynamic debug library which
>> provides a way to filter based on files, functions and modules.
>>
>> Using this, provide an additional flag 'e' to filter event tracing to
>> specified input.
>>
>> For example, tracing all MMIO read/write can be overwhelming and of no
>> use when debugging a specific driver or a subsystem. So switch to
>> dynamic event tracing for register accesses.
>>
>> Example: Tracing register accesses for all drivers in drivers/soc/qcom/*
>> and the trace output is given below:
>>
>>    # dyndbg="file drivers/soc/qcom/* +e" trace_event=rwmmio
>>      or
>>    # echo "file drivers/soc/qcom/* +e" > /sys/kernel/debug/dynamic_debug/control
>>    # cat /sys/kernel/debug/tracing/trace
> FYI, it's best to use /sys/kernel/tracing, as the debug/tracing is only
> there for backward compatibility.

Ah I see, will correct it.


>>      rwmmio_read: rpmh_rsc_probe+0x35c/0x410 readl addr=0xffff80001071000c
>>      rwmmio_read: rpmh_rsc_probe+0x3d0/0x410 readl addr=0xffff800010710004
>>      rwmmio_write: rpmh_rsc_probe+0x3b0/0x410 writel addr=0xffff800010710d00 val=0x3
>>      rwmmio_write: write_tcs_cmd+0x6c/0x78 writel addr=0xffff800010710d30 val=0x10108
> I'd much rather have a module name or something attached to the event that
> ca be filtered on via the trace event filters, than having it determined by
> some side effect done in another directory.

I presume we don't have CALLER_MODULENAME0,1,2.. like CALLER_ADDR0,1,2 
without which we
cannot insert the module name to this trace event since MMIO accessors 
are defined in low level
arch headers and we won't get any useful module information from where 
these accessors are
called. The function name and the offset is good enough to identify the 
exact line and module after
post-processing with tools like GDB, objdump, so I feel we can keep the 
trace event fields limited?

Thanks,
Sai

>
> -- Steve
Steven Rostedt Nov. 9, 2021, 4:59 p.m. UTC | #3
On Tue, 9 Nov 2021 21:52:26 +0530
Sai Prakash Ranjan <quic_saipraka@quicinc.com> wrote:

> >>      rwmmio_read: rpmh_rsc_probe+0x35c/0x410 readl addr=0xffff80001071000c
> >>      rwmmio_read: rpmh_rsc_probe+0x3d0/0x410 readl addr=0xffff800010710004
> >>      rwmmio_write: rpmh_rsc_probe+0x3b0/0x410 writel addr=0xffff800010710d00 val=0x3
> >>      rwmmio_write: write_tcs_cmd+0x6c/0x78 writel addr=0xffff800010710d30 val=0x10108  
> > I'd much rather have a module name or something attached to the event that
> > ca be filtered on via the trace event filters, than having it determined by
> > some side effect done in another directory.  
> 
> I presume we don't have CALLER_MODULENAME0,1,2.. like CALLER_ADDR0,1,2 
> without which we
> cannot insert the module name to this trace event since MMIO accessors 
> are defined in low level
> arch headers and we won't get any useful module information from where 
> these accessors are
> called. The function name and the offset is good enough to identify the 
> exact line and module after
> post-processing with tools like GDB, objdump, so I feel we can keep the 
> trace event fields limited?

I'm thinking we can pass the descriptor to the event and not have it record
it. We could add a new field type for defining the event. Something like:

	__filter_field()

that has size zero in the event itself, but is available to the filtering
logic. Than perhaps we could pass that descriptor to the filter that has
all the information needed.

	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, width);
	log_read_mmio(width, addr, descriptor);

Where descriptor is NULL when dynamic debug in disabled.

Have a way to pass that descriptor to the filtering logic (I'll have to
take a look into it) and then be able to use the normal filtering.

This way you could also create multiple instances, where one instance
records the events coming from one file, and the other records events from
another file, and not have just one big switch that disables all calls to
log_read_mmio().

-- Steve
Sai Prakash Ranjan Nov. 9, 2021, 5:30 p.m. UTC | #4
Hi Steve,

On 11/9/2021 10:29 PM, Steven Rostedt wrote:
> On Tue, 9 Nov 2021 21:52:26 +0530
> Sai Prakash Ranjan <quic_saipraka@quicinc.com> wrote:
>
>>>>       rwmmio_read: rpmh_rsc_probe+0x35c/0x410 readl addr=0xffff80001071000c
>>>>       rwmmio_read: rpmh_rsc_probe+0x3d0/0x410 readl addr=0xffff800010710004
>>>>       rwmmio_write: rpmh_rsc_probe+0x3b0/0x410 writel addr=0xffff800010710d00 val=0x3
>>>>       rwmmio_write: write_tcs_cmd+0x6c/0x78 writel addr=0xffff800010710d30 val=0x10108
>>> I'd much rather have a module name or something attached to the event that
>>> ca be filtered on via the trace event filters, than having it determined by
>>> some side effect done in another directory.
>> I presume we don't have CALLER_MODULENAME0,1,2.. like CALLER_ADDR0,1,2
>> without which we
>> cannot insert the module name to this trace event since MMIO accessors
>> are defined in low level
>> arch headers and we won't get any useful module information from where
>> these accessors are
>> called. The function name and the offset is good enough to identify the
>> exact line and module after
>> post-processing with tools like GDB, objdump, so I feel we can keep the
>> trace event fields limited?
> I'm thinking we can pass the descriptor to the event and not have it record
> it. We could add a new field type for defining the event. Something like:
>
> 	__filter_field()
>
> that has size zero in the event itself, but is available to the filtering
> logic. Than perhaps we could pass that descriptor to the filter that has
> all the information needed.
>
> 	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, width);
> 	log_read_mmio(width, addr, descriptor);
>
> Where descriptor is NULL when dynamic debug in disabled.
>
> Have a way to pass that descriptor to the filtering logic (I'll have to
> take a look into it) and then be able to use the normal filtering.
>
> This way you could also create multiple instances, where one instance
> records the events coming from one file, and the other records events from
> another file, and not have just one big switch that disables all calls to
> log_read_mmio().
>
> -- Steve

Ah that's a very good idea, descriptor does contain the module, file name.
We can probably even pass the module name,file name as string from the 
descriptor itself to event?
Perhaps we can do that for all trace events and not just this trace 
event? Just like the trace event name displayed
with trace events, perhaps have file name,module name displayed when 
dynamic debug is enabled? Filtering by
filename is pretty useful since most of these usecases in debugging will 
be with respect to some driver or subsystems.

Thanks,
Sai
Steven Rostedt Nov. 9, 2021, 5:40 p.m. UTC | #5
On Tue, 9 Nov 2021 23:00:11 +0530
Sai Prakash Ranjan <quic_saipraka@quicinc.com> wrote:

> Ah that's a very good idea, descriptor does contain the module, file name.
> We can probably even pass the module name,file name as string from the 
> descriptor itself to event?
> Perhaps we can do that for all trace events and not just this trace 
> event? Just like the trace event name displayed
> with trace events, perhaps have file name,module name displayed when 
> dynamic debug is enabled? Filtering by
> filename is pretty useful since most of these usecases in debugging will 
> be with respect to some driver or subsystems.

If we add this for all events, it would require a lot of changes to all
users of tracepoints, as it would require adding a new parameter to the
callbacks.

We could add a flag in the registering that states that the callback is OK
for it, and it passes that data as well.

Let me look into this for a bit. I may not have something this week, but
we should look into this more before adding little hacks that do this one
at a time like this patch.

-- Steve
Sai Prakash Ranjan Nov. 9, 2021, 5:49 p.m. UTC | #6
On 11/9/2021 11:10 PM, Steven Rostedt wrote:
> On Tue, 9 Nov 2021 23:00:11 +0530
> Sai Prakash Ranjan <quic_saipraka@quicinc.com> wrote:
>
>> Ah that's a very good idea, descriptor does contain the module, file name.
>> We can probably even pass the module name,file name as string from the
>> descriptor itself to event?
>> Perhaps we can do that for all trace events and not just this trace
>> event? Just like the trace event name displayed
>> with trace events, perhaps have file name,module name displayed when
>> dynamic debug is enabled? Filtering by
>> filename is pretty useful since most of these usecases in debugging will
>> be with respect to some driver or subsystems.
> If we add this for all events, it would require a lot of changes to all
> users of tracepoints, as it would require adding a new parameter to the
> callbacks.
>
> We could add a flag in the registering that states that the callback is OK
> for it, and it passes that data as well.
>
> Let me look into this for a bit. I may not have something this week, but
> we should look into this more before adding little hacks that do this one
> at a time like this patch.
>
> -- Steve

Sure, thanks for the help and review. I can skip this patch adding 
support for dynamic event tracing
till we have something concrete as the previous patches doesn't depend 
on this.

Thanks,
Sai
Jason Baron Nov. 9, 2021, 9:42 p.m. UTC | #7
On 11/9/21 12:49 PM, Sai Prakash Ranjan wrote:
> On 11/9/2021 11:10 PM, Steven Rostedt wrote:
>> On Tue, 9 Nov 2021 23:00:11 +0530
>> Sai Prakash Ranjan <quic_saipraka@quicinc.com> wrote:
>>
>>> Ah that's a very good idea, descriptor does contain the module, file name.
>>> We can probably even pass the module name,file name as string from the
>>> descriptor itself to event?
>>> Perhaps we can do that for all trace events and not just this trace
>>> event? Just like the trace event name displayed
>>> with trace events, perhaps have file name,module name displayed when
>>> dynamic debug is enabled? Filtering by
>>> filename is pretty useful since most of these usecases in debugging will
>>> be with respect to some driver or subsystems.
>> If we add this for all events, it would require a lot of changes to all
>> users of tracepoints, as it would require adding a new parameter to the
>> callbacks.
>>
>> We could add a flag in the registering that states that the callback is OK
>> for it, and it passes that data as well.
>>
>> Let me look into this for a bit. I may not have something this week, but
>> we should look into this more before adding little hacks that do this one
>> at a time like this patch.
>>
>> -- Steve
> 
> Sure, thanks for the help and review. I can skip this patch adding support for dynamic event tracing
> till we have something concrete as the previous patches doesn't depend on this.
> 
> Thanks,
> Sai


Hi,

Yeah there is a 'parallel' thread about adding the tracing ring buffer as a 'back end' to the dynamic debug stuff over here:
https://lore.kernel.org/lkml/20211105192637.2370737-9-jim.cromie@gmail.com/

The attempt there is more generic but I realize now that it is adding the tracing to an 'instance' which is specific to dynamic debug which is being
created via: trace_array_get_by_name(). I would prefer to just have it print to the 'main' trace buffer such that it's easier to read, although I
guess they could still be consolidated via timestamps. Hmmm...I think there was a previous proposal to just add a single tracepoint (that takes a
string) to the dynamic debug layer that could be called if a dynamic debug site is enabled for trace buffer output. Would that satisfy the ftrace
level filtering requirements that you are looking for?

Thanks,

-Jason
Steven Rostedt Nov. 9, 2021, 9:51 p.m. UTC | #8
On Tue, 9 Nov 2021 16:42:48 -0500
Jason Baron <jbaron@akamai.com> wrote:

> Yeah there is a 'parallel' thread about adding the tracing ring buffer as
> a 'back end' to the dynamic debug stuff over here:
> https://lore.kernel.org/lkml/20211105192637.2370737-9-jim.cromie@gmail.com/

As the maintainer of tracefs, why am I not Cc'd on that thread :-(

I'll have to look at that thread later (no time now).

> 
> The attempt there is more generic but I realize now that it is adding the
> tracing to an 'instance' which is specific to dynamic debug which is
> being created via: trace_array_get_by_name(). I would prefer to just have
> it print to the 'main' trace buffer such that it's easier to read,
> although I guess they could still be consolidated via timestamps.
> Hmmm...I think there was a previous proposal to just add a single
> tracepoint (that takes a string) to the dynamic debug layer that could be
> called if a dynamic debug site is enabled for trace buffer output. Would
> that satisfy the ftrace level filtering requirements that you are looking
> for?

What we are looking at there is to pass the dynamic debug descriptor to the
trace event filtering logic, where you could filter on information passed
to it. For example, on a specific file if a trace event is called by
several different files or modules.

-- Steve
Jason Baron Nov. 9, 2021, 10:13 p.m. UTC | #9
On 11/9/21 4:51 PM, Steven Rostedt wrote:
> On Tue, 9 Nov 2021 16:42:48 -0500
> Jason Baron <jbaron@akamai.com> wrote:
> 
>> Yeah there is a 'parallel' thread about adding the tracing ring buffer as
>> a 'back end' to the dynamic debug stuff over here:
>> https://urldefense.com/v3/__https://lore.kernel.org/lkml/20211105192637.2370737-9-jim.cromie@gmail.com/__;!!GjvTz_vk!GdVm3olk4hfEx6PjfDDPg4cIbtY60aeqP0IOwu1-S0qJRBZ-OuvC5I_stkqniA$ 
> 
> As the maintainer of tracefs, why am I not Cc'd on that thread :-(
> 
> I'll have to look at that thread later (no time now).
Agreed - that's my bad. I meant to add you earlier in the discussion...

> 
>>
>> The attempt there is more generic but I realize now that it is adding the
>> tracing to an 'instance' which is specific to dynamic debug which is
>> being created via: trace_array_get_by_name(). I would prefer to just have
>> it print to the 'main' trace buffer such that it's easier to read,
>> although I guess they could still be consolidated via timestamps.
>> Hmmm...I think there was a previous proposal to just add a single
>> tracepoint (that takes a string) to the dynamic debug layer that could be
>> called if a dynamic debug site is enabled for trace buffer output. Would
>> that satisfy the ftrace level filtering requirements that you are looking
>> for?
> 
> What we are looking at there is to pass the dynamic debug descriptor to the
> trace event filtering logic, where you could filter on information passed
> to it. For example, on a specific file if a trace event is called by
> several different files or modules.
> 
> -- Steve

Ok, Could this be done at the dynamic debug level as it can already match on specific files and line numbers currently?

Thanks,

-Jason
Steven Rostedt Nov. 9, 2021, 10:28 p.m. UTC | #10
[ Hmm, should add Mathieu in on this discussion ]

On Tue, 9 Nov 2021 17:13:13 -0500
Jason Baron <jbaron@akamai.com> wrote:

> > What we are looking at there is to pass the dynamic debug descriptor to the
> > trace event filtering logic, where you could filter on information passed
> > to it. For example, on a specific file if a trace event is called by
> > several different files or modules.
> > 
> > -- Steve  
> 
> Ok, Could this be done at the dynamic debug level as it can already match
> on specific files and line numbers currently?

Not sure what you mean by that.

The idea was that this would only be enabled if dynamic debug is enabled
and that the DEFINE_DYNAMIC_DEBUG_METADATA() could be used at the
tracepoint function location (trace_foo()) by the tracepoint macros. And
then if one of the callbacks registered for the tracepoint had a
"dynamic_debug" flag set, it would be passed the descriptor in as a pointer.

And then, for example, the filtering logic of ftrace could then reference
the information of the event, if the user passed in something special.

 # echo 'DEBUG_FILE ~ "drivers/soc/qcom/*"' > events/rwmmio/rwmmio_write/filter
 # echo 1 > events/rwmmio/rwmmio_write/enable

And then only the rwmmio_write events that came from the qcom directory
would be printed.

We would create special event fields like "DEBUG_FILE", "DEBUG_FUNC",
"DEBUG_MOD", "DEBUG_LINE", etc, that could be used if dyndebug is enabled
in the kernel.

Of course this is going to bloat the kernel as it will create a dynamic
debug descriptor at every tracepoint location.

-- Steve
Jason Baron Nov. 10, 2021, 8:03 p.m. UTC | #11
On 11/9/21 5:28 PM, Steven Rostedt wrote:
> [ Hmm, should add Mathieu in on this discussion ]
> 
> On Tue, 9 Nov 2021 17:13:13 -0500
> Jason Baron <jbaron@akamai.com> wrote:
> 
>>> What we are looking at there is to pass the dynamic debug descriptor to the
>>> trace event filtering logic, where you could filter on information passed
>>> to it. For example, on a specific file if a trace event is called by
>>> several different files or modules.
>>>
>>> -- Steve  
>>
>> Ok, Could this be done at the dynamic debug level as it can already match
>> on specific files and line numbers currently?
> 
> Not sure what you mean by that.

I was just trying to say that via dynamic debug one could enable all debugs in a kernel source directory as in your example below via:
# echo "file drivers/soc/qcom/* +p" > /sys/kernel/debug/dynamic_debug/control

So if we are just looking for a printk here, one could just use pr_debug(). Or if we want that print to go the tracing ring buffer we have been
discussing how to add that as a 'backend' for dynamic debug as well.

If we want to use tracepionts, then yeah I think it's going to require adding the file, line, module data to each tracepoint site. I think this is
probably done via the tracing macros and then that could be filtered on, but yes that's going to add bloat.

The proposal here seems to be a mix of things - use the file control that dynamic debug already has to match on file name and then enable a tracepoint
that is behind it. That seems overly complex?

Thanks,

-Jason

> 
> The idea was that this would only be enabled if dynamic debug is enabled
> and that the DEFINE_DYNAMIC_DEBUG_METADATA() could be used at the
> tracepoint function location (trace_foo()) by the tracepoint macros. And
> then if one of the callbacks registered for the tracepoint had a
> "dynamic_debug" flag set, it would be passed the descriptor in as a pointer.
> 
> And then, for example, the filtering logic of ftrace could then reference
> the information of the event, if the user passed in something special.
> 
>  # echo 'DEBUG_FILE ~ "drivers/soc/qcom/*"' > events/rwmmio/rwmmio_write/filter
>  # echo 1 > events/rwmmio/rwmmio_write/enable
> 
> And then only the rwmmio_write events that came from the qcom directory
> would be printed.
> 
> We would create special event fields like "DEBUG_FILE", "DEBUG_FUNC",
> "DEBUG_MOD", "DEBUG_LINE", etc, that could be used if dyndebug is enabled
> in the kernel.
> 
> Of course this is going to bloat the kernel as it will create a dynamic
> debug descriptor at every tracepoint location.
> 
> -- Steve
>
Mathieu Desnoyers Nov. 11, 2021, 1:24 p.m. UTC | #12
----- On Nov 9, 2021, at 5:28 PM, rostedt rostedt@goodmis.org wrote:

> [ Hmm, should add Mathieu in on this discussion ]
> 
> On Tue, 9 Nov 2021 17:13:13 -0500
> Jason Baron <jbaron@akamai.com> wrote:
> 
>> > What we are looking at there is to pass the dynamic debug descriptor to the
>> > trace event filtering logic, where you could filter on information passed
>> > to it. For example, on a specific file if a trace event is called by
>> > several different files or modules.
>> > 
>> > -- Steve
>> 
>> Ok, Could this be done at the dynamic debug level as it can already match
>> on specific files and line numbers currently?
> 
> Not sure what you mean by that.
> 
> The idea was that this would only be enabled if dynamic debug is enabled
> and that the DEFINE_DYNAMIC_DEBUG_METADATA() could be used at the
> tracepoint function location (trace_foo()) by the tracepoint macros. And
> then if one of the callbacks registered for the tracepoint had a
> "dynamic_debug" flag set, it would be passed the descriptor in as a pointer.
> 
> And then, for example, the filtering logic of ftrace could then reference
> the information of the event, if the user passed in something special.
> 
> # echo 'DEBUG_FILE ~ "drivers/soc/qcom/*"' > events/rwmmio/rwmmio_write/filter
> # echo 1 > events/rwmmio/rwmmio_write/enable
> 
> And then only the rwmmio_write events that came from the qcom directory
> would be printed.
> 
> We would create special event fields like "DEBUG_FILE", "DEBUG_FUNC",
> "DEBUG_MOD", "DEBUG_LINE", etc, that could be used if dyndebug is enabled
> in the kernel.
> 
> Of course this is going to bloat the kernel as it will create a dynamic
> debug descriptor at every tracepoint location.

I think there is indeed value in doing this. Where I'm not sure is regarding
how we allow this to be enabled/configured.

The way I see it, it might be sufficient and simpler to do just something along
those lines:

- Introduce a new struct tracepoint_caller_info, which would contain information
  about file, line number and module name where each trace_*() statement is located.
- Add a new CONFIG_TRACEPOINT_CALLER_INFO which generates this new structure at
  build time for kernel and modules. This would indeed bloat the kernel, but it's
  a build-time configurable trade-off.
- Change the prototype for the tracepoint callbacks to add an additional argument
  "struct tracepoint_caller_info *caller_info". When CONFIG_TRACEPOINT_CALLER_INFO
  is disabled, simply have this pointer be NULL. When CONFIG_TRACEPOINT_CALLER_INFO
  is enabled, pass the tracepoint's caller_info structure as parameter.

It should be straightforward to adapt the tracepoint callback prototypes within each
user within the Linux kernel tree. And for out-of-tree users, they have to adapt to
that kind of change already anyway.

Thoughts ?

Thanks,

Mathieu
diff mbox series

Patch

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dce631e678dd..80a1ae234a3b 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -32,6 +32,7 @@  struct _ddebug {
 #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
 #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
 #define _DPRINTK_FLAGS_INCL_TID		(1<<4)
+#define _DPRINTK_FLAGS_EVENT		(1<<5)
 
 #define _DPRINTK_FLAGS_INCL_ANY		\
 	(_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
diff --git a/include/linux/mmio-instrumented.h b/include/linux/mmio-instrumented.h
index 4304224f3be4..4ff5af4bbee8 100644
--- a/include/linux/mmio-instrumented.h
+++ b/include/linux/mmio-instrumented.h
@@ -6,6 +6,7 @@ 
 #ifndef _LINUX_MMIO_INSTRUMENTED_H
 #define _LINUX_MMIO_INSTRUMENTED_H
 
+#include <linux/dynamic_debug.h>
 #include <linux/tracepoint-defs.h>
 
 /*
@@ -25,7 +26,7 @@  void log_read_mmio(const char *width, const volatile void __iomem *addr);
 #define __raw_write(v, a, _l)	({				\
 	volatile void __iomem *_a = (a);			\
 	if (tracepoint_enabled(rwmmio_write))			\
-		log_write_mmio(__stringify(write##_l), v, _a);	\
+		dynamic_log_write_mmio(__stringify(write##_l), v, _a);\
 	arch_raw_write##_l((v), _a);				\
 	})
 
@@ -38,7 +39,7 @@  void log_read_mmio(const char *width, const volatile void __iomem *addr);
 	_t __a;							\
 	const volatile void __iomem *_a = (a);			\
 	if (tracepoint_enabled(rwmmio_read))			\
-		log_read_mmio(__stringify(read##_l), _a);	\
+		dynamic_log_read_mmio(__stringify(read##_l), _a);\
 	__a = arch_raw_read##_l(_a);				\
 	__a;							\
 	})
@@ -48,6 +49,26 @@  void log_read_mmio(const char *width, const volatile void __iomem *addr);
 #define __raw_readl(a)		__raw_read((a), l, u32)
 #define __raw_readq(a)		__raw_read((a), q, u64)
 
+#if defined(CONFIG_DYNAMIC_DEBUG)
+#define dynamic_log_write_mmio(width, value, addr)		\
+do {								\
+	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, width);	\
+	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_EVENT))	\
+		log_write_mmio(width, value, addr);		\
+} while (0)
+
+#define dynamic_log_read_mmio(width, addr)			\
+do {								\
+	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, width);	\
+	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_EVENT))	\
+		log_read_mmio(width, addr);			\
+} while (0)
+
+#else
+#define dynamic_log_write_mmio(width, val, addr)	log_write_mmio(width, val, addr)
+#define dynamic_log_read_mmio(width, addr)		log_read_mmio(width, addr)
+#endif /* CONFIG_DYNAMIC_DEBUG */
+
 #else
 
 #define __raw_writeb(v, a)	arch_raw_writeb(v, a)
@@ -64,6 +85,10 @@  static inline void log_write_mmio(const char *width, u64 val,
 				  volatile void __iomem *addr) {}
 static inline void log_read_mmio(const char *width,
 				 const volatile void __iomem *addr) {}
+static inline void dynamic_log_write_mmio(const char *width, u64 val,
+				  volatile void __iomem *addr) {}
+static inline void dynamic_log_read_mmio(const char *width,
+				 const volatile void __iomem *addr) {}
 
 #endif /* CONFIG_TRACE_MMIO_ACCESS */
 
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index dd7f56af9aed..a852073089d9 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -87,6 +87,7 @@  static inline const char *trim_prefix(const char *path)
 
 static struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_PRINT, 'p' },
+	{ _DPRINTK_FLAGS_EVENT, 'e' },
 	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
 	{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
 	{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },