Pranith Kumar <bobby.prani+q...@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.ben...@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.ben...@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
