Re: [Qemu-devel] [PATCH v2 6/6] new script/analyse-tlb-flushes-simpletrace.py

[Alex Bennée]

Pranith Kumar <address@hidden> writes:

> Hi Alex,
>
> Please find some comments and questions below:
>
> On Wed, May 17, 2017 at 10:52 AM, Alex Bennée <address@hidden> 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 <address@hidden>
>>
>> ---
>> 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