Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: "Yordan Karadzhov (VMware)" <y.karadz@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH 08/15] kernel-shark: Integrate the stream definitions with the C API
Date: Thu, 8 Oct 2020 10:17:50 +0300
Message-ID: <cf2cab03-6797-52d6-e866-5acf8cd99ee6@gmail.com> (raw)
In-Reply-To: <20201007161226.6e1da22e@gandalf.local.home>

Hi Steven,

On 7.10.20 г. 23:12 ч., Steven Rostedt wrote:
> 
> I didn't apply the first patch, and nothing broke up until this patch.
> 
> On Tue, 29 Sep 2020 16:41:16 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> Unfortunately this integration can only happen in a single change,
>> which results into one huge patch, that may be hard to review. In
>> this patch we modify the API itself plus the two most basic examples
>> demonstrating how to use it. The most noticeable change that this
>> new version of the API brings is that the processing of the
>> trace-cmd/Ftrace data is wrapped inside the dedicated implementation
>> of the Data stream interface and no trace-cmd definitions are leaking
>> outside of src/libkshark-tepdata.c.
> 
> I find it hard to believe that we can't break this up more.
> 
> Can you tell me what the old API did and what the new API does. I can
> probably help out in taking steps to go from one to the other. But I don't
> understand what exactly is changing without spending several days staring
> at this code.

The essence of the change is that in the API we are not allowed to 
include any trace-cmd header files. And because we have been using 
trace-cmd definitions and methods everywhere, once we remove the header 
inclusion everything breaks. The only place that is allowed to include 
trace-cmd stuff is the implementation of the readout interface for 
trace-cmd data (src/libkshark-tepdata.c).

I understand that the approach I am taking with the introduction of the 
regression is against all rules and is a BIG compromise. This is the 
reason why I wanted to keep at least the two very basic example 
applications functional during the change. Of course we can further 
break this huge and ugly change into smaller changes that have better 
logical connection, but in such a case it will be very very hard to keep 
the examples functional during those changes.

Thanks!
Yordan

> 
> -- Steve
> 

  reply index

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-29 13:41 [PATCH 00/15] Start KernelShark v2 transformation Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 01/15] kernel-shark: split kernel-shark from trace-cmd repo Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 02/15] kernel-shark: Version 1.2.0 Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 03/15] kernel-shark: Start introducing KernelShark 2.0 Yordan Karadzhov (VMware)
2020-10-07 20:08   ` Steven Rostedt
2020-09-29 13:41 ` [PATCH 04/15] kernel-shark: Use only signed types in kshark_entry Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 05/15] kernel-shark: Introduce libkshark-hash Yordan Karadzhov (VMware)
2020-10-06 21:02   ` Steven Rostedt
2020-09-29 13:41 ` [PATCH 06/15] kernel-shark: Introduce Data streams Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 07/15] kernel-shark: Add stream_id to kshark_entry Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 08/15] kernel-shark: Integrate the stream definitions with the C API Yordan Karadzhov (VMware)
2020-10-07 20:12   ` Steven Rostedt
2020-10-08  7:17     ` Yordan Karadzhov (VMware) [this message]
2020-10-07 20:29   ` Steven Rostedt
2020-09-29 13:41 ` [PATCH 09/15] kernel-shark: Provide merging of multiple data streams Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 10/15] kernel-shark: Integrate the stream definitions with data model Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 11/15] kernel-shark: Use only signed types for model defs Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 12/15] kernel-shark: Add ksmodel_get_bin() Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 13/15] kernel-shark: Protect ksmodel_set_in_range_bining() Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 14/15] kernel-shark: Add methods for time calibration Yordan Karadzhov (VMware)
2020-09-29 13:41 ` [PATCH 15/15] kernel-shark: Integrate streams with libkshark-configio Yordan Karadzhov (VMware)

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=cf2cab03-6797-52d6-e866-5acf8cd99ee6@gmail.com \
    --to=y.karadz@gmail.com \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /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