Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] pseries: Add DPRINTF macros to spapr pci code

[David Gibson]

On Wed, Apr 04, 2012 at 08:34:52PM +0000, Blue Swirl wrote:
> On Wed, Apr 4, 2012 at 20:11, Peter Maydell <address@hidden> wrote:
> > On 4 April 2012 20:18, Blue Swirl <address@hidden> wrote:
> >> On Mon, Apr 2, 2012 at 04:17, David Gibson <address@hidden> wrote:
> >>> From: Alexey Kardashevskiy <address@hidden>
> >>> This adds DPRINTF() macros with the usual conventions to the spapr_pci
> >>> code.
> >>
> >> Please use tracepoints instead of printf statements. Tracing is more
> >> flexible, more efficient and does not suffer from bitrot.
> >
> > I'd much rather enable a #define to turn on debugging than faff about
> > with tracing. It's simple and straightforward, you can do it with a
> > single obvious change and recompile, and nobody has to look up
> > documentation to figure out how it works.
> 
> Laziness. Even the built in help should be sufficient to start using
> tracepoints.
> 
> > If tracepoints were always-compiled-in and enabled at runtime I'd
> > agree with you: then you could have linux-kernel-style "enable debug
> > tracing", "enable warnings about odd guest behaviour", "be silent",
> > etc. But they're not, so they don't gain anything over a simple
> > DPRINTF for programmer debugging.
> 
> False. It's easy to compile in tracepoints
> (--enable-trace-backend=simple) and the overhead is zero or marginal.
> It's not possible to enable or disable DPRINTFs at run time easily
> which is trivial with tracepoints. When the tracepoints are enabled,
> due to binary format they are faster than text output.  There's no
> need to relink the files when triggering on and off tracepoints. Trace
> files are much more compact than text. Processing is offline. Delta
> timestamps are provided. Tracepoints do not suffer from bitrot. From
> programming standpoint, DPRINTF is changed to a function call and the
> format text is put to a different file, that's it.
> 
> Have you ever waded through hundreds of megabytes of DPRINTF stuff or
> other logs, 99.99999% (literally, only a few bytes were interesting!)
> of it not what you want, just because the interesting stuff happens to
> be near to end of it? Even 'vi' is slow when handling these kind of
> files.
> 
> Just for fun, try also enabling all DPRINTFs (small sed script
> exercise left to reader) and see how many breakages there are due to
> bit rotted DPRINTFs.

Madness.  Tracepoints may well be useful, but dprintfs are simple,
used in projects all over the place and instantly understandable by
developers.

Yes, I've frequently waded through hundred megabyte debug logs to find
the few useful bugs.  I'd do it again without hesitation.  The logs
may be big, but they're transient and grep works very well, so I
really don't care.  Not to mention that the fact you can switch on the
dprintfs just for the code you're looking at means they're frequently
not that big in the first place.

Bitrot is also not a big problem.  If you're enabling dprintfs, almost
by definition, you're already hacking the code they appear in, so
fixing some bitrotted format specifiers is rarely more than a moment's
work.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson