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 05/20] kernel-shark: Add stream_id to kshark_entry
Date: Mon, 12 Oct 2020 20:05:44 -0400
Message-ID: <20201012200544.03d67163@oasis.local.home> (raw)
In-Reply-To: <20201012133523.469040-6-y.karadz@gmail.com>

On Mon, 12 Oct 2020 16:35:08 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> kshark_entry contains all information from one trace record, needed
> in order to visualize the time-series of trace records. Here we
> reorganize data fields to kshark_entry in order to make room for
> the unique identifier of the Data stream this entry belongs to.
> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  src/libkshark.c |  2 +-
>  src/libkshark.h | 12 +++++++++---
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libkshark.c b/src/libkshark.c
> index 52aacd30..92e2450c 100644
> --- a/src/libkshark.c
> +++ b/src/libkshark.c
> @@ -489,7 +489,7 @@ static inline void unset_event_filter_flag(struct kshark_context *kshark_ctx,
>  	e->visible &= ~event_mask;
>  }
>  
> -static void set_all_visible(uint16_t *v) {
> +static void set_all_visible(uint8_t *v) {
>  	/*  Keep the original value of the PLUGIN_UNTOUCHED bit flag. */
>  	*v |= 0xFF & ~KS_PLUGIN_UNTOUCHED_MASK;
>  }
> diff --git a/src/libkshark.h b/src/libkshark.h
> index 1165c512..fe0ba7f2 100644
> --- a/src/libkshark.h
> +++ b/src/libkshark.h
> @@ -1,7 +1,7 @@
>  /* SPDX-License-Identifier: LGPL-2.1 */
>  
>  /*
> - * Copyright (C) 2017 VMware Inc, Yordan Karadzhov <y.karadz@gmail.com>
> + * Copyright (C) 2017 VMware Inc, Yordan Karadzhov (VMware) <y.karadz@gmail.com>
>   */
>  
>   /**
> @@ -49,7 +49,10 @@ struct kshark_entry {
>  	 * kshark_filter_masks to check the level of visibility/invisibility
>  	 * of the entry.
>  	 */
> -	uint16_t	visible;
> +	uint8_t		visible;

Why did the visible variable turn from 16bits to 8 bits?

That should be stated in the change log.

> +
> +	/** Data stream identifier. */
> +	int8_t		stream_id;
>  
>  	/** The CPU core of the record. */
>  	int16_t		cpu;
> @@ -57,7 +60,7 @@ struct kshark_entry {
>  	/** The PID of the task the record was generated. */
>  	int32_t		pid;
>  
> -	/** Unique Id ot the trace event type. */
> +	/** Unique Id of the trace event type. */
>  	int32_t		event_id;
>  
>  	/** The offset into the trace file, used to find the record. */
> @@ -319,6 +322,9 @@ struct kshark_data_stream {
>  	struct kshark_data_stream_interface	interface;
>  };
>  
> +/** Hard-coded maximum number of data stream. */
> +#define KS_MAX_NUM_STREAMS	127

Will this become a variable?

Let's say we have a really powerful machine with a lot of memory. And
we have a data file with 500 CPUs traced, and we want to look at all of
them? Will this be a hard limit to change in the future without recompiling.

I've been hit by restrictions like this in code in the past, and know
that things like this can stay in the code for a long time, to a point
where they are no longer reasonable. I just want to make sure that we
don't get stuck with that. I've learned to look at all hard limits with
extra care, knowing that they are always a pain point in the future.

-- Steve


> +
>  /** Size of the task's hash table. */
>  #define KS_TASK_HASH_SHIFT 16
>  #define KS_TASK_HASH_SIZE (1 << KS_TASK_HASH_SHIFT)


  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 [this message]
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
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=20201012200544.03d67163@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