devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Frank Rowand <frowand.list@gmail.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Rob Herring <robh+dt@kernel.org>,
	Tom Zanussi <tom.zanussi@linux.intel.com>,
	Ingo Molnar <mingo@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [RFC PATCH 00/11] tracing: of: Boot time tracing using devicetree
Date: Mon, 15 Jul 2019 06:55:40 -0700	[thread overview]
Message-ID: <c01bd567-0c8b-ec32-773b-fb95bfcdcd50@gmail.com> (raw)
In-Reply-To: <20190625140004.a74443238596b297a558a66f@kernel.org>

Hi Masami,

Note that Masami has submitted v2 of the patch with further discussion.  I
noticed that I had left some comments unanswered in this thread, so I am
doing this reply before moving on to reply to the v2 thread.

Feel free to move the discussion to v2 instead of replying to this email
if you find that easier.

On 6/24/19 10:00 PM, Masami Hiramatsu wrote:
> Hi Frank,
> 
> On Mon, 24 Jun 2019 15:31:07 -0700
> Frank Rowand <frowand.list@gmail.com> wrote:
>>>>> Currently, kernel support boot-time tracing using kernel command-line
>>>>> parameters. But that is very limited because of limited expressions
>>>>> and limited length of command line. Recently, useful features like
>>>>> histogram, synthetic events, etc. are being added to ftrace, but it is
>>>>> clear that we can not expand command-line options to support these
>>>>> features.
>>>>
>>>> "it is clear that we can not expand command-line options" needs a fuller
>>>> explanation.  And maybe further exploration.
>>>
>>> Indeed. As an example of tracing settings in the first mail, even for simple
>>> use-case,  the trace command is long and complicated. I think it is hard to
>>> express that as 1-liner kernel command line. But devicetree looks very good
>>> for expressing structured data. That is great and I like it :)
>>
>> But you could extend the command line paradigm to meet your needs.
> 
> But the kernel command line is a command line. Would you mean encoding the 
> structured setting in binary format with base64 and pass it? :(

If you want to put structured data in the command line, then yes, you could use
an ascii safe form, such as base64, to contain it.  If that is what you want.


> 
>>>> Devicetree is NOT for configuration information.  This has been discussed
>>>> over and over again in mail lists, at various conferences, and was also an
>>>> entire session at plumbers a few years ago:
>>>>
>>>>    https://elinux.org/Device_tree_future#Linux_Plumbers_2016_Device_Tree_Track
>>>
>>> Thanks, I'll check that.
> 
> I found following discussion in etherpad log, https://elinux.org/Device_tree_plumbers_2016_etherpad
> ----
> If you have data that the kernel does not have a good way to get, that's OK to put into DT.
> 
>     Operating points are OK - but should still be structured well.
> ----
> 
> This sounds like if it is structured well, and there are no other way,
> we will be able to use DT as a channel.
> 
>>>>
>>>> There is one part of device tree that does allow non-hardware description,
>>>> which is the "chosen" node which is provided to allow communication between
>>>> the bootloader and the kernel.
>>>
>>> Ah, "chosen" will be suit for me :)
>>
>> No.  This is not communicating boot loader information.
> 
> Hmm, it's a kind of communication with the operator of the boot loader, since there
> is an admin or developer behind it. I think the comminication is to communicate
> with that human. Then if they intend to trace boot process, that is a kind of
> communication.

The quote from the plumbers 2016 devicetree etherpad ("Operating points are OK ...)
conveniently ignores a sentence just a few lines later:

   "If firmware is deciding the operating point, then it's OK to convey it via DT."

The operating points example is clearly communicating boot loader information to
the kernel.

Something the admin or developer wants to communicate is not boot loader
information.


> 
> [...]
>>>>> - Can we use devicetree for configuring kernel dynamically?
>>>>
>>>> No.  Sorry.
>>>>
>>>> My understanding of this proposal is that it is intended to better
>>>> support boot time kernel and driver debugging.  As an alternate
>>>> implementation, could you compile the ftrace configuration information
>>>> directly into a kernel data structure?  It seems like it would not be
>>>> very difficult to populate the data structure data via a few macros.
>>>
>>> No, that is not what I intended. My intention was to trace boot up
>>> process "without recompiling kernel", but with a structured data.
>>
>> That is debugging.  Or if you want to be pedantic, a complex performance
>> measurement of the boot process (more than holding a stopwatch in your
>> hand).
> 
> Yeah, that's right.
> 
>> Recompiling a single object file (containing the ftrace command data)
>> and re-linking the kernel is not a big price in that context).
> 
> No, if I can use DT, I can choose one of them while boot up.
> That will be a big difference.
> (Of course for that purpose, I should work on boot loader to support
> DT overlay)

