Ciao Yordan,
Thanks so much for the comments!
I've just sent the version 2 addressing them.
On 4/20/21 10:12 AM, Yordan Karadzhov wrote:
> Ciao Stefano,
>
> First of all, I am very happy to see you progressing so fast in the
> development of your VMEnters/VMExits matching analysis. I have several
> comments concerning the code (see below).
>
> On 16.04.21 г. 19:46, Stefano De Venuto wrote:
>> Add a tool in examples/ that scans a merged host + guest trace and
>> search for host events that are inside a vmentry/vmexit block (and
>> vice-versa for guest events ouside the block) and report the found
>> ones.
>>
>> It can be useful as a starting point for identifying issues of for
>> checking the effectiveness of host/guest traces synchronization, or
>> even for improving the placing of the tracepoints in the kernel.
>>
>> Signed-off-by: Stefano De Venuto <stefano.devenuto99@xxxxxxxxx>
>> ---
>> examples/checker.c | 202 +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 202 insertions(+)
>> create mode 100644 examples/checker.c
>>
>> diff --git a/examples/checker.c b/examples/checker.c
>> new file mode 100644
>> index 0000000..0b04343
>> --- /dev/null
>> +++ b/examples/checker.c
>> @@ -0,0 +1,202 @@
>
> The first problem is that this patch fails to apply. My guess is that
> you hand-edited the patch and removed some lines starting with "+". Note
> that the total number of additions and removals in the file is indicated
> in the line above. If this number does not match the number of lines
> starting with "+", the patch will fail to apply.
>
> As a general advise - try to avoid hand-editing patches unless you are
> certain you know what you are doing.
>
Ok, thanks for the advice!
>> +
>> + custom_streams[i-1] = custom_stream;
>> + }
>> +
>> + host_guest_mapping = NULL;
>> + n_guest =
>> kshark_tracecmd_get_hostguest_mapping(&host_guest_mapping);
>> + if (n_guest < 0) {
>> + printf("Failed mapping: %d\n", n_guest);
>> + return 1;
>> + }
>> +
>> + entries = NULL;
>> + n_entries = kshark_load_all_entries(kshark_ctx, &entries);
>> +
>> + for (int i = 0; i < n_entries; ++i) {
>> + current = entries[i];
>> +
>> + stream = kshark_get_stream_from_entry(current);
>> + event_name = kshark_get_event_name(current);
>> +
>> + custom_stream = custom_streams[stream->stream_id];
>> +
>> + if (!strcmp(event_name, KVM_ENTRY) || !strcmp(event_name,
>> KVM_EXIT)) {
>> + if (!strcmp(event_name, KVM_ENTRY)) {
>> +
>> + /*
>> + * The recovering process of the vCPU field of the
>> kvm_entry event
>> + * is done by splitting the info field.
>> + */
>> + info = kshark_get_info(current);
>> +
>> + token = strtok(info, " ");
>> + token = strtok(NULL, " ");
>> +
>> + /* Removing the last comma */
>> + token[strlen(token) - 1] = '\0';
>> +
>> + custom_stream->cpus[current->cpu] = atoi(token);
>
> It will be better if you make the recovering of the vCPU above a static
> helper function.
>
I totally replaced that method since with
kshark_read_event_field_int(entry, "vcpu_id", &vcpu) I can get this
information in such a cleaner way!
Also more robust, since plugins can remove the vcpu field from the info.
Regards,
Stefano
|