[v2,1/5] kernel-shark-qt: Add kshark_get_X_easy() functions.

Commit Message

Yordan Karadzhov Oct. 1, 2018, 1:59 p.m. UTC

From: "Yordan Karadzhov (VMware)" <[e-mail]>

The kshark_get_X_easy() functions allow for an easy access to the
tep_record's fields. Using these functions can be inefficient if you
need access to more than one of the fields of the record.

Signed-off-by: Yordan Karadzhov (VMware) <[e-mail]>
---
 kernel-shark-qt/src/libkshark.c | 185 ++++++++++++++++++++++++++++++++
 kernel-shark-qt/src/libkshark.h |  12 +++
 2 files changed, 197 insertions(+)

Comments

Steven Rostedt Oct. 2, 2018, 8:44 p.m. UTC | #1

On Mon,  1 Oct 2018 16:59:17 +0300
Yordan Karadzhov <[e-mail]> wrote:

> From: "Yordan Karadzhov (VMware)" <[e-mail]>
> 
> The kshark_get_X_easy() functions allow for an easy access to the
> tep_record's fields. Using these functions can be inefficient if you
> need access to more than one of the fields of the record.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <[e-mail]>
> ---
>  kernel-shark-qt/src/libkshark.c | 185 ++++++++++++++++++++++++++++++++
>  kernel-shark-qt/src/libkshark.h |  12 +++
>  2 files changed, 197 insertions(+)
> 
> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
> index 506511d..a7983eb 100644
> --- a/kernel-shark-qt/src/libkshark.c
> +++ b/kernel-shark-qt/src/libkshark.c
> @@ -888,6 +888,191 @@ static const char *kshark_get_info(struct tep_handle *pe,
>  	return seq.buffer;
>  }
>  
> +/**
> + * @brief This function allows for an easy access to the original value of the
> + *	  Process Id as recorded in the tep_record object. The record is read
> + *	  from the file only in the case of an entry being touched by a plugin.
> + *	  Be aware that using the kshark_get_X_easy functions can be
> + *	  inefficient if you need an access to more than one of the data fields
> + *	  of the record.
> + *
> + * @param entry: Input location for the KernelShark entry.
> + *
> + * @returns The original value of the Process Id as recorded in the
> + *	    tep_record object on success, otherwise negative error code.
> + */
> +int kshark_get_pid_easy(struct kshark_entry *entry)
> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +	struct tep_record *data;
> +	int pid;
> +
> +	if (!kshark_instance(&kshark_ctx))
> +		return -EFAULT;

Perhaps this should return -ENODEV;

> +
> +	if (entry->visible & KS_PLUGIN_UNTOUCHED_MASK) {

What's the "UNTOUCHED_MASK" mean?

> +		pid = entry->pid;
> +	} else {
> +		data = kshark_read_at(kshark_ctx, entry->offset);
> +		pid = tep_data_pid(kshark_ctx->pevent, data);
> +		free_record(data);

Would it be possible to do:

		entry->visible |= KS_PLUGIN_UNTOUCHED_MASK;
		entry->pid = pid;

?

Of course this depends on what that mask means.

> +	}
> +
> +	return pid;
> +}
> +
> +/**
> + * @brief This function allows for an easy access to the original value of the
> + *	  Task name as recorded in the tep_record object. The record is read
> + *	  from the file only in the case of an entry being touched by a plugin.
> + *	  Be aware that using the kshark_get_X_easy functions can be
> + *	  inefficient if you need an access to more than one of the data fields
> + *	  of the record.
> + *
> + * @param entry: Input location for the KernelShark entry.
> + *
> + * @returns The original name of the task, retrieved from the Process Id
> + *	    recorded in the tep_record object on success, otherwise NULL.
> + */
> +const char *kshark_get_task_easy(struct kshark_entry *entry)
> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +	int pid = kshark_get_pid_easy(entry);
> +
> +	if (pid < 0 || !kshark_instance(&kshark_ctx))

The second part is redundant, as kshark_get_pid_easy() will return 
pid < 0 if that was true.

