On 13.10.20 г. 3:05 ч., Steven Rostedt wrote:
On Mon, 12 Oct 2020 16:35:08 +0300 "Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> 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@xxxxxxxxx> --- 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@xxxxxxxxx> + * Copyright (C) 2017 VMware Inc, Yordan Karadzhov (VMware) <y.karadz@xxxxxxxxx> *//**@@ -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.
After further discussing this on a call with Steven we decided to keep the "visible" field 16 bit and to define the "stream_id" to be a 16 bit integer as well. Those 16 bit will be taken from the event_id. This is OK because the Id of the trace event is coded with only 16 bit in the kernel anyway.
+ + /** 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 127Will 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.
Now when we have 16 bit available for coding the stream Id, we no longer need to enforce such a small limit for the total number of streams loaded.
Thanks Steven! Yordan
-- Steve+ /** Size of the task's hash table. */ #define KS_TASK_HASH_SHIFT 16 #define KS_TASK_HASH_SIZE (1 << KS_TASK_HASH_SHIFT)
|