Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <264b77dd-5509-60f9-248c-a93135b01aa9@quicinc.com>
Date:   Tue, 9 Nov 2021 23:00:11 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.1.2
Subject: Re: [PATCHv3 3/3] dynamic_debug: Add a flag for dynamic event tracing
Content-Language: en-US
To:     Steven Rostedt <rostedt@goodmis.org>
CC:     Will Deacon <will@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        <quic_psodagud@quicinc.com>, Marc Zyngier <maz@kernel.org>,
        <gregkh@linuxfoundation.org>, <arnd@arndb.de>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <mingo@redhat.com>, <jbaron@akamai.com>, <jim.cromie@gmail.com>
References: <cover.1636452784.git.quic_saipraka@quicinc.com>
 <3706af20bc64a320ff8f3ff8950738b988f4bdf5.1636452784.git.quic_saipraka@quicinc.com>
 <20211109104941.2d50eafc@gandalf.local.home>
 <f7c665b9-dc17-5a7f-de80-9fa0605721fc@quicinc.com>
 <20211109115951.1c2b5228@gandalf.local.home>
From:   Sai Prakash Ranjan <quic_saipraka@quicinc.com>
In-Reply-To: <20211109115951.1c2b5228@gandalf.local.home>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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