All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christophe de Dinechin <dinechin@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Michael Tokarev" <mjt@tls.msk.ru>,
	"Laurent Vivier" <laurent@vivier.eu>,
	qemu-devel@nongnu.org,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH v2 3/3] trace: Example of "centralized" recorder tracing
Date: Fri, 03 Jul 2020 12:12:14 +0200	[thread overview]
Message-ID: <lywo3kc3gh.fsf@redhat.com> (raw)
In-Reply-To: <20200702134713.GH152912@stefanha-x1.localdomain>


On 2020-07-02 at 15:47 CEST, Stefan Hajnoczi wrote...
> On Wed, Jul 01, 2020 at 05:15:01PM +0100, Daniel P. Berrangé wrote:
>> On Wed, Jul 01, 2020 at 05:09:06PM +0100, Stefan Hajnoczi wrote:
>> > On Tue, Jun 30, 2020 at 01:41:36PM +0100, Daniel P. Berrangé wrote:
>> > > On Fri, Jun 26, 2020 at 06:27:06PM +0200, Christophe de Dinechin wrote:
>> > > IMHO the whole point of having the pluggable trace backend impls, is
>> > > precisely that we don't have to add multiple different calls in the
>> > > code. A single trace_qemu_mutex_unlock() is supposed to work with
>> > > any backend.
>> >
>> > I think an exception is okay when the other trace backends do not offer
>> > equivalent functionality.
>> >
>> > Who knows if anyone other than Christophe will use this functionality,
>> > but it doesn't cost much to allow it.
>>
>> This patch is just an example though, suggesting this kind of usage is
>> expected to done in other current trace probe locations. The trace wrapper
>> has most of the information required already including a format string,
>> so I'd think it could be wired up to the generator so we don't add extra
>> record() statements through the codebase.

The primary purpose of the recorder is post-mortem dumps, flight recorder
style. Tracing is only a secondary benefit. Not sure if it's worth making a
distinction between events you want to record and those you want to trace.
(Example: You might want to record all command line options, but almost
never trace them)

> At most it should require an
>> extra annotation in the trace-events file to take the extra parameter
>> for grouping, and other trace backends can ignore that.
>
> It's true, it may be possible to put this functionality in the
> trace-events.

Let me think more about integrating these features with other trace
backends. See below for short-term impact.

> Christophe: how does this differ from regular trace events and what
> extra information is needed?

- Grouping, as indicated above, mostly useful in practice to make selection
  of tracing topics easy (e.g. "modules") but also for real-time graphing,
  because typically a state change occurs in different functions, which is
  why I used locking as an example.

- Self-documentation. Right now, the recorder back-end generates rather
  unhelpful help messages.

- Trace buffer size. This is important to make post-mortem dumps usable if
  you record infrequent events alongside much higher-rate ones. For example,
  you may want a large buffer to record info about command-line option
  processing, the default 8 is definitely too small.

- Support for %+s, which tells that a string is safe to print later (e.g. it
  is a compile-time constant, or never ever freed).

- Support for custom formats, e.g. I use %v in the XL compiler for LLVM
  value pointers. This is a bit more advanced, but it would be neat to be
  able to print out QOM objects using %q :-)

For the short term, what about providing trace-named wrappers around these
additional recorder features?

--
Cheers,
Christophe de Dinechin (IRC c3d)



  reply	other threads:[~2020-07-03 10:16 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-26 16:27 [PATCH v2 0/3] trace: Add a trace backend for the recorder library Christophe de Dinechin
2020-06-26 16:27 ` [PATCH v2 1/3] Makefile: Compute libraries for libqemuutil.a and libvhost-user.a Christophe de Dinechin
2020-06-30 12:23   ` Stefan Hajnoczi
2020-06-26 16:27 ` [PATCH v2 2/3] trace: Add support for recorder back-end Christophe de Dinechin
2020-06-30  9:05   ` Dr. David Alan Gilbert
2020-06-30 13:28     ` Christophe de Dinechin
2020-06-30 17:46       ` Dr. David Alan Gilbert
2020-06-30 12:55   ` Stefan Hajnoczi
2020-06-30 13:02   ` Daniel P. Berrangé
2020-07-23 12:12     ` Christophe de Dinechin
2020-07-23 14:06       ` Markus Armbruster
2020-07-23 16:15         ` Christophe de Dinechin
2020-07-27  8:23           ` Markus Armbruster
2020-07-28 11:49             ` Christophe de Dinechin
2020-07-29 11:53               ` Markus Armbruster
2020-07-29 15:52                 ` Christophe de Dinechin
2020-07-30  8:13                   ` Markus Armbruster
2020-07-30 10:29                     ` Christophe de Dinechin
2020-06-26 16:27 ` [PATCH v2 3/3] trace: Example of "centralized" recorder tracing Christophe de Dinechin
2020-06-30 12:41   ` Daniel P. Berrangé
2020-07-01 16:09     ` Stefan Hajnoczi
2020-07-01 16:15       ` Daniel P. Berrangé
2020-07-02 13:47         ` Stefan Hajnoczi
2020-07-03 10:12           ` Christophe de Dinechin [this message]
2020-07-03 13:08             ` Daniel P. Berrangé
2020-07-03 16:45               ` Christophe de Dinechin
2020-06-30 12:58   ` Stefan Hajnoczi
2020-06-26 16:34 ` [PATCH v2 0/3] trace: Add a trace backend for the recorder library no-reply
2020-06-30 12:59 ` Stefan Hajnoczi
2020-07-03 10:37   ` Christophe de Dinechin
2020-07-03 11:27     ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=lywo3kc3gh.fsf@redhat.com \
    --to=dinechin@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=laurent@vivier.eu \
    --cc=mjt@tls.msk.ru \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.