[Qemu-devel] [PATCH for-2.10 0/2] trace: fix simpletrace.stp flight recorder mode

Stefan Hajnoczi posted 2 patches 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170814162142.29666-1-stefanha@redhat.com
Test FreeBSD passed
Test checkpatch failed
Test docker passed
Test s390x passed
There is a newer version of this series
scripts/simpletrace.py                       | 24 ++++++++++++++-----
scripts/tracetool/format/simpletrace_stap.py | 36 ++++++++++++----------------
2 files changed, 33 insertions(+), 27 deletions(-)
[Qemu-devel] [PATCH for-2.10 0/2] trace: fix simpletrace.stp flight recorder mode
Posted by Stefan Hajnoczi 6 years, 8 months ago
The SystemTap flight recorder mode no longer works with simpletrace.stp because
the event ID mapping records are emitted the first time an event fires.
Chances are, the event ID mapping record will not be in ring buffer when the
users wants to print the trace.

This series solves the issue by using the trace-events-all global event
ordering for event IDs for simpletrace.stp.

Stefan Hajnoczi (2):
  trace: use static event ID mapping in simpletrace.stp
  simpletrace: fix flight recorder --no-header option

 scripts/simpletrace.py                       | 24 ++++++++++++++-----
 scripts/tracetool/format/simpletrace_stap.py | 36 ++++++++++++----------------
 2 files changed, 33 insertions(+), 27 deletions(-)

-- 
2.13.4


Re: [Qemu-devel] [PATCH for-2.10 0/2] trace: fix simpletrace.stp flight recorder mode
Posted by Daniel P. Berrange 6 years, 8 months ago
On Mon, Aug 14, 2017 at 05:21:40PM +0100, Stefan Hajnoczi wrote:
> The SystemTap flight recorder mode no longer works with simpletrace.stp because
> the event ID mapping records are emitted the first time an event fires.
> Chances are, the event ID mapping record will not be in ring buffer when the
> users wants to print the trace.
> 
> This series solves the issue by using the trace-events-all global event
> ordering for event IDs for simpletrace.stp.

I see the way you've written it means it'll still honour the event
id mappings in the header, if present, but I wonder what's the point
of keeping that anymore ? If we need to support this ring-buffer mode,
there's no way we can take advantage of having the ID mappings dynamically
defined - we must always guarantee a stable mapping.

IOW, should we just revert

  commit 7f1b588f20d027730676e627713ae3bbf6baab04
  Author: Daniel P. Berrange <berrange@redhat.com>
  Date:   Tue Oct 4 14:35:50 2016 +0100

    trace: emit name <-> ID mapping in simpletrace header
    
entirely ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH for-2.10 0/2] trace: fix simpletrace.stp flight recorder mode
Posted by Stefan Hajnoczi 6 years, 8 months ago
On Mon, Aug 14, 2017 at 05:39:08PM +0100, Daniel P. Berrange wrote:
> On Mon, Aug 14, 2017 at 05:21:40PM +0100, Stefan Hajnoczi wrote:
> > The SystemTap flight recorder mode no longer works with simpletrace.stp because
> > the event ID mapping records are emitted the first time an event fires.
> > Chances are, the event ID mapping record will not be in ring buffer when the
> > users wants to print the trace.
> > 
> > This series solves the issue by using the trace-events-all global event
> > ordering for event IDs for simpletrace.stp.
> 
> I see the way you've written it means it'll still honour the event
> id mappings in the header, if present, but I wonder what's the point
> of keeping that anymore ? If we need to support this ring-buffer mode,
> there's no way we can take advantage of having the ID mappings dynamically
> defined - we must always guarantee a stable mapping.
> 
> IOW, should we just revert
> 
>   commit 7f1b588f20d027730676e627713ae3bbf6baab04
>   Author: Daniel P. Berrange <berrange@redhat.com>
>   Date:   Tue Oct 4 14:35:50 2016 +0100
> 
>     trace: emit name <-> ID mapping in simpletrace header
>     
> entirely ?

