Date:   Thu, 11 Nov 2021 08:24:34 -0500 (EST)
From:   Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To:     rostedt <rostedt@goodmis.org>
Cc:     jbaron <jbaron@akamai.com>,
        Sai Prakash Ranjan <quic_saipraka@quicinc.com>,
        Will Deacon <will@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        quic psodagud <quic_psodagud@quicinc.com>,
        maz <maz@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Arnd Bergmann <arnd@arndb.de>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-arm-msm@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
        jim cromie <jim.cromie@gmail.com>, seanpaul@chromium.org
Message-ID: <1665247433.56.1636637074878.JavaMail.zimbra@efficios.com>
In-Reply-To: <20211109172848.304b1c19@gandalf.local.home>
References: <cover.1636452784.git.quic_saipraka@quicinc.com> <264b77dd-5509-60f9-248c-a93135b01aa9@quicinc.com> <20211109124046.2a772bcb@gandalf.local.home> <c5715db5-965b-c1f5-3e99-04caec3d4f2c@quicinc.com> <e037f449-9784-c78e-431d-43f035a9f49f@akamai.com> <20211109165104.176b4cf9@gandalf.local.home> <55a9fe7b-5573-0f80-e075-758b377a6c47@akamai.com> <20211109172848.304b1c19@gandalf.local.home>
Subject: Re: [PATCHv3 3/3] dynamic_debug: Add a flag for dynamic event
 tracing
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: dynamic_debug: Add a flag for dynamic event tracing
Thread-Index: ETB17VPNXK5+4Gp8n5NYuqqK7RA4yg==
Precedence: bulk

----- 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

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com