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: Thu, 29 Oct 2020 10:04:21 -0400
Message-ID: <20201029100421.33720af1@gandalf.local.home> (raw)
In-Reply-To: <d67dd724-abe1-3696-29f7-75358f98df87@gmail.com>

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

  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
2020-10-29 10:10     ` Yordan Karadzhov (VMware)
2020-10-29 14:04       ` Steven Rostedt [this message]
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=20201029100421.33720af1@gandalf.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