Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: "Yordan Karadzhov (VMware)" <y.karadz@gmail.com>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH v2 07/20] kernel-shark: Add basic methods for Data streams
Date: Mon, 12 Oct 2020 20:18:47 -0400
Message-ID: <20201012201847.1218c974@oasis.local.home> (raw)
In-Reply-To: <20201012133523.469040-8-y.karadz@gmail.com>

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.


  reply index

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-12 13:35 [PATCH v2 00/20] Start KernelShark v2 transformation Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 01/20] kernel-shark: Start introducing KernelShark 2.0 Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 02/20] kernel-shark: Use only signed types in kshark_entry Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 03/20] kernel-shark: Introduce libkshark-hash Yordan Karadzhov (VMware)
2020-10-12 14:05   ` Steven Rostedt
2020-10-12 14:05     ` Steven Rostedt
2020-10-12 14:18       ` Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 04/20] kernel-shark: Introduce Data streams Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 05/20] kernel-shark: Add stream_id to kshark_entry Yordan Karadzhov (VMware)
2020-10-13  0:05   ` Steven Rostedt
2020-10-29 10:08     ` Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 06/20] kernel-shark: Rename static methods in libkshark Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 07/20] kernel-shark: Add basic methods for Data streams Yordan Karadzhov (VMware)
2020-10-13  0:18   ` Steven Rostedt [this message]
2020-10-29 10:10     ` Yordan Karadzhov (VMware)
2020-10-29 14:04       ` Steven Rostedt
2020-10-29 14:49         ` Yordan Karadzhov (VMware)
2020-10-30  1:57           ` Steven Rostedt
2020-11-03 13:38             ` Yordan Karadzhov (VMware)
2020-11-04 15:41               ` Steven Rostedt
2020-11-05 14:35                 ` Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 08/20] kernel-shark: Housekeeping before implementing stream interface Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 09/20] kernel-shark: Add stream interface for trace-cmd data Yordan Karadzhov (VMware)
2020-10-13  0:44   ` Steven Rostedt
2020-10-29 11:16     ` Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 10/20] kernel-shark: Start using data streams Yordan Karadzhov (VMware)
2020-10-14 18:56   ` Steven Rostedt
2020-11-05 14:58     ` Yordan Karadzhov (VMware)
2020-11-05 18:17       ` Steven Rostedt
2020-11-06 14:31         ` Yordan Karadzhov (VMware)
2020-11-06 15:18           ` Steven Rostedt
2020-11-09 14:49             ` Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 11/20] kernel-shark: Remove dead code Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 12/20] kernel-shark: Redesign the plugin interface Yordan Karadzhov (VMware)
2020-10-14 21:09   ` Steven Rostedt
2020-10-12 13:35 ` [PATCH v2 13/20] kernel-shark: Complete the stream integration Yordan Karadzhov (VMware)
2020-10-14 23:52   ` Steven Rostedt
2020-10-12 13:35 ` [PATCH v2 14/20] kernel-shark: Provide merging of multiple data streams Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 15/20] kernel-shark: Integrate the stream definitions with data model Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 16/20] kernel-shark: Use only signed types for model defs Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 17/20] kernel-shark: Add ksmodel_get_bin() Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 18/20] kernel-shark: Protect ksmodel_set_in_range_bining() Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 19/20] kernel-shark: Add methods for time calibration Yordan Karadzhov (VMware)
2020-10-12 13:35 ` [PATCH v2 20/20] kernel-shark: Integrate streams with libkshark-configio Yordan Karadzhov (VMware)
2020-11-05 19:22   ` Steven Rostedt
2020-11-09 14:55     ` Yordan Karadzhov (VMware)
2020-11-09 15:28       ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201012201847.1218c974@oasis.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=y.karadz@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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

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

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

Example config snippet for mirrors

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


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