This is debugging kernel drivers.  I do not think that it is a big cost for
a kernel developer to re-link the kernel.  On any reasonable modern
development system this should be a matter of seconds, not minutes.

Compiling a devicetree source is not significantly less time.  (To be
fair, you imply that you would have various compiled devicetrees to
choose from at boot time.  It may be realistic to have a library of
ftrace commands, or it may be more realistic that someone debugging
a kernel driver will create a unique ftrace command set for the
particular case they are debugging.)

> 
>>  Or if
>> you create a new communication channel, you will have the cost of
>> creating that data object (certainly not much different than compiling
>> a devicetree) and have the bootloader provide the ftrace data object
>> to the kernel.
> 
> Yes, and for me, that sounds like just a reinvention of the wheel.
> If I can reuse devicetree infrastructure, it is easily done (as I
> implemented in this series. It's just about 500LOC (and YAML document)

Or you can just use the existing ftrace boot command line syntax.


> 
> I can clone drivers/of/ code only for that new communication channel,
> but that makes no one happy. :(
> 
>>> For such purpose, we have to implement a tool to parse and pack the
>>> data and a channel to load it at earlier stage in bootloader. And
>>> those are already done by devicetree. Thus I thought I could get a
>>> piggyback on devicetree.
>>
>> Devicetree is not the universal dumping ground for communicating
>> information to a booting kernel.  Please create another communication
>> channel.
> 
> Why should we so limit the availability of even a small corner of existing
> open source software...?

Because the proposal is a mis-use of devicetree.

> 
> Thank you,
> 

  reply	other threads:[~2019-07-15 13:55 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-21 16:18 [RFC PATCH 00/11] tracing: of: Boot time tracing using devicetree Masami Hiramatsu
2019-06-21 16:18 ` [RFC PATCH 01/11] tracing: Apply soft-disabled and filter to tracepoints printk Masami Hiramatsu
2019-06-21 16:18 ` [RFC PATCH 02/11] tracing: kprobes: Output kprobe event to printk buffer Masami Hiramatsu
2019-06-21 16:18 ` [RFC PATCH 03/11] tracing: Expose EXPORT_SYMBOL_GPL symbol Masami Hiramatsu
2019-06-21 16:18 ` [RFC PATCH 04/11] tracing: kprobes: Register to dynevent earlier stage Masami Hiramatsu
2019-06-21 16:18 ` [RFC PATCH 05/11] tracing: Accept different type for synthetic event fields Masami Hiramatsu
2019-06-21 16:18 ` [RFC PATCH 06/11] tracing: Add NULL trace-array check in print_synth_event() Masami Hiramatsu
2019-06-21 16:19 ` [RFC PATCH 07/11] dt-bindings: tracing: Add ftrace binding document Masami Hiramatsu
2019-06-21 16:19 ` [RFC PATCH 08/11] tracing: of: Add setup tracing by devicetree support Masami Hiramatsu
2019-06-21 16:19 ` [RFC PATCH 09/11] tracing: of: Add trace event settings Masami Hiramatsu
2019-06-21 16:19 ` [RFC PATCH 10/11] tracing: of: Add kprobe event support Masami Hiramatsu
2019-06-21 16:19 ` [RFC PATCH 11/11] tracing: of: Add synthetic " Masami Hiramatsu
2019-06-23 19:58 ` [RFC PATCH 00/11] tracing: of: Boot time tracing using devicetree Frank Rowand
2019-06-24  2:52   ` Masami Hiramatsu
2019-06-24 22:31     ` Frank Rowand
2019-06-25  5:00       ` Masami Hiramatsu
2019-07-15 13:55         ` Frank Rowand [this message]
2019-07-17  0:57           ` Masami Hiramatsu
2019-06-26 21:58 ` Rob Herring
2019-06-27  2:55   ` Frank Rowand
2019-06-27 10:58   ` Masami Hiramatsu
2019-07-02  9:47     ` Masami Hiramatsu
2019-07-02 13:30       ` Rob Herring

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=c01bd567-0c8b-ec32-773b-fb95bfcdcd50@gmail.com \
    --to=frowand.list@gmail.com \
    --cc=acme@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tom.zanussi@linux.intel.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).