[PATCH] kernel-shark-2.alpha: Use new tracecmd API to open guest tracing file

[PATCH] kernel-shark-2.alpha: Use new tracecmd API to open guest tracing file

[Tzvetomir Stoyanov (VMware)]

From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>

The new tracecmd API tracecmd_open_merge() is used to open guest
tracing file and synchronize it to the host file.
This change is for testing purposes only, to test the new per CPU
timestamps synchronization logic.
It assumes that the stream with id 0 is the host one, and all others are
guest streams.
This change depends on commit:
 "trace-cmd: Add new API to merge two trace files"
where the new tracecmd API is introduced.

Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
---
 src/libkshark-tepdata.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
index 8678e12..b80b945 100644
--- a/src/libkshark-tepdata.c
+++ b/src/libkshark-tepdata.c
@@ -1008,8 +1008,17 @@ int kshark_tep_init_input(struct kshark_data_stream *stream,
 	tep_handle = calloc(1, sizeof(*tep_handle));
 	if (!tep_handle)
 		return -EFAULT;
-
-	tep_handle->input = tracecmd_open(file);
+	if (stream->stream_id > 0) {
+		struct kshark_data_stream *primary;
+
+		primary = kshark_get_data_stream(kshark_ctx, 0);
+		if (primary && primary->file)
+			tep_handle->input = tracecmd_open_merge(file,
+								primary->file);
+		else
+			tep_handle->input = tracecmd_open(file);
+	} else
+		tep_handle->input = tracecmd_open(file);
 	if (!tep_handle->input) {
 		free(tep_handle);
 		stream->interface.handle = NULL;
-- 
2.25.1

Re: [PATCH] kernel-shark-2.alpha: Use new tracecmd API to open guest tracing file

[Yordan Karadzhov (VMware)]

On 1.04.20 г. 19:45 ч., Tzvetomir Stoyanov (VMware) wrote:
> From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> 
> The new tracecmd API tracecmd_open_merge() is used to open guest
> tracing file and synchronize it to the host file.
> This change is for testing purposes only, to test the new per CPU
> timestamps synchronization logic.
> It assumes that the stream with id 0 is the host one, and all others are
> guest streams.
> This change depends on commit:
>   "trace-cmd: Add new API to merge two trace files"
> where the new tracecmd API is introduced.
> 
> Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> ---
>   src/libkshark-tepdata.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
> index 8678e12..b80b945 100644
> --- a/src/libkshark-tepdata.c
> +++ b/src/libkshark-tepdata.c
> @@ -1008,8 +1008,17 @@ int kshark_tep_init_input(struct kshark_data_stream *stream,
>   	tep_handle = calloc(1, sizeof(*tep_handle));
>   	if (!tep_handle)
>   		return -EFAULT;
> -
> -	tep_handle->input = tracecmd_open(file);

Hi Ceco,

Is it possible to avoid the hard-coded ID number of the primary stream 
(zero in this case)?

Maybe we can add data field to "struct tepdata_handle" that can be used 
to indicate if the stream is "primary" or not. Note that "struct 
tepdata_handle" is defined only for "libkshark-tepdata.c".

Also do we consider a case when we will need to load more than one 
"primary" stream?

Thanks!
Yordan


> +	if (stream->stream_id > 0) {
> +		struct kshark_data_stream *primary;
> +
> +		primary = kshark_get_data_stream(kshark_ctx, 0);
> +		if (primary && primary->file)
> +			tep_handle->input = tracecmd_open_merge(file,
> +								primary->file);
> +		else
> +			tep_handle->input = tracecmd_open(file);
> +	} else
> +		tep_handle->input = tracecmd_open(file);
>   	if (!tep_handle->input) {
>   		free(tep_handle);
>   		stream->interface.handle = NULL;
> 

Re: [PATCH] kernel-shark-2.alpha: Use new tracecmd API to open guest tracing file

[Tzvetomir Stoyanov]

On Fri, Apr 3, 2020 at 2:39 PM Yordan Karadzhov (VMware)
<y.karadz@gmail.com> wrote:
>
>
>
> On 1.04.20 г. 19:45 ч., Tzvetomir Stoyanov (VMware) wrote:
> > From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> >
> > The new tracecmd API tracecmd_open_merge() is used to open guest
> > tracing file and synchronize it to the host file.
> > This change is for testing purposes only, to test the new per CPU
> > timestamps synchronization logic.
> > It assumes that the stream with id 0 is the host one, and all others are
> > guest streams.
> > This change depends on commit:
> >   "trace-cmd: Add new API to merge two trace files"
> > where the new tracecmd API is introduced.
> >
> > Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> > ---
> >   src/libkshark-tepdata.c | 13 +++++++++++--
> >   1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
> > index 8678e12..b80b945 100644
> > --- a/src/libkshark-tepdata.c
> > +++ b/src/libkshark-tepdata.c
> > @@ -1008,8 +1008,17 @@ int kshark_tep_init_input(struct kshark_data_stream *stream,
> >       tep_handle = calloc(1, sizeof(*tep_handle));
> >       if (!tep_handle)
> >               return -EFAULT;
> > -
> > -     tep_handle->input = tracecmd_open(file);
>
> Hi Ceco,
>
> Is it possible to avoid the hard-coded ID number of the primary stream
> (zero in this case)?

Yes, the first version of the patch is more a POC, to test the new
tracecmd_open_merge() API.

>
> Maybe we can add data field to "struct tepdata_handle" that can be used
> to indicate if the stream is "primary" or not. Note that "struct
> tepdata_handle" is defined only for "libkshark-tepdata.c".

I think a new "unsigned int flags" member should be added to
struct kshark_data_stream;
and flags should indicate if the stream is primary or not, depending
on the way the file is loaded:
 using "-i" option, or "File -> Open" from the menu - primary
 using "-a" option, or "File -> Append" from the menu - secondary

>
> Also do we consider a case when we will need to load more than one
> "primary" stream?

In the host - guest tracing, there is no use case with more than one
primary stream,
but may be there could be other use cases where more than one primary
stream will
be needed ?

Is it possible to append more than one secondary files, using the
current KernelShark 2 ?
I tried appending two files, specifying "-a" option twice, but only
the first "-a" file was appended.

>
> Thanks!
> Yordan
>
>
> > +     if (stream->stream_id > 0) {
> > +             struct kshark_data_stream *primary;
> > +
> > +             primary = kshark_get_data_stream(kshark_ctx, 0);
> > +             if (primary && primary->file)
> > +                     tep_handle->input = tracecmd_open_merge(file,
> > +                                                             primary->file);
> > +             else
> > +                     tep_handle->input = tracecmd_open(file);
> > +     } else
> > +             tep_handle->input = tracecmd_open(file);
> >       if (!tep_handle->input) {
> >               free(tep_handle);
> >               stream->interface.handle = NULL;
> >



-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

Re: [PATCH] kernel-shark-2.alpha: Use new tracecmd API to open guest tracing file

[Steven Rostedt]

On Fri, 3 Apr 2020 15:26:49 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> >
> > Also do we consider a case when we will need to load more than one
> > "primary" stream?  
> 
> In the host - guest tracing, there is no use case with more than one
> primary stream,
> but may be there could be other use cases where more than one primary
> stream will
> be needed ?
> 
> Is it possible to append more than one secondary files, using the
> current KernelShark 2 ?
> I tried appending two files, specifying "-a" option twice, but only
> the first "-a" file was appended.

We should start thinking about nested VMs. What happens if we have a host,
a guest-host and a guest? That is, the guest is a VM on top of the
guest-host, which is a VM on top of the host.

And there could be a guest on top of that guest as well!

-- Steve

Re: [PATCH] kernel-shark-2.alpha: Use new tracecmd API to open guest tracing file

[Yordan Karadzhov (VMware)]

On 3.04.20 г. 15:26 ч., Tzvetomir Stoyanov wrote:
> On Fri, Apr 3, 2020 at 2:39 PM Yordan Karadzhov (VMware)
> <y.karadz@gmail.com> wrote:
>>
>>
>>
>> On 1.04.20 г. 19:45 ч., Tzvetomir Stoyanov (VMware) wrote:
>>> From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
>>>
>>> The new tracecmd API tracecmd_open_merge() is used to open guest
>>> tracing file and synchronize it to the host file.
>>> This change is for testing purposes only, to test the new per CPU
>>> timestamps synchronization logic.
>>> It assumes that the stream with id 0 is the host one, and all others are
>>> guest streams.
>>> This change depends on commit:
>>>    "trace-cmd: Add new API to merge two trace files"
>>> where the new tracecmd API is introduced.
>>>
>>> Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
>>> ---
>>>    src/libkshark-tepdata.c | 13 +++++++++++--
>>>    1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
>>> index 8678e12..b80b945 100644
>>> --- a/src/libkshark-tepdata.c
>>> +++ b/src/libkshark-tepdata.c
>>> @@ -1008,8 +1008,17 @@ int kshark_tep_init_input(struct kshark_data_stream *stream,
>>>        tep_handle = calloc(1, sizeof(*tep_handle));
>>>        if (!tep_handle)
>>>                return -EFAULT;
>>> -
>>> -     tep_handle->input = tracecmd_open(file);
>>
>> Hi Ceco,
>>
>> Is it possible to avoid the hard-coded ID number of the primary stream
>> (zero in this case)?
> 
> Yes, the first version of the patch is more a POC, to test the new
> tracecmd_open_merge() API.
> 
>>
>> Maybe we can add data field to "struct tepdata_handle" that can be used
>> to indicate if the stream is "primary" or not. Note that "struct
>> tepdata_handle" is defined only for "libkshark-tepdata.c".
> 
> I think a new "unsigned int flags" member should be added to
> struct kshark_data_stream;
> and flags should indicate if the stream is primary or not, depending
> on the way the file is loaded:
>   using "-i" option, or "File -> Open" from the menu - primary
>   using "-a" option, or "File -> Append" from the menu - secondary
>

Maybe it has to be a bit more sophisticated. Does the "primary" stream 
know how to recognize its "secondary" streams?


>>
>> Also do we consider a case when we will need to load more than one
>> "primary" stream?
> 
> In the host - guest tracing, there is no use case with more than one
> primary stream,
> but may be there could be other use cases where more than one primary
> stream will
> be needed ?
> 
> Is it possible to append more than one secondary files, using the
> current KernelShark 2 ?
> I tried appending two files, specifying "-a" option twice, but only
> the first "-a" file was appended.
Yes, this can be done from the menus, but you are right that something 
goes wrong when trying to do this from the command line. I will 
investigate and will try to get it fixed.

Thanks!
Yordan

> 
>>
>> Thanks!
>> Yordan
>>
>>
>>> +     if (stream->stream_id > 0) {
>>> +             struct kshark_data_stream *primary;
>>> +
>>> +             primary = kshark_get_data_stream(kshark_ctx, 0);
>>> +             if (primary && primary->file)
>>> +                     tep_handle->input = tracecmd_open_merge(file,
>>> +                                                             primary->file);
>>> +             else
>>> +                     tep_handle->input = tracecmd_open(file);
>>> +     } else
>>> +             tep_handle->input = tracecmd_open(file);
>>>        if (!tep_handle->input) {
>>>                free(tep_handle);
>>>                stream->interface.handle = NULL;
>>>
> 
> 
> 

Re: [PATCH] kernel-shark-2.alpha: Use new tracecmd API to open guest tracing file

[Tzvetomir Stoyanov]

On Fri, Apr 3, 2020 at 6:09 PM Yordan Karadzhov (VMware)
<y.karadz@gmail.com> wrote:
>
>
>
> On 3.04.20 г. 15:26 ч., Tzvetomir Stoyanov wrote:
> > On Fri, Apr 3, 2020 at 2:39 PM Yordan Karadzhov (VMware)
> > <y.karadz@gmail.com> wrote:
> >>
> >>
> >>
> >> On 1.04.20 г. 19:45 ч., Tzvetomir Stoyanov (VMware) wrote:
> >>> From: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> >>>
> >>> The new tracecmd API tracecmd_open_merge() is used to open guest
> >>> tracing file and synchronize it to the host file.
> >>> This change is for testing purposes only, to test the new per CPU
> >>> timestamps synchronization logic.
> >>> It assumes that the stream with id 0 is the host one, and all others are
> >>> guest streams.
> >>> This change depends on commit:
> >>>    "trace-cmd: Add new API to merge two trace files"
> >>> where the new tracecmd API is introduced.
> >>>
> >>> Signed-off-by: Tzvetomir (VMware)  Stoyanov <tz.stoyanov@gmail.com>
> >>> ---
> >>>    src/libkshark-tepdata.c | 13 +++++++++++--
> >>>    1 file changed, 11 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/src/libkshark-tepdata.c b/src/libkshark-tepdata.c
> >>> index 8678e12..b80b945 100644
> >>> --- a/src/libkshark-tepdata.c
> >>> +++ b/src/libkshark-tepdata.c
> >>> @@ -1008,8 +1008,17 @@ int kshark_tep_init_input(struct kshark_data_stream *stream,
> >>>        tep_handle = calloc(1, sizeof(*tep_handle));
> >>>        if (!tep_handle)
> >>>                return -EFAULT;
> >>> -
> >>> -     tep_handle->input = tracecmd_open(file);
> >>
> >> Hi Ceco,
> >>
> >> Is it possible to avoid the hard-coded ID number of the primary stream
> >> (zero in this case)?
> >
> > Yes, the first version of the patch is more a POC, to test the new
> > tracecmd_open_merge() API.
> >
> >>
> >> Maybe we can add data field to "struct tepdata_handle" that can be used
> >> to indicate if the stream is "primary" or not. Note that "struct
> >> tepdata_handle" is defined only for "libkshark-tepdata.c".
> >
> > I think a new "unsigned int flags" member should be added to
> > struct kshark_data_stream;
> > and flags should indicate if the stream is primary or not, depending
> > on the way the file is loaded:
> >   using "-i" option, or "File -> Open" from the menu - primary
> >   using "-a" option, or "File -> Append" from the menu - secondary
> >
>
> Maybe it has to be a bit more sophisticated. Does the "primary" stream
> know how to recognize its "secondary" streams?
>
Yes, there are tracing IDs in both host and guest files. When these IDs match,
files are from a same tracing session.
I can add "parent_stream_id" member in "struct kshark_data_stream" and
initialize it
when reading the files, according to the trace IDs. This can be useful
in KVMCombo
plugin also, for proper PID to VCPU mapping.

>
> >>
> >> Also do we consider a case when we will need to load more than one
> >> "primary" stream?
> >
> > In the host - guest tracing, there is no use case with more than one
> > primary stream,
> > but may be there could be other use cases where more than one primary
> > stream will
> > be needed ?
> >
> > Is it possible to append more than one secondary files, using the
> > current KernelShark 2 ?
> > I tried appending two files, specifying "-a" option twice, but only
> > the first "-a" file was appended.
> Yes, this can be done from the menus, but you are right that something
> goes wrong when trying to do this from the command line. I will
> investigate and will try to get it fixed.
>
> Thanks!
> Yordan
>
> >
> >>
> >> Thanks!
> >> Yordan
> >>
> >>
> >>> +     if (stream->stream_id > 0) {
> >>> +             struct kshark_data_stream *primary;
> >>> +
> >>> +             primary = kshark_get_data_stream(kshark_ctx, 0);
> >>> +             if (primary && primary->file)
> >>> +                     tep_handle->input = tracecmd_open_merge(file,
> >>> +                                                             primary->file);
> >>> +             else
> >>> +                     tep_handle->input = tracecmd_open(file);
> >>> +     } else
> >>> +             tep_handle->input = tracecmd_open(file);
> >>>        if (!tep_handle->input) {
> >>>                free(tep_handle);
> >>>                stream->interface.handle = NULL;
> >>>
> >
> >
> >



-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

Re: [PATCH] kernel-shark-2.alpha: Use new tracecmd API to open guest tracing file

[Yordan Karadzhov (VMware)]

On 3.04.20 г. 20:12 ч., Tzvetomir Stoyanov wrote:
>> Maybe it has to be a bit more sophisticated. Does the "primary" stream
>> know how to recognize its "secondary" streams?
>>
> Yes, there are tracing IDs in both host and guest files. When these IDs match,
> files are from a same tracing session.
> I can add "parent_stream_id" member in "struct kshark_data_stream" and
> initialize it
> when reading the files, according to the trace IDs.

Not sure if this is the right place to put this information. I do not 
want to have "tep data" specific fields in "struct kshark_data_stream". 
Is it possible to have this mapping stored in in "struct tepdata_handle" 
instead.

Thanks!
Yordan

Re: [PATCH] kernel-shark-2.alpha: Use new tracecmd API to open guest tracing file

[Tzvetomir Stoyanov]

On Mon, Apr 6, 2020 at 4:24 PM Yordan Karadzhov (VMware)
<y.karadz@gmail.com> wrote:
>
>
>
> On 3.04.20 г. 20:12 ч., Tzvetomir Stoyanov wrote:
> >> Maybe it has to be a bit more sophisticated. Does the "primary" stream
> >> know how to recognize its "secondary" streams?
> >>
> > Yes, there are tracing IDs in both host and guest files. When these IDs match,
> > files are from a same tracing session.
> > I can add "parent_stream_id" member in "struct kshark_data_stream" and
> > initialize it
> > when reading the files, according to the trace IDs.
>
> Not sure if this is the right place to put this information. I do not
> want to have "tep data" specific fields in "struct kshark_data_stream".
> Is it possible to have this mapping stored in in "struct tepdata_handle"
> instead.
Actually, the only new thing in "struct kshark_data_stream" will be the id of
the parent stream, "int parent_stream_id". All tracecmd specific mappings are in
"struct tracecmd_input" and will be accessible via tracecmd APIs.
Currently I assume that the stream with ID 0 is parent to all other
streams, opened with "-a" option.
This assumption will be removed when this "parent_stream_id" is introduced.

>
> Thanks!
> Yordan



-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center