qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Pranith Kumar <bobby.prani+qemu@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	"Emilio G. Cota" <cota@braap.org>,
	qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH v2 6/6] new script/analyse-tlb-flushes-simpletrace.py
Date: Wed, 31 May 2017 08:53:42 +0100	[thread overview]
Message-ID: <8737bltqbd.fsf@linaro.org> (raw)
In-Reply-To: <CAJhHMCB0j9=6XBMa0GVaOt+Acf4m5Ow2DhvvvcekKJXhfM+qSg@mail.gmail.com>


Pranith Kumar <bobby.prani+qemu@gmail.com> writes:

> Hi Alex,
>
> Please find some comments and questions below:
>
> On Wed, May 17, 2017 at 10:52 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
>> This is a simple helper script to extract TLB flush stats from the a
>> simpletrace file and plot the results.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> ---
>> v2
>>   - re-factored for new trace events
>>   - added time and latency graphs
>> ---
>>  scripts/analyse-tlb-flushes-simpletrace.py | 144 +++++++++++++++++++++++++++++
>>  1 file changed, 144 insertions(+)
>>  create mode 100755 scripts/analyse-tlb-flushes-simpletrace.py
>>
>> diff --git a/scripts/analyse-tlb-flushes-simpletrace.py b/scripts/analyse-tlb-flushes-simpletrace.py
>> new file mode 100755
>> index 0000000000..03fab8c86b
>> --- /dev/null
>> +++ b/scripts/analyse-tlb-flushes-simpletrace.py
>
> <snip>
>
>> +
>> +def get_args():
>> +    "Grab options"
>> +    parser = argparse.ArgumentParser()
>> +    parser.add_argument("--output", "-o", type=str, help="Render plot to file")
>> +    parser.add_argument("--vcpus", type=int, help="Number of vCPUS")
>
> It is not really clear what this argument is for. I guess you are
> saying how many cpus the guest from which trace file was generated
> had? What happens if we pass in less number of vcpus than used for
> generation?

Yeah it's a bit of a hack. I need to know how many vcpus there are so
when we save an "all vcpu" event we can set the event in:

    def __save_queue(self, vcpu, record):
        self.flush_total += 1
        # FIXME: don't seem to see -1
        if vcpu > 0x7fffffff:
            self.flush_all += 1
            for i in range(0, self.nvcpus):
                self.vcpu_last[i].append(record)
        else:
            self.vcpu_last[vcpu].append(record)


>
>> +    parser.add_argument("--graph", choices=['time', 'latency'], default='time')
>
> What does latency here indicate? I tried this argument on a sample
> trace file I generated, and it had three empty boxes.

Time just tracks the culmlative total of flush events over time where as
latency gives a distribution of time from queuing the flush event to it
being processed.

>
>> +    parser.add_argument("events", type=str, help='trace file read from')
>> +    parser.add_argument("tracefile", type=str, help='trace file read from')
>
> The help text for 'events' file here should be something like 'the
> trace events file'.

Good catch, will fix.

>
> Thanks,


--
Alex Bennée

  reply	other threads:[~2017-05-31  7:53 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-17 14:52 [Qemu-devel] [PATCH v2 0/6] gdb updates and cputlb traces Alex Bennée
2017-05-17 14:52 ` [Qemu-devel] [PATCH v2 1/6] scripts/replay-dump.py: replay log dumper Alex Bennée
2017-05-31  5:03   ` Philippe Mathieu-Daudé
2017-05-17 14:52 ` [Qemu-devel] [PATCH v2 2/6] scripts/qemu-gdb/timers.py: new helper to dump timer state Alex Bennée
2017-05-17 14:52 ` [Qemu-devel] [PATCH v2 3/6] scripts/qemu-gdb/tcg: new helper to dump tcg state Alex Bennée
2017-05-31  1:41   ` Philippe Mathieu-Daudé
2017-05-17 14:52 ` [Qemu-devel] [PATCH v2 4/6] cputlb: remove tlb_flush_count Alex Bennée
2017-05-17 14:52 ` [Qemu-devel] [PATCH v2 5/6] cputlb: add trace events Alex Bennée
2017-05-31  5:20   ` Philippe Mathieu-Daudé
2017-05-17 14:52 ` [Qemu-devel] [PATCH v2 6/6] new script/analyse-tlb-flushes-simpletrace.py Alex Bennée
2017-05-30 23:33   ` Pranith Kumar
2017-05-31  7:53     ` Alex Bennée [this message]
2017-05-18  9:42 ` [Qemu-devel] [PATCH v2 0/6] gdb updates and cputlb traces Stefan Hajnoczi

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=8737bltqbd.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=bobby.prani+qemu@gmail.com \
    --cc=cota@braap.org \
    --cc=pbonzini@redhat.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).