> +		return NULL;
> +
> +	return tep_data_comm_from_pid(kshark_ctx->pevent, pid);
> +}
> +
> +/**
> + * @brief This function allows for an easy access to the latency information
> + *	  recorded in the tep_record object. The record is read from the file
> + *	  using the offset field of kshark_entry. Be aware that using the
> + *	  kshark_get_X_easy functions can be inefficient if you need an access
> + *	  to more than one of the data fields of the record.
> + *
> + * @param entry: Input location for the KernelShark entry.
> + *
> + * @returns On success the function returns a string showing the latency
> + *	    information, coded into 5 fields:
> + *	    interrupts disabled, need rescheduling, hard/soft interrupt,
> + *	    preempt count and lock depth. On failure it returns NULL.
> + */
> +const char *kshark_get_latency_easy(struct kshark_entry *entry)
> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +	struct tep_record *data;
> +	const char *lat;
> +
> +	if (!kshark_instance(&kshark_ctx))
> +		return NULL;
> +
> +	data = kshark_read_at(kshark_ctx, entry->offset);
> +	lat = kshark_get_latency(kshark_ctx->pevent, data);
> +	free_record(data);
> +
> +	return lat;
> +}
> +
> +/**
> + * @brief This function allows for an easy access to the original value of the
> + *	  Event Id as recorded in the tep_record object. The record is read
> + *	  from the file only in the case of an entry being touched by a plugin.
> + *	  Be aware that using the kshark_get_X_easy functions can be
> + *	  inefficient if you need an access to more than one of the data fields
> + *	  of the record.
> + *
> + * @param entry: Input location for the KernelShark entry.
> + *
> + * @returns The original value of the Event Id as recorded in the
> + *	    tep_record object on success, otherwise negative error code.
> + */
> +int kshark_get_event_id_easy(struct kshark_entry *entry)
> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +	struct tep_record *data;
> +	int event_id;
> +
> +	if (!kshark_instance(&kshark_ctx))
> +		return -EFAULT;
> +
> +	if (entry->visible & KS_PLUGIN_UNTOUCHED_MASK) {
> +		event_id = entry->event_id;
> +	} else {
> +		data = kshark_read_at(kshark_ctx, entry->offset);
> +		event_id = tep_data_type(kshark_ctx->pevent, data);
> +		free_record(data);

If the above can update "easy" untouch flag, then this too.

> +	}
> +
> +	return event_id;
> +}
> +
> +/**
> + * @brief This function allows for an easy access to the original name of the
> + * 	  trace event as recorded in the tep_record object. The record is read
> + *	  from the file only in the case of an entry being touched by a plugin.
> + *	  Be aware that using the kshark_get_X_easy functions can be
> + *	  inefficient if you need an access to more than one of the data fields
> + *	  of the record.
> + *
> + * @param entry: Input location for the KernelShark entry.
> + *
> + * @returns The mane of the trace event recorded in the tep_record object on
> + * 	    success, otherwise "[UNKNOWN EVENT]" or NULL.
> + */
> +const char *kshark_get_event_name_easy(struct kshark_entry *entry)
> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +	struct event_format *event;
> +
> +	int event_id = kshark_get_event_id_easy(entry);
> +
> +	if (event_id < 0 || !kshark_instance(&kshark_ctx))

Again this is redundant.

-- Steve

> +		return NULL;
> +
> +	event = tep_data_event_from_type(kshark_ctx->pevent, event_id);
> +	if (event)
> +		return event->name;
> +
> +	return "[UNKNOWN EVENT]";
> +}
> +
> +/**
> + * @brief This function allows for an easy access to the tep_record's info
> + *	  streang. The record is read from the file using the offset field of
> + *	  kshark_entry. Be aware that using the kshark_get_X_easy functions can
> + *	  be inefficient if you need an access to more than one of the data
> + *	  fields of the record.
> + *
> + * @param entry: Input location for the KernelShark entry.
> + *
> + * @returns A string showing the data output of the trace event on success,
> + *	    otherwise NULL.
> + */
> +const char *kshark_get_info_easy(struct kshark_entry *entry)
> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +	struct event_format *event;
> +	struct tep_record *data;
> +	const char *info = NULL;
> +	int event_id;
> +
> +	if (!kshark_instance(&kshark_ctx))
> +		return NULL;
> +
> +	data = kshark_read_at(kshark_ctx, entry->offset);
> +
> +	event_id = tep_data_type(kshark_ctx->pevent, data);
> +	event = tep_data_event_from_type(kshark_ctx->pevent, event_id);
> +	if (event)
> +		info = kshark_get_info(kshark_ctx->pevent, data, event);
> +
> +	free_record(data);
> +
> +	return info;
> +}
> +
>  /**
>   * @brief Dump into a string the content of one entry. The function allocates
>   *	  a null terminated string and returns a pointer to this string. The
> diff --git a/kernel-shark-qt/src/libkshark.h b/kernel-shark-qt/src/libkshark.h
> index e846c85..f00a584 100644
> --- a/kernel-shark-qt/src/libkshark.h
> +++ b/kernel-shark-qt/src/libkshark.h
> @@ -148,6 +148,18 @@ void kshark_close(struct kshark_context *kshark_ctx);
>  
>  void kshark_free(struct kshark_context *kshark_ctx);
>  
> +int kshark_get_pid_easy(struct kshark_entry *entry);
> +
> +const char *kshark_get_task_easy(struct kshark_entry *entry);
> +
> +const char *kshark_get_latency_easy(struct kshark_entry *entry);
> +
> +int kshark_get_event_id_easy(struct kshark_entry *entry);
> +
> +const char *kshark_get_event_name_easy(struct kshark_entry *entry);
> +
> +const char *kshark_get_info_easy(struct kshark_entry *entry);
> +
>  char* kshark_dump_entry(const struct kshark_entry *entry);
>  
>  struct tep_record *kshark_read_at(struct kshark_context *kshark_ctx,

Yordan Karadzhov Oct. 3, 2018, 7:34 a.m. UTC | #2

On  2.10.2018 23:44, Steven Rostedt wrote:
> On Mon,  1 Oct 2018 16:59:17 +0300
> Yordan Karadzhov <[e-mail]> wrote:
> 
>> From: "Yordan Karadzhov (VMware)" <[e-mail]>
>>
>> The kshark_get_X_easy() functions allow for an easy access to the
>> tep_record's fields. Using these functions can be inefficient if you
>> need access to more than one of the fields of the record.
>>
>> Signed-off-by: Yordan Karadzhov (VMware) <[e-mail]>
>> ---
>>   kernel-shark-qt/src/libkshark.c | 185 ++++++++++++++++++++++++++++++++
>>   kernel-shark-qt/src/libkshark.h |  12 +++
>>   2 files changed, 197 insertions(+)
>>
>> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
>> index 506511d..a7983eb 100644
>> --- a/kernel-shark-qt/src/libkshark.c
>> +++ b/kernel-shark-qt/src/libkshark.c
>> @@ -888,6 +888,191 @@ static const char *kshark_get_info(struct tep_handle *pe,
>>   	return seq.buffer;
>>   }
>>   
>> +/**
>> + * @brief This function allows for an easy access to the original value of the
>> + *	  Process Id as recorded in the tep_record object. The record is read
>> + *	  from the file only in the case of an entry being touched by a plugin.
>> + *	  Be aware that using the kshark_get_X_easy functions can be
>> + *	  inefficient if you need an access to more than one of the data fields
>> + *	  of the record.
>> + *
>> + * @param entry: Input location for the KernelShark entry.
>> + *
>> + * @returns The original value of the Process Id as recorded in the
>> + *	    tep_record object on success, otherwise negative error code.
>> + */
>> +int kshark_get_pid_easy(struct kshark_entry *entry)
>> +{
>> +	struct kshark_context *kshark_ctx = NULL;
>> +	struct tep_record *data;
>> +	int pid;
>> +
>> +	if (!kshark_instance(&kshark_ctx))
>> +		return -EFAULT;
> 
> Perhaps this should return -ENODEV;
> 
>> +
>> +	if (entry->visible & KS_PLUGIN_UNTOUCHED_MASK) {
> 
> What's the "UNTOUCHED_MASK" mean?

We use the KS_PLUGIN_UNTOUCHED_MASK to check the flag (bit)
which indicates if the entry has been touched and potential modified by 
a plugin callback function. If the bit is set this means untouched.

This flag (bit) gets set when we load the data ( libkshark.c / 
get_records())

	entry->visible = 0xFF;

...

	/* Execute all plugin-provided actions (if any). */
	evt_handler = kshark_ctx->event_handlers;
	while ((evt_handler = kshark_find_event_handler(evt_handler,
						entry->event_id))) {
		evt_handler->event_func(kshark_ctx, rec, entry);
		evt_handler = evt_handler->next;
		if (!evt_handler)
			entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK;
	}

> 
>> +		pid = entry->pid;
>> +	} else {

In this case the entry has been touched by a plugin. Because of this we 
do not trust the value of "entry->pid".

>> +		data = kshark_read_at(kshark_ctx, entry->offset);
>> +		pid = tep_data_pid(kshark_ctx->pevent, data);
>> +		free_record(data);
> 
> Would it be possible to do:
> 
> 		entry->visible |= KS_PLUGIN_UNTOUCHED_MASK;
> 		entry->pid = pid;
> 
> ?
> 
> Of course this depends on what that mask means.
> 
>> +	}
>> +
>> +	return pid;
>> +}
>> +

Yordan Karadzhov Oct. 5, 2018, 2:30 p.m. UTC | #3

Hi Steven,

I guess this email thread was lost on your side.

Is my explanation of the "UNTOUCHED_MASK" OK?
Should I send you v3 of the patches with the other fixes you asked for?

On  3.10.2018 10:34, Yordan Karadzhov wrote:
> 
> 
> On  2.10.2018 23:44, Steven Rostedt wrote:
>> On Mon,  1 Oct 2018 16:59:17 +0300
>> Yordan Karadzhov <[e-mail]> wrote:
>>
>>> From: "Yordan Karadzhov (VMware)" <[e-mail]>
>>>
>>> The kshark_get_X_easy() functions allow for an easy access to the
>>> tep_record's fields. Using these functions can be inefficient if you
>>> need access to more than one of the fields of the record.
>>>
>>> Signed-off-by: Yordan Karadzhov (VMware) <[e-mail]>
>>> ---
>>>    kernel-shark-qt/src/libkshark.c | 185 ++++++++++++++++++++++++++++++++
>>>    kernel-shark-qt/src/libkshark.h |  12 +++
>>>    2 files changed, 197 insertions(+)
>>>
>>> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
>>> index 506511d..a7983eb 100644
>>> --- a/kernel-shark-qt/src/libkshark.c
>>> +++ b/kernel-shark-qt/src/libkshark.c
>>> @@ -888,6 +888,191 @@ static const char *kshark_get_info(struct tep_handle *pe,
>>>    	return seq.buffer;
>>>    }
>>>    
>>> +/**
>>> + * @brief This function allows for an easy access to the original value of the
>>> + *	  Process Id as recorded in the tep_record object. The record is read
>>> + *	  from the file only in the case of an entry being touched by a plugin.
>>> + *	  Be aware that using the kshark_get_X_easy functions can be
>>> + *	  inefficient if you need an access to more than one of the data fields
>>> + *	  of the record.
>>> + *
>>> + * @param entry: Input location for the KernelShark entry.
>>> + *
>>> + * @returns The original value of the Process Id as recorded in the
>>> + *	    tep_record object on success, otherwise negative error code.
>>> + */
>>> +int kshark_get_pid_easy(struct kshark_entry *entry)
>>> +{
>>> +	struct kshark_context *kshark_ctx = NULL;
>>> +	struct tep_record *data;
>>> +	int pid;
>>> +
>>> +	if (!kshark_instance(&kshark_ctx))
>>> +		return -EFAULT;
>>
>> Perhaps this should return -ENODEV;
>>
>>> +
>>> +	if (entry->visible & KS_PLUGIN_UNTOUCHED_MASK) {
>>
>> What's the "UNTOUCHED_MASK" mean?
> 
> We use the KS_PLUGIN_UNTOUCHED_MASK to check the flag (bit)
> which indicates if the entry has been touched and potential modified by
> a plugin callback function. If the bit is set this means untouched.
> 
> This flag (bit) gets set when we load the data ( libkshark.c /
> get_records())
> 
> 	entry->visible = 0xFF;
> 
> ...
> 
> 	/* Execute all plugin-provided actions (if any). */
> 	evt_handler = kshark_ctx->event_handlers;
> 	while ((evt_handler = kshark_find_event_handler(evt_handler,
> 						entry->event_id))) {
> 		evt_handler->event_func(kshark_ctx, rec, entry);
> 		evt_handler = evt_handler->next;
> 		if (!evt_handler)
> 			entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK;
> 	}
> 
>>
>>> +		pid = entry->pid;
>>> +	} else {
> 
> In this case the entry has been touched by a plugin. Because of this we
> do not trust the value of "entry->pid".
> 
>>> +		data = kshark_read_at(kshark_ctx, entry->offset);
>>> +		pid = tep_data_pid(kshark_ctx->pevent, data);
>>> +		free_record(data);
>>
>> Would it be possible to do:
>>
>> 		entry->visible |= KS_PLUGIN_UNTOUCHED_MASK;
>> 		entry->pid = pid;
>>
>> ?
>>
>> Of course this depends on what that mask means.
>>
>>> +	}
>>> +
>>> +	return pid;
>>> +}
>>> +

Steven Rostedt Oct. 5, 2018, 2:46 p.m. UTC | #4

On Fri, 5 Oct 2018 17:30:17 +0300
"Yordan Karadzhov (VMware)" <[e-mail]> wrote:

> Hi Steven,
> 
> I guess this email thread was lost on your side.

Not lost, just forgotten about ;-)

> 
> Is my explanation of the "UNTOUCHED_MASK" OK?
> Should I send you v3 of the patches with the other fixes you asked for?
> 
> On  3.10.2018 10:34, Yordan Karadzhov wrote:
> > 
> > 
> > On  2.10.2018 23:44, Steven Rostedt wrote:  
> >> On Mon,  1 Oct 2018 16:59:17 +0300
> >> Yordan Karadzhov <[e-mail]> wrote:
> >>  
> >>> From: "Yordan Karadzhov (VMware)" <[e-mail]>
> >>>
> >>> The kshark_get_X_easy() functions allow for an easy access to the
> >>> tep_record's fields. Using these functions can be inefficient if you
> >>> need access to more than one of the fields of the record.
> >>>
> >>> Signed-off-by: Yordan Karadzhov (VMware) <[e-mail]>
> >>> ---
> >>>    kernel-shark-qt/src/libkshark.c | 185 ++++++++++++++++++++++++++++++++
> >>>    kernel-shark-qt/src/libkshark.h |  12 +++
> >>>    2 files changed, 197 insertions(+)
> >>>
> >>> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
> >>> index 506511d..a7983eb 100644
> >>> --- a/kernel-shark-qt/src/libkshark.c
> >>> +++ b/kernel-shark-qt/src/libkshark.c
> >>> @@ -888,6 +888,191 @@ static const char *kshark_get_info(struct tep_handle *pe,
> >>>    	return seq.buffer;
> >>>    }
> >>>    
> >>> +/**
> >>> + * @brief This function allows for an easy access to the original value of the
> >>> + *	  Process Id as recorded in the tep_record object. The record is read
> >>> + *	  from the file only in the case of an entry being touched by a plugin.
> >>> + *	  Be aware that using the kshark_get_X_easy functions can be
> >>> + *	  inefficient if you need an access to more than one of the data fields
> >>> + *	  of the record.
> >>> + *
> >>> + * @param entry: Input location for the KernelShark entry.
> >>> + *
> >>> + * @returns The original value of the Process Id as recorded in the
> >>> + *	    tep_record object on success, otherwise negative error code.
> >>> + */
> >>> +int kshark_get_pid_easy(struct kshark_entry *entry)
> >>> +{
> >>> +	struct kshark_context *kshark_ctx = NULL;
> >>> +	struct tep_record *data;
> >>> +	int pid;
> >>> +
> >>> +	if (!kshark_instance(&kshark_ctx))
> >>> +		return -EFAULT;  
> >>
> >> Perhaps this should return -ENODEV;
> >>  
> >>> +
> >>> +	if (entry->visible & KS_PLUGIN_UNTOUCHED_MASK) {  
> >>
> >> What's the "UNTOUCHED_MASK" mean?  
> > 
> > We use the KS_PLUGIN_UNTOUCHED_MASK to check the flag (bit)
> > which indicates if the entry has been touched and potential modified by
> > a plugin callback function. If the bit is set this means untouched.
> > 
> > This flag (bit) gets set when we load the data ( libkshark.c /
> > get_records())
> > 
> > 	entry->visible = 0xFF;
> > 
> > ...
> > 
> > 	/* Execute all plugin-provided actions (if any). */
> > 	evt_handler = kshark_ctx->event_handlers;
> > 	while ((evt_handler = kshark_find_event_handler(evt_handler,
> > 						entry->event_id))) {
> > 		evt_handler->event_func(kshark_ctx, rec, entry);
> > 		evt_handler = evt_handler->next;
> > 		if (!evt_handler)
> > 			entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK;
> > 	}
> >   
> >>  
> >>> +		pid = entry->pid;
> >>> +	} else {  
> > 
> > In this case the entry has been touched by a plugin. Because of this we
> > do not trust the value of "entry->pid".

I guess my question is, can we reset the entry->pid back to something
we do trust, and clear the flag?

Or do we not want to modify it, because the plugin now owns the value?

-- Steve

> >   
> >>> +		data = kshark_read_at(kshark_ctx, entry->offset);
> >>> +		pid = tep_data_pid(kshark_ctx->pevent, data);
> >>> +		free_record(data);  
> >>
> >> Would it be possible to do:
> >>
> >> 		entry->visible |= KS_PLUGIN_UNTOUCHED_MASK;
> >> 		entry->pid = pid;
> >>
> >> ?
> >>
> >> Of course this depends on what that mask means.
> >>  
> >>> +	}
> >>> +
> >>> +	return pid;
> >>> +}
> >>> +

