Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Stefano De Venuto <stefano.devenuto99@gmail.com>
To: Yordan Karadzhov <y.karadz@gmail.com>, rostedt@goodmis.org
Cc: linux-trace-devel@vger.kernel.org, dfaggioli@suse.com
Subject: Re: [RFC] Simple tool for VMEnters/VMExits matching and trace validation
Date: Sat, 1 May 2021 08:11:14 +0200
Message-ID: <6630d3a1-629f-bde4-b34e-ce8fb307a6f7@gmail.com> (raw)
In-Reply-To: <7edad92d-3297-fd42-9ac2-0334816fb524@gmail.com>

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@gmail.com>
>> ---
>>   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


      parent reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-16 16:46 Stefano De Venuto
2021-04-16 17:47 ` Dario Faggioli
2021-04-16 20:48   ` Stefano De Venuto
2021-04-16 21:32     ` Steven Rostedt
2021-04-17  6:43       ` Dario Faggioli
2021-04-20  8:12 ` Yordan Karadzhov
2021-04-21  2:17   ` Steven Rostedt
2021-04-21  9:14     ` Yordan Karadzhov
2021-05-01  6:11     ` Stefano De Venuto
2021-05-01  6:11   ` Stefano De Venuto [this message]

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=6630d3a1-629f-bde4-b34e-ce8fb307a6f7@gmail.com \
    --to=stefano.devenuto99@gmail.com \
    --cc=dfaggioli@suse.com \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=y.karadz@gmail.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

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org
	public-inbox-index linux-trace-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git