Re: [PATCH v10 08/10] dyndbg: add print-to-tracefs, selftest with it - RFC

From: Jason Baron <jbaron@akamai.com>
To: Vincent Whitchurch <vincent.whitchurch@axis.com>,
	Jim Cromie <jim.cromie@gmail.com>
Cc: gregkh@linuxfoundation.org, robdclark@gmail.com, sean@poorly.run,
	daniel.vetter@ffwll.ch, seanpaul@chromium.org, lyude@redhat.com,
	linux-kernel@vger.kernel.org, rostedt@goodmis.org,
	mathieu.desnoyers@efficios.com, dri-devel@lists.freedesktop.org,
	amd-gfx@lists.freedesktop.org,
	intel-gvt-dev@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org, quic_saipraka@quicinc.com,
	will@kernel.org, catalin.marinas@arm.com,
	quic_psodagud@quicinc.com, maz@kernel.org, arnd@arndb.de,
	linux-arm-kernel@lists.infradead.org,
	linux-arm-msm@vger.kernel.org, mingo@redhat.com
Subject: Re: [PATCH v10 08/10] dyndbg: add print-to-tracefs, selftest with it - RFC
Date: Fri, 12 Nov 2021 10:08:41 -0500	[thread overview]
Message-ID: <f3914fa9-8b22-d54e-3f77-d998e74094b9@akamai.com> (raw)
In-Reply-To: <20211112114953.GA1381@axis.com>

On 11/12/21 6:49 AM, Vincent Whitchurch wrote:
> On Thu, Nov 11, 2021 at 03:02:04PM -0700, Jim Cromie wrote:
>> Sean Paul proposed, in:
>> https://urldefense.com/v3/__https://patchwork.freedesktop.org/series/78133/__;!!GjvTz_vk!HcKnMRByYkIdyF1apqQjlN5aBIomzJR1an3YWXM6KXs0EftVMQdrewRA8Dki4A$ 
>> drm/trace: Mirror DRM debug logs to tracefs
>>
>> His patchset's objective is to be able to independently steer some of
>> the drm.debug stream to an alternate tracing destination, by splitting
>> drm_debug_enabled() into syslog & trace flavors, and enabling them
>> separately.  2 advantages were identified:
>>
>> 1- syslog is heavyweight, tracefs is much lighter
>> 2- separate selection of enabled categories means less traffic
>>
>> Dynamic-Debug can do 2nd exceedingly well:
>>
>> A- all work is behind jump-label's NOOP, zero off cost.
>> B- exact site selectivity, precisely the useful traffic.
>>    can tailor enabled set interactively, at shell.
>>
>> Since the tracefs interface is effective for drm (the threads suggest
>> so), adding that interface to dynamic-debug has real potential for
>> everyone including drm.
>>
>> if CONFIG_TRACING:
>>
>> Grab Sean's trace_init/cleanup code, use it to provide tracefs
>> available by default to all pr_debugs.  This will likely need some
>> further per-module treatment; perhaps something reflecting hierarchy
>> of module,file,function,line, maybe with a tuned flattening.
>>
>> endif CONFIG_TRACING
>>
>> Add a new +T flag to enable tracing, independent of +p, and add and
>> use 3 macros: dyndbg_site_is_enabled/logging/tracing(), to encapsulate
>> the flag checks.  Existing code treats T like other flags.
> 
> I posted a patchset a while ago to do something very similar, but that
> got stalled for some reason and I unfortunately didn't follow it up:
> 
>  https://urldefense.com/v3/__https://lore.kernel.org/lkml/20200825153338.17061-1-vincent.whitchurch@axis.com/__;!!GjvTz_vk!HcKnMRByYkIdyF1apqQjlN5aBIomzJR1an3YWXM6KXs0EftVMQdrewRGytKHPg$ 
> 
> A key difference between that patchset and this patch (besides that
> small fact that I used +x instead of +T) was that my patchset allowed
> the dyndbg trace to be emitted to the main buffer and did not force them
> to be in an instance-specific buffer.

Yes, I agree I'd prefer that we print here to the 'main' buffer - it seems to keep things simpler and easier to combine the output from different
sources as you mentioned.

Thanks,

-Jason

> 
> That feature is quite important at least for my use case since I often
> use dyndbg combined with function tracing, and the latter doesn't work
> on non-main instances according to Documentation/trace/ftrace.rst.
> 
> For example, here's a random example of a bootargs from one of my recent
> debugging sessions:
> 
>  trace_event=printk:* ftrace_filter=_mmc*,mmc*,sd*,dw_mci*,mci*
>  ftrace=function trace_buf_size=20M dyndbg="file drivers/mmc/* +x"
> 