On 11/18/21 10:24 AM, Pekka Paalanen wrote:

On Thu, 18 Nov 2021 09:29:27 -0500
Jason Baron [off-list ref] wrote:

quoted

On 11/16/21 3:46 AM, Pekka Paalanen wrote:

quoted

On Fri, 12 Nov 2021 10:08:41 -0500
Jason Baron [off-list ref] wrote:
  

quoted

On 11/12/21 6:49 AM, Vincent Whitchurch wrote:  

quoted

On Thu, Nov 11, 2021 at 03:02:04PM -0700, Jim Cromie wrote:    

quoted

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.  

Hi,

I'm not quite sure I understand this discussion, but I would like to
remind you all of what Sean's original work is about:

Userspace configures DRM tracing into a flight recorder buffer (I guess
this is what you refer to "instance-specific buffer").

Userspace runs happily for months, and then hits a problem: a failure
in the DRM sub-system most likely, e.g. an ioctl that should never
fail, failed. Userspace handles that failure by dumping the flight
recorder buffer into a file and saving or sending a bug report. The
flight recorder contents give a log of all relevant DRM in-kernel
actions leading to the unexpected failure to help developers debug it.

I don't mind if one can additionally send the flight recorder stream to
the main buffer, but I do want the separate flight recorder buffer to
be an option so that a) unrelated things cannot flood the interesting
bits out of it, and b) the scope of collected information is relevant.

The very reason for this work is problems that are very difficult to
reproduce in practice, either because the problem itself is triggered
very rarely and randomly, or because the end users of the system have
either no knowledge or no access to reconfigure debug logging and then
reproduce the problem with good debug logs.

Thank you very much for pushing this work forward!

  

So I think Vincent (earlier in the thread) was saying that he finds it
very helpful have dynamic debug output go to the 'main' trace buffer,
while you seem to be saying you'd prefer it just go to dynamic debug
specific trace buffer.

Seems like we have different use cases: traditional debugging, and
in-production flight recorder for problem reporting. I'm not surprised
if they need different treatment.

quoted

So we certainly can have dynamic output potentially go to both places -
although I think this would mean two tracepoints? But I really wonder
if we really need a separate tracing buffer for dynamic debug when
what goes to the 'main' buffer can be controlled and filtered to avoid
your concern around a 'flood'?

If the DRM tracing goes into the main buffer, then systems in
production cannot have any other sub-system traced in a similar
fashion. To me it would feel very arrogant to say that to make use of
DRM flight recording, you cannot trace much or anything else.

The very purpose of the flight recorder is run in production all the
time, not in a special debugging session.

There is also the question of access and contents of the trace buffer.
Ultimately, if automatic bug reports are enabled in a system, the
contents of the trace buffer would be sent as-is to some bug tracking
system. If there is a chance to put non-DRM stuff in the trace buffer,
that could be a security problem.

My use case is Weston. When Weston encounters an unexpected problem in
production, something should automatically capture the DRM flight
recorder contents and save it alongside the Weston log. Would be really
nice if Weston itself could do that, but I suspect it is going to need
root privileges so it needs some helper daemon.

Maybe Sean can reiterate their use case more?


Thanks,
pq

Ok, so in this current thread the proposal was to create a "dyndbg-tracefs"
buffer to put the dynamic debug output (including drm output from dynamic
debug) into. And I was saying let's just put in the 'main' trace buffer
(predicated on a dynamic debug specific tracepoint), since there seems
to be a a use-case for that and it keeps things simpler.

But I went back to Sean's original patch, and it creates a drm specific
trace buffer "drm" (via trace_array_get_by_name("drm")). Here:
https://patchwork.freedesktop.org/patch/445549/?series=78133&rev=5

So I think that may be some of the confusion here? The current thread/
proposal is not for a drm specific trace buffer...

Having a subsystem specific trace buffer would allow subsystem specific
trace log permissions depending on the sensitivity of the data. But
doesn't drm output today go to the system log which is typically world
readable today?

So I could see us supporting subsystem specific trace buffer output
via dynamic debug here. We could add new dev_debug() variants that
allow say a trace buffer to be supplied. So in that way subsystems
could 'opt-out' of having their data put into the global trace buffer.
And perhaps some subsystems we would want to allow output to both
buffers? The subsystem specific one and the global one?

Thanks,

-Jason





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel