diff mbox series

[v2,07/20] kernel-shark: Add basic methods for Data streams

Message ID 20201012133523.469040-8-y.karadz@gmail.com (mailing list archive)
State Superseded
Headers show
Series Start KernelShark v2 transformation | expand

Commit Message

Yordan Karadzhov Oct. 12, 2020, 1:35 p.m. UTC
Here we introduce the basic mechanisms for using data streams.
For the moment these are just stand alone definitions and the
integration with the API is yet to be introduced in the following
patches.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 src/libkshark.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/libkshark.h | 171 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 368 insertions(+)

Comments

Steven Rostedt Oct. 13, 2020, 12:18 a.m. UTC | #1
On Mon, 12 Oct 2020 16:35:10 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> +static struct kshark_data_stream *kshark_stream_alloc()
> +{
> +	struct kshark_data_stream *stream;
> +
> +	stream = calloc(1, sizeof(*stream));
> +	if (!stream)
> +		goto fail;
> +
> +	stream->show_task_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
> +	stream->hide_task_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
> +
> +	stream->show_event_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
> +	stream->hide_event_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
> +
> +	stream->show_cpu_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
> +	stream->hide_cpu_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
> +
> +	stream->tasks = kshark_hash_id_alloc(KS_TASK_HASH_NBITS);
> +
> +	if (!stream->show_task_filter ||
> +	    !stream->hide_task_filter ||
> +	    !stream->show_event_filter ||
> +	    !stream->hide_event_filter ||
> +	    !stream->tasks) {
> +		    goto fail;
> +	}
> +
> +	stream->format = KS_INVALIDE_DATA;
> +
> +	return stream;
> +
> + fail:
> +	fprintf(stderr, "Failed to allocate memory for data stream.\n");

I don't think we need the print. Not if this is a library routine. The
calloc failure should set the errno that the caller should be able to
figure out what happened.

> +	if (stream)
> +		kshark_stream_free(stream);
> +
> +	return NULL;
> +}
> +
> +/**
> + * @brief Add new Data stream.
> + *
> + * @param kshark_ctx: Input location for context pointer.
> + *
> + * @returns Zero on success or a negative errno code on failure.
> + */
> +int kshark_add_stream(struct kshark_context *kshark_ctx)
> +{
> +	struct kshark_data_stream *stream;
> +
> +	if (kshark_ctx->n_streams == KS_MAX_NUM_STREAMS)
> +		return -EMFILE;
> +
> +	stream = kshark_stream_alloc();

Need to check for success.

> +	stream->stream_id = kshark_ctx->n_streams;
> +
> +	if (pthread_mutex_init(&stream->input_mutex, NULL) != 0) {
> +		kshark_stream_free(stream);
> +		return -EAGAIN;
> +	}
> +
> +	kshark_ctx->stream[kshark_ctx->n_streams++] = stream;
> +
> +	return stream->stream_id;
> +}
> +
> +/**
> + * @brief Get the Data stream object having given Id.
> + *
> + * @param kshark_ctx: Input location for context pointer.
> + * @param sd: Data stream identifier.
> + *
> + * @returns Pointer to a Data stream object if the sream exists. Otherwise
> + *	    NULL.
> + */
> +struct kshark_data_stream *
> +kshark_get_data_stream(struct kshark_context *kshark_ctx, int sd)
> +{
> +	if (sd >= 0 && sd < KS_MAX_NUM_STREAMS)
> +		return kshark_ctx->stream[sd];
> +
> +	return NULL;
> +}
> +
> +/**
> + * @brief Get the Data stream object corresponding to a given entry
> + *
> + * @param entry: Input location for the KernelShark entry.
> + *
> + * @returns Pointer to a Data stream object on success. Otherwise NULL.
> + */
> +struct kshark_data_stream *
> +kshark_get_stream_from_entry(const struct kshark_entry *entry)
> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +
> +	if (!kshark_instance(&kshark_ctx))
> +		return NULL;
> +
> +	return kshark_get_data_stream(kshark_ctx, entry->stream_id);
> +}
> +
> +/**
> + * @brief Get an array containing the Ids of all opened Trace data streams.
> + * 	  The User is responsible for freeing the array.
> + *
> + * @param kshark_ctx: Input location for context pointer.
> + */
> +int *kshark_all_streams(struct kshark_context *kshark_ctx)
> +{
> +	int *ids, i, count = 0;
> +
> +	ids = malloc(kshark_ctx->n_streams * (sizeof(*ids)));

I think calloc is more appropriate for the above.

	ids = calloc(kshark_ctx->n_streams, sizeof(*ids));

> +	if (!ids) {
> +		fprintf(stderr,
> +			"Failed to allocate memory for stream array.\n");

Probably don't need the print.

> +		return NULL;
> +	}
> +
> +	for (i = 0; i < KS_MAX_NUM_STREAMS; ++i)
> +		if (kshark_ctx->stream[i])
> +			ids[count++] = i;

Definitely need the calloc, as malloc doesn't initialize the array to
zero. Thus some ids[] will not be initialized.

> +
> +	return ids;
> +}
> +
>  /**
>   * @brief Close the trace data file and free the trace data handle.
>   *
> @@ -252,6 +399,56 @@ void kshark_free(struct kshark_context *kshark_ctx)
>  	free(kshark_ctx);
>  }
>  
> +/**
> + * @brief Get the name of the command/task from its Process Id.
> + *
> + * @param sd: Data stream identifier.
> + * @param pid: Process Id of the command/task.
> + */
> +char *kshark_comm_from_pid(int sd, int pid)

I wonder if we should abstract this further, and call it
"kshark_name_from_id()", as comm and pid are specific to processes, and
we may have a stream that will represent something other than processes.

> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +	struct kshark_data_stream *stream;
> +	struct kshark_entry e;
> +
> +	if (!kshark_instance(&kshark_ctx))
> +		return NULL;
> +
> +	stream = kshark_get_data_stream(kshark_ctx, sd);
> +	if (!stream)
> +		return NULL;
> +
> +	e.visible = KS_PLUGIN_UNTOUCHED_MASK;
> +	e.pid = pid;
> +
> +	return stream->interface.get_task(stream, &e);
> +}
> +
> +/**
> + * @brief Get the name of the event from its Id.
> + *
> + * @param sd: Data stream identifier.
> + * @param event_id: The unique Id of the event type.
> + */
> +char *kshark_event_from_id(int sd, int event_id)
> +{
> +	struct kshark_context *kshark_ctx = NULL;
> +	struct kshark_data_stream *stream;
> +	struct kshark_entry e;
> +
> +	if (!kshark_instance(&kshark_ctx))
> +		return NULL;
> +
> +	stream = kshark_get_data_stream(kshark_ctx, sd);
> +	if (!stream)
> +		return NULL;
> +
> +	e.visible = KS_PLUGIN_UNTOUCHED_MASK;
> +	e.event_id = event_id;
> +
> +	return stream->interface.get_event_name(stream, &e);
> +}
> +
>  static struct kshark_task_list *
>  kshark_find_task(struct kshark_context *kshark_ctx, uint32_t key, int pid)
>  {
> diff --git a/src/libkshark.h b/src/libkshark.h
> index fe0ba7f2..e299d067 100644
> --- a/src/libkshark.h
> +++ b/src/libkshark.h
> @@ -340,6 +340,12 @@ struct kshark_task_list {
>  
>  /** Structure representing a kshark session. */
>  struct kshark_context {
> +	/** Array of data stream descriptors. */
> +	struct kshark_data_stream	**stream;
> +
> +	/** The number of data streams. */
> +	int				n_streams;
> +
>  	/** Input handle for the trace data file. */
>  	struct tracecmd_input	*handle;
>  
> @@ -397,6 +403,16 @@ bool kshark_instance(struct kshark_context **kshark_ctx);
>  
>  bool kshark_open(struct kshark_context *kshark_ctx, const char *file);
>  
> +int kshark_add_stream(struct kshark_context *kshark_ctx);
> +
> +struct kshark_data_stream *
> +kshark_get_data_stream(struct kshark_context *kshark_ctx, int sd);
> +
> +struct kshark_data_stream *
> +kshark_get_stream_from_entry(const struct kshark_entry *entry);
> +
> +int *kshark_all_streams(struct kshark_context *kshark_ctx);
> +
>  ssize_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
>  				 struct kshark_entry ***data_rows);
>  
> @@ -416,6 +432,10 @@ void kshark_close(struct kshark_context *kshark_ctx);
>  
>  void kshark_free(struct kshark_context *kshark_ctx);
>  
> +char *kshark_comm_from_pid(int sd, int pid);
> +
> +char *kshark_event_from_id(int sd, int event_id);
> +
>  int kshark_get_pid_easy(struct kshark_entry *entry);
>  
>  const char *kshark_get_task_easy(struct kshark_entry *entry);
> @@ -432,6 +452,157 @@ void kshark_convert_nano(uint64_t time, uint64_t *sec, uint64_t *usec);
>  
>  char* kshark_dump_entry(const struct kshark_entry *entry);
>  
> +static inline int kshark_get_pid(const struct kshark_entry *entry)
> +{
> +	struct kshark_data_stream *stream =
> +		kshark_get_stream_from_entry(entry);
> +
> +	if (!stream)
> +		return -1;
> +
> +	return stream->interface.get_pid(stream, entry);
> +}
> +
> +static inline int kshark_get_event_id(const struct kshark_entry *entry)
> +{
> +	struct kshark_data_stream *stream =
> +		kshark_get_stream_from_entry(entry);
> +
> +	if (!stream)
> +		return -1;
> +
> +	return stream->interface.get_event_id(stream, entry);
> +}
> +
> +static inline int *kshark_get_all_event_ids(struct kshark_data_stream *stream)
> +{
> +	return stream->interface.get_all_event_ids(stream);
> +}
> +
> +static inline char *kshark_get_event_name(const struct kshark_entry *entry)
> +{
> +	struct kshark_data_stream *stream =
> +		kshark_get_stream_from_entry(entry);
> +
> +	if (!stream)
> +		return NULL;
> +
> +	return stream->interface.get_event_name(stream, entry);
> +}
> +
> +static inline char *kshark_get_task(const struct kshark_entry *entry)
> +{
> +	struct kshark_data_stream *stream =
> +		kshark_get_stream_from_entry(entry);
> +
> +	if (!stream)
> +		return NULL;
> +
> +	return stream->interface.get_task(stream, entry);
> +}
> +
> +static inline char *kshark_get_latency(const struct kshark_entry *entry)
> +{
> +	struct kshark_data_stream *stream =
> +		kshark_get_stream_from_entry(entry);
> +
> +	if (!stream)
> +		return NULL;
> +
> +	return stream->interface.get_latency(stream, entry);
> +}
> +
> +static inline char *kshark_get_info(const struct kshark_entry *entry)
> +{
> +	struct kshark_data_stream *stream =
> +		kshark_get_stream_from_entry(entry);
> +
> +	if (!stream)
> +		return NULL;
> +
> +	return stream->interface.get_info(stream, entry);
> +}
> +
> +static inline int kshark_read_event_field(const struct kshark_entry *entry,
> +					  const char* field, int64_t *val)
> +{
> +	struct kshark_data_stream *stream =
> +		kshark_get_stream_from_entry(entry);
> +
> +	if (!stream)
> +		return -1;
> +
> +	return stream->interface.read_event_field_int64(stream, entry,
> +							field, val);
> +}
> +
> +/**
> + * @brief Load the content of the trace data file asociated with a given
> + *	  Data stream identifie into an array of kshark_entries.
> + *	  If one or more filters are set, the "visible" fields of each entry
> + *	  is updated according to the criteria provided by the filters. The
> + *	  field "filter_mask" of the session's context is used to control the
> + *	  level of visibility/invisibility of the filtered entries.
> + *
> + * @param kshark_ctx: Input location for context pointer.
> + * @param sd: Data stream identifier.
> + * @param data_rows: Output location for the trace data. The user is
> + *		     responsible for freeing the elements of the outputted
> + *		     array.
> + *
> + * @returns The size of the outputted data in the case of success, or a
> + *	    negative error code on failure.
> + */
> +static inline ssize_t kshark_load_entries(struct kshark_context *kshark_ctx,
> +					  int sd,
> +					  struct kshark_entry ***data_rows)
> +{
> +	struct kshark_data_stream *stream;
> +
> +	stream = kshark_get_data_stream(kshark_ctx, sd);
> +	if (!stream)
> +		return -EBADF;
> +
> +	return stream->interface.load_entries(stream, kshark_ctx, data_rows);
> +}
> +
> +/**
> + * @brief Load the content of the trace data file asociated with a given
> + *	  Data stream identifie into a data matrix. The user is responsible

identifie?

> + *	  for freeing the outputted data.
> + *
> + * @param kshark_ctx: Input location for context pointer.
> + * @param sd: Data stream identifier.
> + * @param cpu_array: Output location for the CPU column (array) of the matrix.
> + * @param event_array: Output location for the Event Id column (array) of the
> + *		       matrix.
> + * @param _array: Output location for the  column (array) of the matrix.
> + * @param offset_array: Output location for the offset column (array) of the
> + *			matrix.
> + * @param ts_array: Output location for the time stamp column (array) of the
> + *		    matrix.

Hmm, how is this matrix composed? How do each realate, via the ts_array?

-- Steve


> + */
> +static inline ssize_t kshark_load_matrix(struct kshark_context *kshark_ctx,
> +					 int sd,
> +					 int16_t **cpu_array,
> +					 int32_t **pid_array,
> +					 int32_t **event_array,
> +					 int64_t **offset_array,
> +					 int64_t **ts_array)
> +{
> +	struct kshark_data_stream *stream;
> +
> +	stream = kshark_get_data_stream(kshark_ctx, sd);
> +	if (!stream)
> +		return -EBADF;
> +
> +	return stream->interface.load_matrix(stream, kshark_ctx, cpu_array,
> +								 pid_array,
> +								 event_array,
> +								 offset_array,
> +								 ts_array);
> +}
> +
>  /**
>   * Custom entry info function type. To be user for dumping info for custom
>   * KernelShark entryes.
Yordan Karadzhov Oct. 29, 2020, 10:10 a.m. UTC | #2
On 13.10.20 г. 3:18 ч., Steven Rostedt wrote:
> On Mon, 12 Oct 2020 16:35:10 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
>> +static struct kshark_data_stream *kshark_stream_alloc()
>> +{
>> +	struct kshark_data_stream *stream;
>> +
>> +
>> +	return stream;
>> +
>> + fail:
>> +	fprintf(stderr, "Failed to allocate memory for data stream.\n");
> 
> I don't think we need the print. Not if this is a library routine. The
> calloc failure should set the errno that the caller should be able to
> figure out what happened.
> 

Agree. Will be fixed in v3.

>> +	if (stream)
>> +		kshark_stream_free(stream);
>> +
>> +	return NULL;
>> +}
>> +
>> +/**
>> + * @brief Add new Data stream.
>> + *
>> + * @param kshark_ctx: Input location for context pointer.
>> + *
>> + * @returns Zero on success or a negative errno code on failure.
>> + */
>> +int kshark_add_stream(struct kshark_context *kshark_ctx)
>> +{
>> +	struct kshark_data_stream *stream;
>> +
>> +	if (kshark_ctx->n_streams == KS_MAX_NUM_STREAMS)
>> +		return -EMFILE;
>> +
>> +	stream = kshark_stream_alloc();
> 
> Need to check for success.

OK. Fix in v3.

> 
>> +	stream->stream_id = kshark_ctx->n_streams;
>> +
>> +	if (pthread_mutex_init(&stream->input_mutex, NULL) != 0) {
>> +		kshark_stream_free(stream);
>> +		return -EAGAIN;

>> +/**
>> + * @brief Get an array containing the Ids of all opened Trace data streams.
>> + * 	  The User is responsible for freeing the array.
>> + *
>> + * @param kshark_ctx: Input location for context pointer.
>> + */
>> +int *kshark_all_streams(struct kshark_context *kshark_ctx)
>> +{
>> +	int *ids, i, count = 0;
>> +
>> +	ids = malloc(kshark_ctx->n_streams * (sizeof(*ids)));
> 
> I think calloc is more appropriate for the above.
> 
> 	ids = calloc(kshark_ctx->n_streams, sizeof(*ids));
> 

Fix in v3.

>> +	if (!ids) {
>> +		fprintf(stderr,
>> +			"Failed to allocate memory for stream array.\n");
> 
> Probably don't need the print.

Fix in v3.

> 
>> +		return NULL;
>> +	}
>> +
>> +	for (i = 0; i < KS_MAX_NUM_STREAMS; ++i)
>> +		if (kshark_ctx->stream[i])
>> +			ids[count++] = i;
> 
> Definitely need the calloc, as malloc doesn't initialize the array to
> zero. Thus some ids[] will not be initialized.
> 
>> +
>> +	return ids;
>> +}
>> +
>>   /**
>>    * @brief Close the trace data file and free the trace data handle.
>>    *
>> @@ -252,6 +399,56 @@ void kshark_free(struct kshark_context *kshark_ctx)
>>   	free(kshark_ctx);
>>   }
>>   
>> +/**
>> + * @brief Get the name of the command/task from its Process Id.
>> + *
>> + * @param sd: Data stream identifier.
>> + * @param pid: Process Id of the command/task.
>> + */
>> +char *kshark_comm_from_pid(int sd, int pid)
> 
> I wonder if we should abstract this further, and call it
> "kshark_name_from_id()", as comm and pid are specific to processes, and
> we may have a stream that will represent something other than processes.
> 

This is just a helper function that wraps the corresponding method of 
the interface. In the future we can add a similar helper/wrapper 
function with different name if we have streams that contain no processes.

However, you are actually making a very good point. It may be even 
better if we abstract the interface itself, instead of trying to have 
more abstract method names. We can keep the existing interface of 
methods unchanged, but in the definition of "struct kshark_data_stream" 
make the interface void*

struct kshark_data_stream {
	/** Data stream identifier. */
	uint8_t			stream_id;

....

	/** List of Plugin's Draw handlers. */
	struct kshark_draw_handler		*draw_handlers;

	/**
	 * Abstract interface of methods used to operate over the data
	 * from a given stream. An implementation must be provided.
	 */
	void	*interface;
};

and then the wrapping functions may look like this

char *kshark_comm_from_pid(int sd, int pid)
{
	struct kshark_data_stream_interface *interface;
	struct kshark_context *kshark_ctx = NULL;
	struct kshark_data_stream *stream;
	struct kshark_entry e;

	if (!kshark_instance(&kshark_ctx))
		return NULL;

	stream = kshark_get_data_stream(kshark_ctx, sd);
	if (!stream)
		return NULL;

	interface = stream->interface;
	if (!interface || !interface->get_task)
		return NULL;

	e.visible = KS_PLUGIN_UNTOUCHED_MASK;
	e.pid = pid;

	return interface->get_task(stream, &e);
}

And in the future we can add more interface implementations and more 
helper functions.

What do you think?

Thanks!
Yordan



>> +{
>> +	struct kshark_context *kshark_ctx = NULL;
>> +	struct kshark_data_stream *stream;
>> +	struct kshark_entry e;
>> +
>> +	if (!kshark_instance(&kshark_ctx))
>> +		return NULL;
>> +
>> +	stream = kshark_get_data_stream(kshark_ctx, sd);
>> +	if (!stream)
>> +		return NULL;
>> +
>> +	e.visible = KS_PLUGIN_UNTOUCHED_MASK;
>> +	e.pid = pid;
>> +
>> +	return stream->interface.get_task(stream, &e);
>> +}
>> +
>> +/**
>> + * @brief Get the name of the event from its Id.
>> + *
>> + * @param sd: Data stream identifier.
>> + * @param event_id: The unique Id of the event type.
>> + */
>> +char *kshark_event_from_id(int sd, int event_id)
>> +{
>> +	struct kshark_context *kshark_ctx = NULL;
>> +	struct kshark_data_stream *stream;
>> +	struct kshark_entry e;
>> +
>> +	if (!kshark_instance(&kshark_ctx))
>> +		return NULL;
>> +
>> +	stream = kshark_get_data_stream(kshark_ctx, sd);
>> +	if (!stream)
>> +		return NULL;
>> +
>> +	e.visible = KS_PLUGIN_UNTOUCHED_MASK;
>> +	e.event_id = event_id;
>> +
>> +	return stream->interface.get_event_name(stream, &e);
>> +}
>> +
>>   static struct kshark_task_list *
>>   kshark_find_task(struct kshark_context *kshark_ctx, uint32_t key, int pid)
>>   {
>> diff --git a/src/libkshark.h b/src/libkshark.h
>> index fe0ba7f2..e299d067 100644
>> --- a/src/libkshark.h
>> +++ b/src/libkshark.h
>> @@ -340,6 +340,12 @@ struct kshark_task_list {
>>   
>>   /** Structure representing a kshark session. */
>>   struct kshark_context {
>> +	/** Array of data stream descriptors. */
>> +	struct kshark_data_stream	**stream;
>> +
>> +	/** The number of data streams. */
>> +	int				n_streams;
>> +
>>   	/** Input handle for the trace data file. */
>>   	struct tracecmd_input	*handle;
>>   
>> @@ -397,6 +403,16 @@ bool kshark_instance(struct kshark_context **kshark_ctx);
>>   
>>   bool kshark_open(struct kshark_context *kshark_ctx, const char *file);
>>   
>> +int kshark_add_stream(struct kshark_context *kshark_ctx);
>> +
>> +struct kshark_data_stream *
>> +kshark_get_data_stream(struct kshark_context *kshark_ctx, int sd);
>> +
>> +struct kshark_data_stream *
>> +kshark_get_stream_from_entry(const struct kshark_entry *entry);
>> +
>> +int *kshark_all_streams(struct kshark_context *kshark_ctx);
>> +
>>   ssize_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
>>   				 struct kshark_entry ***data_rows);
>>   
>> @@ -416,6 +432,10 @@ void kshark_close(struct kshark_context *kshark_ctx);
>>   
>>   void kshark_free(struct kshark_context *kshark_ctx);
>>   
>> +char *kshark_comm_from_pid(int sd, int pid);
>> +
>> +char *kshark_event_from_id(int sd, int event_id);
>> +
>>   int kshark_get_pid_easy(struct kshark_entry *entry);
>>   
>>   const char *kshark_get_task_easy(struct kshark_entry *entry);
>> @@ -432,6 +452,157 @@ void kshark_convert_nano(uint64_t time, uint64_t *sec, uint64_t *usec);
>>   
>>   char* kshark_dump_entry(const struct kshark_entry *entry);
>>   
>> +static inline int kshark_get_pid(const struct kshark_entry *entry)
>> +{
>> +	struct kshark_data_stream *stream =
>> +		kshark_get_stream_from_entry(entry);
>> +
>> +	if (!stream)
>> +		return -1;
>> +
>> +	return stream->interface.get_pid(stream, entry);
>> +}
>> +
>> +static inline int kshark_get_event_id(const struct kshark_entry *entry)
>> +{
>> +	struct kshark_data_stream *stream =
>> +		kshark_get_stream_from_entry(entry);
>> +
>> +	if (!stream)
>> +		return -1;
>> +
>> +	return stream->interface.get_event_id(stream, entry);
>> +}
>> +
>> +static inline int *kshark_get_all_event_ids(struct kshark_data_stream *stream)
>> +{
>> +	return stream->interface.get_all_event_ids(stream);
>> +}
>> +
>> +static inline char *kshark_get_event_name(const struct kshark_entry *entry)
>> +{
>> +	struct kshark_data_stream *stream =
>> +		kshark_get_stream_from_entry(entry);
>> +
>> +	if (!stream)
>> +		return NULL;
>> +
>> +	return stream->interface.get_event_name(stream, entry);
>> +}
>> +
>> +static inline char *kshark_get_task(const struct kshark_entry *entry)
>> +{
>> +	struct kshark_data_stream *stream =
>> +		kshark_get_stream_from_entry(entry);
>> +
>> +	if (!stream)
>> +		return NULL;
>> +
>> +	return stream->interface.get_task(stream, entry);
>> +}
>> +
>> +static inline char *kshark_get_latency(const struct kshark_entry *entry)
>> +{
>> +	struct kshark_data_stream *stream =
>> +		kshark_get_stream_from_entry(entry);
>> +
>> +	if (!stream)
>> +		return NULL;
>> +
>> +	return stream->interface.get_latency(stream, entry);
>> +}
>> +
>> +static inline char *kshark_get_info(const struct kshark_entry *entry)
>> +{
>> +	struct kshark_data_stream *stream =
>> +		kshark_get_stream_from_entry(entry);
>> +
>> +	if (!stream)
>> +		return NULL;
>> +
>> +	return stream->interface.get_info(stream, entry);
>> +}
>> +
>> +static inline int kshark_read_event_field(const struct kshark_entry *entry,
>> +					  const char* field, int64_t *val)
>> +{
>> +	struct kshark_data_stream *stream =
>> +		kshark_get_stream_from_entry(entry);
>> +
>> +	if (!stream)
>> +		return -1;
>> +
>> +	return stream->interface.read_event_field_int64(stream, entry,
>> +							field, val);
>> +}
>> +
>> +/**
>> + * @brief Load the content of the trace data file asociated with a given
>> + *	  Data stream identifie into an array of kshark_entries.
>> + *	  If one or more filters are set, the "visible" fields of each entry
>> + *	  is updated according to the criteria provided by the filters. The
>> + *	  field "filter_mask" of the session's context is used to control the
>> + *	  level of visibility/invisibility of the filtered entries.
>> + *
>> + * @param kshark_ctx: Input location for context pointer.
>> + * @param sd: Data stream identifier.
>> + * @param data_rows: Output location for the trace data. The user is
>> + *		     responsible for freeing the elements of the outputted
>> + *		     array.
>> + *
>> + * @returns The size of the outputted data in the case of success, or a
>> + *	    negative error code on failure.
>> + */
>> +static inline ssize_t kshark_load_entries(struct kshark_context *kshark_ctx,
>> +					  int sd,
>> +					  struct kshark_entry ***data_rows)
>> +{
>> +	struct kshark_data_stream *stream;
>> +
>> +	stream = kshark_get_data_stream(kshark_ctx, sd);
>> +	if (!stream)
>> +		return -EBADF;
>> +
>> +	return stream->interface.load_entries(stream, kshark_ctx, data_rows);
>> +}
>> +
>> +/**
>> + * @brief Load the content of the trace data file asociated with a given
>> + *	  Data stream identifie into a data matrix. The user is responsible
> 
> identifie?
> 
>> + *	  for freeing the outputted data.
>> + *
>> + * @param kshark_ctx: Input location for context pointer.
>> + * @param sd: Data stream identifier.
>> + * @param cpu_array: Output location for the CPU column (array) of the matrix.
>> + * @param event_array: Output location for the Event Id column (array) of the
>> + *		       matrix.
>> + * @param _array: Output location for the  column (array) of the matrix.
>> + * @param offset_array: Output location for the offset column (array) of the
>> + *			matrix.
>> + * @param ts_array: Output location for the time stamp column (array) of the
>> + *		    matrix.
> 
> Hmm, how is this matrix composed? How do each realate, via the ts_array?
> 
> -- Steve
> 
> 
>> + */
>> +static inline ssize_t kshark_load_matrix(struct kshark_context *kshark_ctx,
>> +					 int sd,
>> +					 int16_t **cpu_array,
>> +					 int32_t **pid_array,
>> +					 int32_t **event_array,
>> +					 int64_t **offset_array,
>> +					 int64_t **ts_array)
>> +{
>> +	struct kshark_data_stream *stream;
>> +
>> +	stream = kshark_get_data_stream(kshark_ctx, sd);
>> +	if (!stream)
>> +		return -EBADF;
>> +
>> +	return stream->interface.load_matrix(stream, kshark_ctx, cpu_array,
>> +								 pid_array,
>> +								 event_array,
>> +								 offset_array,
>> +								 ts_array);
>> +}
>> +
>>   /**
>>    * Custom entry info function type. To be user for dumping info for custom
>>    * KernelShark entryes.
>
Steven Rostedt Oct. 29, 2020, 2:04 p.m. UTC | #3
On Thu, 29 Oct 2020 12:10:36 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> >> +/**
> >> + * @brief Get the name of the command/task from its Process Id.
> >> + *
> >> + * @param sd: Data stream identifier.
> >> + * @param pid: Process Id of the command/task.
> >> + */
> >> +char *kshark_comm_from_pid(int sd, int pid)  
> > 
> > I wonder if we should abstract this further, and call it
> > "kshark_name_from_id()", as comm and pid are specific to processes, and
> > we may have a stream that will represent something other than processes.
> >   
> 
> This is just a helper function that wraps the corresponding method of 
> the interface. In the future we can add a similar helper/wrapper 
> function with different name if we have streams that contain no processes.
> 
> However, you are actually making a very good point. It may be even 
> better if we abstract the interface itself, instead of trying to have 
> more abstract method names. We can keep the existing interface of 
> methods unchanged, but in the definition of "struct kshark_data_stream" 
> make the interface void*
> 
> struct kshark_data_stream {
> 	/** Data stream identifier. */
> 	uint8_t			stream_id;
> 
> ....
> 
> 	/** List of Plugin's Draw handlers. */
> 	struct kshark_draw_handler		*draw_handlers;
> 
> 	/**
> 	 * Abstract interface of methods used to operate over the data
> 	 * from a given stream. An implementation must be provided.
> 	 */
> 	void	*interface;
> };
> 
> and then the wrapping functions may look like this
> 
> char *kshark_comm_from_pid(int sd, int pid)
> {
> 	struct kshark_data_stream_interface *interface;
> 	struct kshark_context *kshark_ctx = NULL;
> 	struct kshark_data_stream *stream;
> 	struct kshark_entry e;
> 
> 	if (!kshark_instance(&kshark_ctx))
> 		return NULL;
> 
> 	stream = kshark_get_data_stream(kshark_ctx, sd);
> 	if (!stream)
> 		return NULL;
> 
> 	interface = stream->interface;
> 	if (!interface || !interface->get_task)
> 		return NULL;
> 
> 	e.visible = KS_PLUGIN_UNTOUCHED_MASK;
> 	e.pid = pid;
> 
> 	return interface->get_task(stream, &e);
> }
> 
> And in the future we can add more interface implementations and more 
> helper functions.
> 
> What do you think?


Do we need to make the interface "void *" and not just a non defined type
"struct kshark_data_stream_interface *"?

Just have:

struct kshark_data_stream_interface;

And then reference it without defining it, and have the streams define it.
If you need this to have inheritance and a bit of polymorphism you can do
that too :-) That is, if you want this interface to have something common
among all streams.

struct kshark_data_stream_interface {
	int		type;
	int		common_data;
};

then local to the file that implements it:

struct data_stream_interface {
	struct kshark_data_stream_interface	kinterface;
	int					unique_data;
};


{
	struct data_stream_interface	*interface;


	interface = (struct data_stream_interface *)stream->interface;

	if (interface->kinterface.type != my_type)
		return (or error);

	unique_data = interface->unique_data;

}


This is even how the trace events work in the kernel. For example:

struct trace_entry {
	unsigned short		type;
	unsigned char		flags;
	unsigned char		preempt_count;
	int			pid;
};

struct kprobe_trace_entry_head {
	struct trace_entry	ent;
	unsigned long		ip;
};

struct kretprobe_trace_entry_head {
	struct trace_entry	ent;
	unsigned long		func;
	unsigned long		ret_ip;
};

-- Steve
Yordan Karadzhov Oct. 29, 2020, 2:49 p.m. UTC | #4
On 29.10.20 г. 16:04 ч., Steven Rostedt wrote:
> On Thu, 29 Oct 2020 12:10:36 +0200
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>>>> +/**
>>>> + * @brief Get the name of the command/task from its Process Id.
>>>> + *
>>>> + * @param sd: Data stream identifier.
>>>> + * @param pid: Process Id of the command/task.
>>>> + */
>>>> +char *kshark_comm_from_pid(int sd, int pid)
>>>
>>> I wonder if we should abstract this further, and call it
>>> "kshark_name_from_id()", as comm and pid are specific to processes, and
>>> we may have a stream that will represent something other than processes.
>>>    
>>
>> This is just a helper function that wraps the corresponding method of
>> the interface. In the future we can add a similar helper/wrapper
>> function with different name if we have streams that contain no processes.
>>
>> However, you are actually making a very good point. It may be even
>> better if we abstract the interface itself, instead of trying to have
>> more abstract method names. We can keep the existing interface of
>> methods unchanged, but in the definition of "struct kshark_data_stream"
>> make the interface void*
>>
>> struct kshark_data_stream {
>> 	/** Data stream identifier. */
>> 	uint8_t			stream_id;
>>
>> ....
>>
>> 	/** List of Plugin's Draw handlers. */
>> 	struct kshark_draw_handler		*draw_handlers;
>>
>> 	/**
>> 	 * Abstract interface of methods used to operate over the data
>> 	 * from a given stream. An implementation must be provided.
>> 	 */
>> 	void	*interface;
>> };
>>
>> and then the wrapping functions may look like this
>>
>> char *kshark_comm_from_pid(int sd, int pid)
>> {
>> 	struct kshark_data_stream_interface *interface;
>> 	struct kshark_context *kshark_ctx = NULL;
>> 	struct kshark_data_stream *stream;
>> 	struct kshark_entry e;
>>
>> 	if (!kshark_instance(&kshark_ctx))
>> 		return NULL;
>>
>> 	stream = kshark_get_data_stream(kshark_ctx, sd);
>> 	if (!stream)
>> 		return NULL;
>>
>> 	interface = stream->interface;
>> 	if (!interface || !interface->get_task)
>> 		return NULL;
>>
>> 	e.visible = KS_PLUGIN_UNTOUCHED_MASK;
>> 	e.pid = pid;
>>
>> 	return interface->get_task(stream, &e);
>> }
>>
>> And in the future we can add more interface implementations and more
>> helper functions.
>>
>> What do you think?
> 
> 
> Do we need to make the interface "void *" and not just a non defined type
> "struct kshark_data_stream_interface *"?
> 
> Just have:
> 
> struct kshark_data_stream_interface;
> 
> And then reference it without defining it, and have the streams define it.
> If you need this to have inheritance and a bit of polymorphism you can do
> that too :-) That is, if you want this interface to have something common
> among all streams.
> 

Maybe I don't understand your idea very well, but I think what you 
suggest has very different behavior. What I want is the implementation 
of the interface to stay in the same header file (libkshark.h). In the 
future we can add more interfaces but this will be again in the same 
header (libkshark.h).

The readout plugins will include libkshark.h and will have to chose one 
of the available interfaces and implement its methods like this:

static int kshark_tep_stream_init(struct kshark_data_stream *stream,
				  struct tracecmd_input *input)
{
	struct kshark_data_stream_interface *interface;

	stream->interface = interface = calloc(1, sizeof(*interface));
	if (!interface)
		return -ENOMEM;

....

	interface->get_pid = tepdata_get_pid;
	interface->get_task = tepdata_get_task;
	interface->get_event_id = tepdata_get_event_id;

....

Note that the plugins will include libkshark.h but libkshark will never 
include headers from plugins.

Does it make any sense, or maybe I just don't understand your suggestion?

Thanks a lot!
Yordan

> struct kshark_data_stream_interface {
> 	int		type;
> 	int		common_data;
> };
> 
> then local to the file that implements it:
> 
> struct data_stream_interface {
> 	struct kshark_data_stream_interface	kinterface;
> 	int					unique_data;
> };
> 
> 
> {
> 	struct data_stream_interface	*interface;
> 
> 
> 	interface = (struct data_stream_interface *)stream->interface;
> 
> 	if (interface->kinterface.type != my_type)
> 		return (or error);
> 
> 	unique_data = interface->unique_data;
> 
> }
> 
> 
> This is even how the trace events work in the kernel. For example:
> 
> struct trace_entry {
> 	unsigned short		type;
> 	unsigned char		flags;
> 	unsigned char		preempt_count;
> 	int			pid;
> };
> 
> struct kprobe_trace_entry_head {
> 	struct trace_entry	ent;
> 	unsigned long		ip;
> };
> 
> struct kretprobe_trace_entry_head {
> 	struct trace_entry	ent;
> 	unsigned long		func;
> 	unsigned long		ret_ip;
> };
> 
> -- Steve
>
Steven Rostedt Oct. 30, 2020, 1:57 a.m. UTC | #5
On Thu, 29 Oct 2020 16:49:03 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> Maybe I don't understand your idea very well, but I think what you 
> suggest has very different behavior. What I want is the implementation 
> of the interface to stay in the same header file (libkshark.h). In the 
> future we can add more interfaces but this will be again in the same 
> header (libkshark.h).

Maybe I got confused ;-)

Is there going to be different interface structures? Why the "void *"
and not just supply a "struct kshark_data_stream *"?

That way at least you have some kind of type checking when tasks move
things around. I try to avoid using "void *" because it can easily be
the source of unwanted bugs, due to the lack of type checking.

-- Steve
Yordan Karadzhov Nov. 3, 2020, 1:38 p.m. UTC | #6
On 30.10.20 г. 3:57 ч., Steven Rostedt wrote:
> On Thu, 29 Oct 2020 16:49:03 +0200
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> Maybe I don't understand your idea very well, but I think what you
>> suggest has very different behavior. What I want is the implementation
>> of the interface to stay in the same header file (libkshark.h). In the
>> future we can add more interfaces but this will be again in the same
>> header (libkshark.h).
> 
> Maybe I got confused ;-)
> 
> Is there going to be different interface structures? Why the "void *"
> and not just supply a "struct kshark_data_stream *"?

Hi Steven,

Yes, my idea is that in the future we may decide to change something in 
the interface, or to have a second completely different interface, while 
we may still need to keep the interface version that we have now for 
backward compatibility.

> 
> That way at least you have some kind of type checking when tasks move
> things around. I try to avoid using "void *" because it can easily be
> the source of unwanted bugs, due to the lack of type checking.

What if we define the interface to start with an integer identifier?

/** Data interface identifier. */
typedef enum kshark_data_interface_id {
	/** An interface with unknown type. */
	KS_INVALIDE_INTERFACE,

	/** Generic interface suitable Ftrace data. */
	KS_GENERIC_DATA_INTERFACE,
} kshark_data_interface_id;

/**
  * Structure representing the interface of methods used to operate over
  * the data from a given stream.
  */
struct kshark_generic_stream_interface {
	/** Interface version identifier. */
	int			type; /* MUST BE FIRST ENTRY */

	/** Method used to retrieve the Process Id of the entry. */
	stream_get_int_func	get_pid;

	/** Method used to retrieve the Event Id of the entry. */
	stream_get_int_func	get_event_id;

....

and it can be used like this:

char *kshark_get_aux_field(const struct kshark_entry *entry)
{
	struct kshark_generic_stream_interface *interface;
	struct kshark_data_stream *stream =
		kshark_get_stream_from_entry(entry);

	....

	interface = stream->interface;
	if (interface->type == KS_GENERIC_DATA_INTERFACE &&
	    interface->aux_field)
		return interface->aux_field(stream, entry);

	return NULL;
}

What do you think?


Thanks a lot!
Yordan


> 
> -- Steve
>
Steven Rostedt Nov. 4, 2020, 3:41 p.m. UTC | #7
On Tue, 3 Nov 2020 15:38:33 +0200
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> What if we define the interface to start with an integer identifier?
> 
> /** Data interface identifier. */
> typedef enum kshark_data_interface_id {
> 	/** An interface with unknown type. */
> 	KS_INVALIDE_INTERFACE,
> 
> 	/** Generic interface suitable Ftrace data. */
> 	KS_GENERIC_DATA_INTERFACE,
> } kshark_data_interface_id;
> 
> /**
>   * Structure representing the interface of methods used to operate over
>   * the data from a given stream.
>   */
> struct kshark_generic_stream_interface {
> 	/** Interface version identifier. */
> 	int			type; /* MUST BE FIRST ENTRY */
> 
> 	/** Method used to retrieve the Process Id of the entry. */
> 	stream_get_int_func	get_pid;
> 
> 	/** Method used to retrieve the Event Id of the entry. */
> 	stream_get_int_func	get_event_id;
> 
> ....
> 
> and it can be used like this:
> 
> char *kshark_get_aux_field(const struct kshark_entry *entry)
> {
> 	struct kshark_generic_stream_interface *interface;
> 	struct kshark_data_stream *stream =
> 		kshark_get_stream_from_entry(entry);
> 
> 	....
> 
> 	interface = stream->interface;
> 	if (interface->type == KS_GENERIC_DATA_INTERFACE &&
> 	    interface->aux_field)
> 		return interface->aux_field(stream, entry);
> 
> 	return NULL;
> }
> 
> What do you think?


This was actually what I was getting at ;-)

That is a common practice in the Linux kernel.

-- Steve
Yordan Karadzhov Nov. 5, 2020, 2:35 p.m. UTC | #8
On 4.11.20 г. 17:41 ч., Steven Rostedt wrote:
> On Tue, 3 Nov 2020 15:38:33 +0200
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> What if we define the interface to start with an integer identifier?
>>
>> /** Data interface identifier. */
>> typedef enum kshark_data_interface_id {
>> 	/** An interface with unknown type. */
>> 	KS_INVALIDE_INTERFACE,
>>
>> 	/** Generic interface suitable Ftrace data. */
>> 	KS_GENERIC_DATA_INTERFACE,
>> } kshark_data_interface_id;
>>
>> /**
>>    * Structure representing the interface of methods used to operate over
>>    * the data from a given stream.
>>    */
>> struct kshark_generic_stream_interface {
>> 	/** Interface version identifier. */
>> 	int			type; /* MUST BE FIRST ENTRY */
>>
>> 	/** Method used to retrieve the Process Id of the entry. */
>> 	stream_get_int_func	get_pid;
>>
>> 	/** Method used to retrieve the Event Id of the entry. */
>> 	stream_get_int_func	get_event_id;
>>
>> ....
>>
>> and it can be used like this:
>>
>> char *kshark_get_aux_field(const struct kshark_entry *entry)
>> {
>> 	struct kshark_generic_stream_interface *interface;
>> 	struct kshark_data_stream *stream =
>> 		kshark_get_stream_from_entry(entry);
>>
>> 	....
>>
>> 	interface = stream->interface;
>> 	if (interface->type == KS_GENERIC_DATA_INTERFACE &&
>> 	    interface->aux_field)
>> 		return interface->aux_field(stream, entry);
>>
>> 	return NULL;
>> }
>>
>> What do you think?
> 
> 
> This was actually what I was getting at ;-)
> 
> That is a common practice in the Linux kernel.

OK, thanks a lot!
Will be implemented in v3.

cheers,
Yordan

> 
> -- Steve
>
diff mbox series

Patch

diff --git a/src/libkshark.c b/src/libkshark.c
index 6a1258dc..ffeb66a7 100644
--- a/src/libkshark.c
+++ b/src/libkshark.c
@@ -166,6 +166,153 @@  bool kshark_open(struct kshark_context *kshark_ctx, const char *file)
 	return true;
 }
 
+static void kshark_stream_free(struct kshark_data_stream *stream)
+{
+	if (!stream)
+		return;
+
+	kshark_hash_id_free(stream->show_task_filter);
+	kshark_hash_id_free(stream->hide_task_filter);
+
+	kshark_hash_id_free(stream->show_event_filter);
+	kshark_hash_id_free(stream->hide_event_filter);
+
+	kshark_hash_id_free(stream->show_cpu_filter);
+	kshark_hash_id_free(stream->hide_cpu_filter);
+
+	kshark_hash_id_free(stream->tasks);
+
+	free(stream->file);
+	free(stream->name);
+	free(stream);
+}
+
+static struct kshark_data_stream *kshark_stream_alloc()
+{
+	struct kshark_data_stream *stream;
+
+	stream = calloc(1, sizeof(*stream));
+	if (!stream)
+		goto fail;
+
+	stream->show_task_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
+	stream->hide_task_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
+
+	stream->show_event_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
+	stream->hide_event_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
+
+	stream->show_cpu_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
+	stream->hide_cpu_filter = kshark_hash_id_alloc(KS_FILTER_HASH_NBITS);
+
+	stream->tasks = kshark_hash_id_alloc(KS_TASK_HASH_NBITS);
+
+	if (!stream->show_task_filter ||
+	    !stream->hide_task_filter ||
+	    !stream->show_event_filter ||
+	    !stream->hide_event_filter ||
+	    !stream->tasks) {
+		    goto fail;
+	}
+
+	stream->format = KS_INVALIDE_DATA;
+
+	return stream;
+
+ fail:
+	fprintf(stderr, "Failed to allocate memory for data stream.\n");
+	if (stream)
+		kshark_stream_free(stream);
+
+	return NULL;
+}
+
+/**
+ * @brief Add new Data stream.
+ *
+ * @param kshark_ctx: Input location for context pointer.
+ *
+ * @returns Zero on success or a negative errno code on failure.
+ */
+int kshark_add_stream(struct kshark_context *kshark_ctx)
+{
+	struct kshark_data_stream *stream;
+
+	if (kshark_ctx->n_streams == KS_MAX_NUM_STREAMS)
+		return -EMFILE;
+
+	stream = kshark_stream_alloc();
+	stream->stream_id = kshark_ctx->n_streams;
+
+	if (pthread_mutex_init(&stream->input_mutex, NULL) != 0) {
+		kshark_stream_free(stream);
+		return -EAGAIN;
+	}
+
+	kshark_ctx->stream[kshark_ctx->n_streams++] = stream;
+
+	return stream->stream_id;
+}
+
+/**
+ * @brief Get the Data stream object having given Id.
+ *
+ * @param kshark_ctx: Input location for context pointer.
+ * @param sd: Data stream identifier.
+ *
+ * @returns Pointer to a Data stream object if the sream exists. Otherwise
+ *	    NULL.
+ */
+struct kshark_data_stream *
+kshark_get_data_stream(struct kshark_context *kshark_ctx, int sd)
+{
+	if (sd >= 0 && sd < KS_MAX_NUM_STREAMS)
+		return kshark_ctx->stream[sd];
+
+	return NULL;
+}
+
+/**
+ * @brief Get the Data stream object corresponding to a given entry
+ *
+ * @param entry: Input location for the KernelShark entry.
+ *
+ * @returns Pointer to a Data stream object on success. Otherwise NULL.
+ */
+struct kshark_data_stream *
+kshark_get_stream_from_entry(const struct kshark_entry *entry)
+{
+	struct kshark_context *kshark_ctx = NULL;
+
+	if (!kshark_instance(&kshark_ctx))
+		return NULL;
+
+	return kshark_get_data_stream(kshark_ctx, entry->stream_id);
+}
+
+/**
+ * @brief Get an array containing the Ids of all opened Trace data streams.
+ * 	  The User is responsible for freeing the array.
+ *
+ * @param kshark_ctx: Input location for context pointer.
+ */
+int *kshark_all_streams(struct kshark_context *kshark_ctx)
+{
+	int *ids, i, count = 0;
+
+	ids = malloc(kshark_ctx->n_streams * (sizeof(*ids)));
+	if (!ids) {
+		fprintf(stderr,
+			"Failed to allocate memory for stream array.\n");
+		return NULL;
+	}
+
+	for (i = 0; i < KS_MAX_NUM_STREAMS; ++i)
+		if (kshark_ctx->stream[i])
+			ids[count++] = i;
+
+	return ids;
+}
+
 /**
  * @brief Close the trace data file and free the trace data handle.
  *
@@ -252,6 +399,56 @@  void kshark_free(struct kshark_context *kshark_ctx)
 	free(kshark_ctx);
 }
 
+/**
+ * @brief Get the name of the command/task from its Process Id.
+ *
+ * @param sd: Data stream identifier.
+ * @param pid: Process Id of the command/task.
+ */
+char *kshark_comm_from_pid(int sd, int pid)
+{
+	struct kshark_context *kshark_ctx = NULL;
+	struct kshark_data_stream *stream;
+	struct kshark_entry e;
+
+	if (!kshark_instance(&kshark_ctx))
+		return NULL;
+
+	stream = kshark_get_data_stream(kshark_ctx, sd);
+	if (!stream)
+		return NULL;
+
+	e.visible = KS_PLUGIN_UNTOUCHED_MASK;
+	e.pid = pid;
+
+	return stream->interface.get_task(stream, &e);
+}
+
+/**
+ * @brief Get the name of the event from its Id.
+ *
+ * @param sd: Data stream identifier.
+ * @param event_id: The unique Id of the event type.
+ */
+char *kshark_event_from_id(int sd, int event_id)
+{
+	struct kshark_context *kshark_ctx = NULL;
+	struct kshark_data_stream *stream;
+	struct kshark_entry e;
+
+	if (!kshark_instance(&kshark_ctx))
+		return NULL;
+
+	stream = kshark_get_data_stream(kshark_ctx, sd);
+	if (!stream)
+		return NULL;
+
+	e.visible = KS_PLUGIN_UNTOUCHED_MASK;
+	e.event_id = event_id;
+
+	return stream->interface.get_event_name(stream, &e);
+}
+
 static struct kshark_task_list *
 kshark_find_task(struct kshark_context *kshark_ctx, uint32_t key, int pid)
 {
diff --git a/src/libkshark.h b/src/libkshark.h
index fe0ba7f2..e299d067 100644
--- a/src/libkshark.h
+++ b/src/libkshark.h
@@ -340,6 +340,12 @@  struct kshark_task_list {
 
 /** Structure representing a kshark session. */
 struct kshark_context {
+	/** Array of data stream descriptors. */
+	struct kshark_data_stream	**stream;
+
+	/** The number of data streams. */
+	int				n_streams;
+
 	/** Input handle for the trace data file. */
 	struct tracecmd_input	*handle;
 
@@ -397,6 +403,16 @@  bool kshark_instance(struct kshark_context **kshark_ctx);
 
 bool kshark_open(struct kshark_context *kshark_ctx, const char *file);
 
+int kshark_add_stream(struct kshark_context *kshark_ctx);
+
+struct kshark_data_stream *
+kshark_get_data_stream(struct kshark_context *kshark_ctx, int sd);
+
+struct kshark_data_stream *
+kshark_get_stream_from_entry(const struct kshark_entry *entry);
+
+int *kshark_all_streams(struct kshark_context *kshark_ctx);
+
 ssize_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 				 struct kshark_entry ***data_rows);
 
@@ -416,6 +432,10 @@  void kshark_close(struct kshark_context *kshark_ctx);
 
 void kshark_free(struct kshark_context *kshark_ctx);
 
+char *kshark_comm_from_pid(int sd, int pid);
+
+char *kshark_event_from_id(int sd, int event_id);
+
 int kshark_get_pid_easy(struct kshark_entry *entry);
 
 const char *kshark_get_task_easy(struct kshark_entry *entry);
@@ -432,6 +452,157 @@  void kshark_convert_nano(uint64_t time, uint64_t *sec, uint64_t *usec);
 
 char* kshark_dump_entry(const struct kshark_entry *entry);
 
+static inline int kshark_get_pid(const struct kshark_entry *entry)
+{
+	struct kshark_data_stream *stream =
+		kshark_get_stream_from_entry(entry);
+
+	if (!stream)
+		return -1;
+
+	return stream->interface.get_pid(stream, entry);
+}
+
+static inline int kshark_get_event_id(const struct kshark_entry *entry)
+{
+	struct kshark_data_stream *stream =
+		kshark_get_stream_from_entry(entry);
+
+	if (!stream)
+		return -1;
+
+	return stream->interface.get_event_id(stream, entry);
+}
+
+static inline int *kshark_get_all_event_ids(struct kshark_data_stream *stream)
+{
+	return stream->interface.get_all_event_ids(stream);
+}
+
+static inline char *kshark_get_event_name(const struct kshark_entry *entry)
+{
+	struct kshark_data_stream *stream =
+		kshark_get_stream_from_entry(entry);
+
+	if (!stream)
+		return NULL;
+
+	return stream->interface.get_event_name(stream, entry);
+}
+
+static inline char *kshark_get_task(const struct kshark_entry *entry)
+{
+	struct kshark_data_stream *stream =
+		kshark_get_stream_from_entry(entry);
+
+	if (!stream)
+		return NULL;
+
+	return stream->interface.get_task(stream, entry);
+}
+
+static inline char *kshark_get_latency(const struct kshark_entry *entry)
+{
+	struct kshark_data_stream *stream =
+		kshark_get_stream_from_entry(entry);
+
+	if (!stream)
+		return NULL;
+
+	return stream->interface.get_latency(stream, entry);
+}
+
+static inline char *kshark_get_info(const struct kshark_entry *entry)
+{
+	struct kshark_data_stream *stream =
+		kshark_get_stream_from_entry(entry);
+
+	if (!stream)
+		return NULL;
+
+	return stream->interface.get_info(stream, entry);
+}
+
+static inline int kshark_read_event_field(const struct kshark_entry *entry,
+					  const char* field, int64_t *val)
+{
+	struct kshark_data_stream *stream =
+		kshark_get_stream_from_entry(entry);
+
+	if (!stream)
+		return -1;
+
+	return stream->interface.read_event_field_int64(stream, entry,
+							field, val);
+}
+
+/**
+ * @brief Load the content of the trace data file asociated with a given
+ *	  Data stream identifie into an array of kshark_entries.
+ *	  If one or more filters are set, the "visible" fields of each entry
+ *	  is updated according to the criteria provided by the filters. The
+ *	  field "filter_mask" of the session's context is used to control the
+ *	  level of visibility/invisibility of the filtered entries.
+ *
+ * @param kshark_ctx: Input location for context pointer.
+ * @param sd: Data stream identifier.
+ * @param data_rows: Output location for the trace data. The user is
+ *		     responsible for freeing the elements of the outputted
+ *		     array.
+ *
+ * @returns The size of the outputted data in the case of success, or a
+ *	    negative error code on failure.
+ */
+static inline ssize_t kshark_load_entries(struct kshark_context *kshark_ctx,
+					  int sd,
+					  struct kshark_entry ***data_rows)
+{
+	struct kshark_data_stream *stream;
+
+	stream = kshark_get_data_stream(kshark_ctx, sd);
+	if (!stream)
+		return -EBADF;
+
+	return stream->interface.load_entries(stream, kshark_ctx, data_rows);
+}
+
+/**
+ * @brief Load the content of the trace data file asociated with a given
+ *	  Data stream identifie into a data matrix. The user is responsible
+ *	  for freeing the outputted data.
+ *
+ * @param kshark_ctx: Input location for context pointer.
+ * @param sd: Data stream identifier.
+ * @param cpu_array: Output location for the CPU column (array) of the matrix.
+ * @param event_array: Output location for the Event Id column (array) of the
+ *		       matrix.
+ * @param _array: Output location for the  column (array) of the matrix.
+ * @param offset_array: Output location for the offset column (array) of the
+ *			matrix.
+ * @param ts_array: Output location for the time stamp column (array) of the
+ *		    matrix.
+ */
+static inline ssize_t kshark_load_matrix(struct kshark_context *kshark_ctx,
+					 int sd,
+					 int16_t **cpu_array,
+					 int32_t **pid_array,
+					 int32_t **event_array,
+					 int64_t **offset_array,
+					 int64_t **ts_array)
+{
+	struct kshark_data_stream *stream;
+
+	stream = kshark_get_data_stream(kshark_ctx, sd);
+	if (!stream)
+		return -EBADF;
+
+	return stream->interface.load_matrix(stream, kshark_ctx, cpu_array,
+								 pid_array,
+								 event_array,
+								 offset_array,
+								 ts_array);
+}
+
 /**
  * Custom entry info function type. To be user for dumping info for custom
  * KernelShark entryes.