We cannot revert the commit because QEMU dynamically assigns event IDs
on startup.  Event IDs are not known ahead of time when the 'simple'
tracer is used.

Event IDs are only static when simpletrace.stp is used with the 'dtrace'
tracer since simpletrace.stp has global knowledge of all trace events
ahead of time.

Stefan
Re: [Qemu-devel] [PATCH for-2.10 0/2] trace: fix simpletrace.stp flight recorder mode
Posted by Stefan Hajnoczi 6 years, 8 months ago
On Mon, Aug 14, 2017 at 05:39:08PM +0100, Daniel P. Berrange wrote:
> On Mon, Aug 14, 2017 at 05:21:40PM +0100, Stefan Hajnoczi wrote:
> > The SystemTap flight recorder mode no longer works with simpletrace.stp because
> > the event ID mapping records are emitted the first time an event fires.
> > Chances are, the event ID mapping record will not be in ring buffer when the
> > users wants to print the trace.
> > 
> > This series solves the issue by using the trace-events-all global event
> > ordering for event IDs for simpletrace.stp.
> 
> I see the way you've written it means it'll still honour the event
> id mappings in the header, if present, but I wonder what's the point
> of keeping that anymore ? If we need to support this ring-buffer mode,
> there's no way we can take advantage of having the ID mappings dynamically
> defined - we must always guarantee a stable mapping.
> 
> IOW, should we just revert
> 
>   commit 7f1b588f20d027730676e627713ae3bbf6baab04
>   Author: Daniel P. Berrange <berrange@redhat.com>
>   Date:   Tue Oct 4 14:35:50 2016 +0100
> 
>     trace: emit name <-> ID mapping in simpletrace header
>     
> entirely ?

Follow-up to my reply:

We can revert just the changes to simpletrace_stap.py.  This means:

1. Event ID mapping records are no longer emitted.
2. No simpletrace file header is emitted.

I like this idea because it's simpler than my current patch.  Users will
have to use the ./simpletrace.py --no-header option again when printing
simpletrace.stp traces, but that isn't a problem.

Stefan
Re: [Qemu-devel] [PATCH for-2.10 0/2] trace: fix simpletrace.stp flight recorder mode
Posted by no-reply@patchew.org 6 years, 8 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20170814162142.29666-1-stefanha@redhat.com
Subject: [Qemu-devel] [PATCH for-2.10 0/2] trace: fix simpletrace.stp flight recorder mode
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20170814164002.23982-1-brijesh.singh@amd.com -> patchew/20170814164002.23982-1-brijesh.singh@amd.com
Switched to a new branch 'test'
45e9c56a4f simpletrace: fix flight recorder --no-header option
192d4fc2bc trace: use static event ID mapping in simpletrace.stp

=== OUTPUT BEGIN ===
Checking PATCH 1/2: trace: use static event ID mapping in simpletrace.stp...
Checking PATCH 2/2: simpletrace: fix flight recorder --no-header option...
ERROR: line over 90 characters
#39: FILE: scripts/simpletrace.py:101:
+    """Deserialize trace records from a file, yielding record tuples (event_num, timestamp, pid, arg1, ..., arg6).

total: 1 errors, 0 warnings, 46 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH for-2.10 0/2] trace: fix simpletrace.stp flight recorder mode
Posted by Stefan Hajnoczi 6 years, 8 months ago
On Mon, Aug 14, 2017 at 09:45:19AM -0700, no-reply@patchew.org wrote:
> === OUTPUT BEGIN ===
> Checking PATCH 1/2: trace: use static event ID mapping in simpletrace.stp...
> Checking PATCH 2/2: simpletrace: fix flight recorder --no-header option...
> ERROR: line over 90 characters
> #39: FILE: scripts/simpletrace.py:101:
> +    """Deserialize trace records from a file, yielding record tuples (event_num, timestamp, pid, arg1, ..., arg6).

This line was not changed by this patch.

Stefan