On 02/21/2012 07:47 AM, David Ahern wrote:
- show the result:
>> perf kvm-events report
>
> It would be nice to have example reports in this commit message.
>
Okay.
>> +DESCRIPTION
>> +-----------
>> +You can analyze some crucial kvm events and statistics with this
>> +'perf kvm-events' command. Currently, vmexit, mmio and ioport events
>> +are supported.
> First sentence should be written in the 3rd person. eg.,
> This command generates a statistical analysis of KVM events.
>
Okay.
>> +--events=<value>::
>> + events to be analyzed. Possible values: vmexit, mmio, ioport.
>
> Add a comment stating which event type is the default.
>
OK, will fix.
>>
>> +-k::
>> +--key=<value>::
>> + Sorting key. Possible values: sample(default, sort by samples
>> number),
>> +time(sort by average time).
> Space before both of the '('.
>
Yes, will fix.
>> +static const char *get_exit_reason(u64 exit_code, int isa)
>> +{
>> + int table_size = ARRAY_SIZE(svm_exit_reasons);
>> + struct exit_reasons_table *table = svm_exit_reasons;
>> +
>> + if (isa == 1) {
>> + table = vmx_exit_reasons;
>> + table_size = ARRAY_SIZE(vmx_exit_reasons);
>> + }
> Why not use globals that are set once in read_events() after looking up
> cpu_isa? Then you don't have to state a preference on default init here --
> AMD or Intel. And the isa argument will not be needed here.
>
I agree.
>> + #define DEFAULT_VCPU_NUM 32
> Why 32 for the default number of vcpus in a guest? Seems like a lot for the
> typical VM. Versus something like 4 or 8.
>>
Hmm, since 32 is the default vcpu number in the old kernel (IIRC), but your
suggestion
sounds good.
>> + /* Both begin and end events did not get the key. */
>> + if (!event&& key->key == INVALID_KEY)
>> + return;
>> +
> Should not be able to get here with event unset, so the next 2 lines should
> not be needed. ie., you only want to process events where the begin event was
> seen in which case event is defined.
In some case, the 'begin event' just records the start timestamp, the actually
event
is recognised in the 'end event'.
Take mmio-read for example, in the old kernel, we use kvm-exit as the 'begin
event'
and kvm_mmio(KVM_TRACE_MMIO_READ...) is the 'end event'.
>> + "perf kvm events report [<options>]",
> missing '-' between kvm and events
>
...
>> +static const char * const kvm_events_usage[] = {
>> + "perf kvm events [<options>] {record|report}",
> missing '-' between kvm and events
Sorry for my careless, these will be fixed in the next version.
Thanks very much for your review, David! :)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
