On 12.11.20 г. 22:50 ч., Steven Rostedt wrote:
On Thu, 12 Nov 2020 16:23:42 +0200 "Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> wrote:With the help of Data stream, KernelShark will be able to load and merge multiple trace files (streams). Each stream can have different plugins or filters, registered for it, which means that the raw trace data of the streams can have different formats, and will allow for a great degree of customization of the provided data visualization. In this patch we only provide the basic definitions. The actual integration of the Data streams into the C API of KernelShark will happen in the following patches. Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@xxxxxxxxx> --- src/libkshark.h | 216 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 216 insertions(+) diff --git a/src/libkshark.h b/src/libkshark.h index cc20077f..c8fdd266 100644 --- a/src/libkshark.h +++ b/src/libkshark.h @@ -121,6 +121,222 @@ void kshark_hash_id_free(struct kshark_hash_id *hash);int *kshark_hash_ids(struct kshark_hash_id *hash); +struct kshark_data_stream;+ +/** A function type to be used to initialize the interface of the data stream. */ +typedef int (*interface_init_func) (struct kshark_data_stream *, + const char *); + +/** A function type to be used to initialize the interface of the data stream. */ +typedef int (*interface_close_func) (struct kshark_data_stream *, + const char *);These two typedefs should probably be introduced when a patch that uses them is added. Otherwise they are basically meaningless for the reviewer. Also, it the above looks like a cut-and-paste error as the close has the same description as the init. I don't even see the above used in later patches :-/
You are right. This is a dead code left over from previous versions of the way input plugins have been loaded.
+ +/** A function type to be used by the method interface of the data stream. */ +typedef char *(*stream_get_str_func) (struct kshark_data_stream *, + const struct kshark_entry *); + +/** A function type to be used by the method interface of the data stream. */ +typedef const int (*stream_get_int_func) (struct kshark_data_stream *, + const struct kshark_entry *); + +/** A function type to be used by the method interface of the data stream. */ +typedef int (*stream_find_id_func) (struct kshark_data_stream *, + const char *); + +/** A function type to be used by the method interface of the data stream. */ +typedef int *(*stream_get_ids_func) (struct kshark_data_stream *); + +/** A function type to be used by the method interface of the data stream. */ +typedef int (*stream_get_names_func) (struct kshark_data_stream *, + const struct kshark_entry *, + char ***); + +/** Event field format identifier. */ +typedef enum kshark_event_field_format { + /** A field of unknown type. */ + KS_INVALIDE_FIELD,KS_INVALID_FIELD ?
My idea is this identifier to be used for event fields having type that is not supported by the interface. For example strings or arrays.
+ + /** Integer number */ + KS_INTEGER_FIELD, + + /** Floating-point number */ + KS_FLOAT_FIELD +} kshark_event_field_format; + +/** A function type to be used by the method interface of the data stream. */ +typedef kshark_event_field_format +(*stream_event_field_type) (struct kshark_data_stream *, + const struct kshark_entry *, + const char *); + +/** A function type to be used by the method interface of the data stream. */ +typedef const int (*stream_read_event_field) (struct kshark_data_stream *, + const struct kshark_entry *, + const char *, + int64_t *); + +/** A function type to be used by the method interface of the data stream. */ +typedef const int (*stream_read_record_field) (struct kshark_data_stream *, + void *, + const char *, + int64_t *); + +struct kshark_context; + +/** A function type to be used by the method interface of the data stream. */ +typedef ssize_t (*load_entries_func) (struct kshark_data_stream *, + struct kshark_context *, + struct kshark_entry ***); + +/** A function type to be used by the method interface of the data stream. */ +typedef ssize_t (*load_matrix_func) (struct kshark_data_stream *, + struct kshark_context *, + int16_t **event_array, + int16_t **cpu_array, + int32_t **pid_array, + int64_t **offset_array, + int64_t **ts_array); + +/** Data format identifier. */ +typedef enum kshark_data_format { + /** A data of unknown type. */ + KS_INVALIDE_DATA,KS_INVALID_DATA ?
this identifier will be used in the case you are trying to open an arbitrary file that contains no tracing data (.jpeg for example).
+ + /** Ftrace data. */ + KS_TEP_DATA, + + /** VMware SchedTrace data. */ + KS_VMW_ST_DATA, +} kshark_data_format;I wonder if this should be a string value that gets registered? Otherwise we will have to be the registry of every new stream format added. This is the approach that Tzvetomir took for protocol ids, because that way we don't need to keep track of them: https://lore.kernel.org/r/20201029111816.247241-2-tz.stoyanov@xxxxxxxxx
I wonder how we can avoid name collisions, especially in the case when some of the data readout plugins will be proprietary. Also I don't think we can expect more than a dozen of distinct data formats to be supported.
Thanks! Yordan
-- Steve+ +/** Data interface identifier. */ +typedef enum kshark_data_interface_id { + /** An interface with unknown type. */ + KS_INVALIDE_INTERFACE, + + /** Generic interface suitable for Ftrace data. */ + KS_GENERIC_DATA_INTERFACE, +} kshark_data_interface_id; +
|