Yordan Karadzhov Oct. 5, 2018, 3:27 p.m. UTC | #5

On  5.10.2018 17:46, Steven Rostedt wrote:
>>>>> +/**
>>>>> + * @brief This function allows for an easy access to the original value of the
>>>>> + *	  Process Id as recorded in the tep_record object. The record is read
>>>>> + *	  from the file only in the case of an entry being touched by a plugin.
>>>>> + *	  Be aware that using the kshark_get_X_easy functions can be
>>>>> + *	  inefficient if you need an access to more than one of the data fields
>>>>> + *	  of the record.
>>>>> + *
>>>>> + * @param entry: Input location for the KernelShark entry.
>>>>> + *
>>>>> + * @returns The original value of the Process Id as recorded in the
>>>>> + *	    tep_record object on success, otherwise negative error code.
>>>>> + */
>>>>> +int kshark_get_pid_easy(struct kshark_entry *entry)
>>>>> +{
>>>>> +	struct kshark_context *kshark_ctx = NULL;
>>>>> +	struct tep_record *data;
>>>>> +	int pid;
>>>>> +
>>>>> +	if (!kshark_instance(&kshark_ctx))
>>>>> +		return -EFAULT;
>>>> Perhaps this should return -ENODEV;
>>>>   
>>>>> +
>>>>> +	if (entry->visible & KS_PLUGIN_UNTOUCHED_MASK) {
>>>> What's the "UNTOUCHED_MASK" mean?
>>> We use the KS_PLUGIN_UNTOUCHED_MASK to check the flag (bit)
>>> which indicates if the entry has been touched and potential modified by
>>> a plugin callback function. If the bit is set this means untouched.
>>>
>>> This flag (bit) gets set when we load the data ( libkshark.c /
>>> get_records())
>>>
>>> 	entry->visible = 0xFF;
>>>
>>> ...
>>>
>>> 	/* Execute all plugin-provided actions (if any). */
>>> 	evt_handler = kshark_ctx->event_handlers;
>>> 	while ((evt_handler = kshark_find_event_handler(evt_handler,
>>> 						entry->event_id))) {
>>> 		evt_handler->event_func(kshark_ctx, rec, entry);
>>> 		evt_handler = evt_handler->next;
>>> 		if (!evt_handler)
>>> 			entry->visible &= ~KS_PLUGIN_UNTOUCHED_MASK;
>>> 	}
>>>    
>>>>   
>>>>> +		pid = entry->pid;
>>>>> +	} else {
>>> In this case the entry has been touched by a plugin. Because of this we
>>> do not trust the value of "entry->pid".
> I guess my question is, can we reset the entry->pid back to something
> we do trust, and clear the flag?
> 
> Or do we not want to modify it, because the plugin now owns the value?
> 

If the value of "entry->pid" has been changed then the new value will be 
used when plotting graphs and we do not want to change this.
However, we still want to be able to retrieve the original value. The 
description of the function explains that it will return the original value.

Thanks!
Yordan

> -- Steve
>

Steven Rostedt Oct. 5, 2018, 3:44 p.m. UTC | #6

On Fri, 5 Oct 2018 18:27:20 +0300
"Yordan Karadzhov (VMware)" <[e-mail]> wrote:


> If the value of "entry->pid" has been changed then the new value will be 
> used when plotting graphs and we do not want to change this.
> However, we still want to be able to retrieve the original value. The 
> description of the function explains that it will return the original value.

Right. I wanted to figure out why we have two values for pid?

OK, just clarifying what was the intent.

Please send v3 with the other updates.

Thanks!

-- Steve

Patch

diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index 506511d..a7983eb 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -888,6 +888,191 @@  static const char *kshark_get_info(struct tep_handle *pe,
 	return seq.buffer;
 }
 
+/**
+ * @brief This function allows for an easy access to the original value of the
+ *	  Process Id as recorded in the tep_record object. The record is read
+ *	  from the file only in the case of an entry being touched by a plugin.
+ *	  Be aware that using the kshark_get_X_easy functions can be
+ *	  inefficient if you need an access to more than one of the data fields
+ *	  of the record.
+ *
+ * @param entry: Input location for the KernelShark entry.
+ *
+ * @returns The original value of the Process Id as recorded in the
+ *	    tep_record object on success, otherwise negative error code.
+ */
+int kshark_get_pid_easy(struct kshark_entry *entry)
+{
+	struct kshark_context *kshark_ctx = NULL;
+	struct tep_record *data;
+	int pid;
+
+	if (!kshark_instance(&kshark_ctx))
+		return -EFAULT;
+
+	if (entry->visible & KS_PLUGIN_UNTOUCHED_MASK) {
+		pid = entry->pid;
+	} else {
+		data = kshark_read_at(kshark_ctx, entry->offset);
+		pid = tep_data_pid(kshark_ctx->pevent, data);
+		free_record(data);
+	}
+
+	return pid;
+}
+
+/**
+ * @brief This function allows for an easy access to the original value of the
+ *	  Task name as recorded in the tep_record object. The record is read
+ *	  from the file only in the case of an entry being touched by a plugin.
+ *	  Be aware that using the kshark_get_X_easy functions can be
+ *	  inefficient if you need an access to more than one of the data fields
+ *	  of the record.
+ *
+ * @param entry: Input location for the KernelShark entry.
+ *
+ * @returns The original name of the task, retrieved from the Process Id
+ *	    recorded in the tep_record object on success, otherwise NULL.
+ */
+const char *kshark_get_task_easy(struct kshark_entry *entry)
+{
+	struct kshark_context *kshark_ctx = NULL;
+	int pid = kshark_get_pid_easy(entry);
+
+	if (pid < 0 || !kshark_instance(&kshark_ctx))
+		return NULL;
+
+	return tep_data_comm_from_pid(kshark_ctx->pevent, pid);
+}
+
+/**
+ * @brief This function allows for an easy access to the latency information
+ *	  recorded in the tep_record object. The record is read from the file
+ *	  using the offset field of kshark_entry. Be aware that using the
+ *	  kshark_get_X_easy functions can be inefficient if you need an access
+ *	  to more than one of the data fields of the record.
+ *
+ * @param entry: Input location for the KernelShark entry.
+ *
+ * @returns On success the function returns a string showing the latency
+ *	    information, coded into 5 fields:
+ *	    interrupts disabled, need rescheduling, hard/soft interrupt,
+ *	    preempt count and lock depth. On failure it returns NULL.
+ */
+const char *kshark_get_latency_easy(struct kshark_entry *entry)
+{
+	struct kshark_context *kshark_ctx = NULL;
+	struct tep_record *data;
+	const char *lat;
+
+	if (!kshark_instance(&kshark_ctx))
+		return NULL;
+
+	data = kshark_read_at(kshark_ctx, entry->offset);
+	lat = kshark_get_latency(kshark_ctx->pevent, data);
+	free_record(data);
+
+	return lat;
+}
+
+/**
+ * @brief This function allows for an easy access to the original value of the
+ *	  Event Id as recorded in the tep_record object. The record is read
+ *	  from the file only in the case of an entry being touched by a plugin.
+ *	  Be aware that using the kshark_get_X_easy functions can be
+ *	  inefficient if you need an access to more than one of the data fields
+ *	  of the record.
+ *
+ * @param entry: Input location for the KernelShark entry.
+ *
+ * @returns The original value of the Event Id as recorded in the
+ *	    tep_record object on success, otherwise negative error code.
+ */
+int kshark_get_event_id_easy(struct kshark_entry *entry)
+{
+	struct kshark_context *kshark_ctx = NULL;
+	struct tep_record *data;
+	int event_id;
+
+	if (!kshark_instance(&kshark_ctx))
+		return -EFAULT;
+
+	if (entry->visible & KS_PLUGIN_UNTOUCHED_MASK) {
+		event_id = entry->event_id;
+	} else {
+		data = kshark_read_at(kshark_ctx, entry->offset);
+		event_id = tep_data_type(kshark_ctx->pevent, data);
+		free_record(data);
+	}
+
+	return event_id;
+}
+
+/**
+ * @brief This function allows for an easy access to the original name of the
+ * 	  trace event as recorded in the tep_record object. The record is read
+ *	  from the file only in the case of an entry being touched by a plugin.
+ *	  Be aware that using the kshark_get_X_easy functions can be
+ *	  inefficient if you need an access to more than one of the data fields
+ *	  of the record.
+ *
+ * @param entry: Input location for the KernelShark entry.
+ *
+ * @returns The mane of the trace event recorded in the tep_record object on
+ * 	    success, otherwise "[UNKNOWN EVENT]" or NULL.
+ */
+const char *kshark_get_event_name_easy(struct kshark_entry *entry)
+{
+	struct kshark_context *kshark_ctx = NULL;
+	struct event_format *event;
+
+	int event_id = kshark_get_event_id_easy(entry);
+
+	if (event_id < 0 || !kshark_instance(&kshark_ctx))
+		return NULL;
+
+	event = tep_data_event_from_type(kshark_ctx->pevent, event_id);
+	if (event)
+		return event->name;
+
+	return "[UNKNOWN EVENT]";
+}
+
+/**
+ * @brief This function allows for an easy access to the tep_record's info
+ *	  streang. The record is read from the file using the offset field of
+ *	  kshark_entry. Be aware that using the kshark_get_X_easy functions can
+ *	  be inefficient if you need an access to more than one of the data
+ *	  fields of the record.
+ *
+ * @param entry: Input location for the KernelShark entry.
+ *
+ * @returns A string showing the data output of the trace event on success,
+ *	    otherwise NULL.
+ */
+const char *kshark_get_info_easy(struct kshark_entry *entry)
+{
+	struct kshark_context *kshark_ctx = NULL;
+	struct event_format *event;
+	struct tep_record *data;
+	const char *info = NULL;
+	int event_id;
+
+	if (!kshark_instance(&kshark_ctx))
+		return NULL;
+
+	data = kshark_read_at(kshark_ctx, entry->offset);
+
+	event_id = tep_data_type(kshark_ctx->pevent, data);
+	event = tep_data_event_from_type(kshark_ctx->pevent, event_id);
+	if (event)
+		info = kshark_get_info(kshark_ctx->pevent, data, event);
+
+	free_record(data);
+
+	return info;
+}
+
 /**
  * @brief Dump into a string the content of one entry. The function allocates
  *	  a null terminated string and returns a pointer to this string. The
diff --git a/kernel-shark-qt/src/libkshark.h b/kernel-shark-qt/src/libkshark.h
index e846c85..f00a584 100644
--- a/kernel-shark-qt/src/libkshark.h
+++ b/kernel-shark-qt/src/libkshark.h
@@ -148,6 +148,18 @@  void kshark_close(struct kshark_context *kshark_ctx);
 
 void kshark_free(struct kshark_context *kshark_ctx);
 
+int kshark_get_pid_easy(struct kshark_entry *entry);
+
+const char *kshark_get_task_easy(struct kshark_entry *entry);
+
+const char *kshark_get_latency_easy(struct kshark_entry *entry);
+
+int kshark_get_event_id_easy(struct kshark_entry *entry);
+
+const char *kshark_get_event_name_easy(struct kshark_entry *entry);
+
+const char *kshark_get_info_easy(struct kshark_entry *entry);
+
 char* kshark_dump_entry(const struct kshark_entry *entry);
 
 struct tep_record *kshark_read_at(struct kshark_context *kshark_ctx,