All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lai Jiangshan <laijs@cn.fujitsu.com>
To: Steven Rostedt <srostedt@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH] ring_buffer: enlarge RB_MAX_SMALL_DATA
Date: Wed, 15 Apr 2009 17:14:59 +0800	[thread overview]
Message-ID: <49E5A593.6090605@cn.fujitsu.com> (raw)
In-Reply-To: <1239633781.21121.25.camel@localhost.localdomain>

Steven Rostedt wrote:
> On Mon, 2009-04-13 at 11:00 +0800, Lai Jiangshan wrote:
>> When I am writing userspace tools for ftrace, I found
>> RB_MAX_SMALL_DATA is too small, some events waste an 'u32'
>> to save the actually length.
> 
> Although I like the idea, I want to look at something else.
> 
> 2^27 is: 134,217,728
> 2^26 is: 67,108,864
> 
> That is the count in nanoseconds. Thus we go from 134 millisecs to 67
> millisecs before we must add an extended counter.
> 
> I guess that is not an issue, since 67ms is still quite big. For sparse
> tracing, it could add more extended counters where none were needed. But
> this I doubt this is an issue.

67ms < 0.1sec, It sounds not very good.

> 
>> This fix will break previous userspace tools,
>> so complaints are also welcome.
> 
> Unfortunately, this changes the API to userspace. For those parsers that
> do this in binary. I think the answer is, before we add this, we export
> the format of the ring buffer headers just like we do for other formats.
> This way, a user tool can default to the old way if the format file does
> not exist, and can know the current format with the file.
> 
> I'll work on adding that format file sometime this week.
>

I think ftrace is still in develop-circle, changing its API to userspace
is sometimes OK. Will you agreed this fix after you add that format file?
It saves about 0%-12%(depends on tracer) memory.

How about this method:

{u32 type_len:5, time_delta:27;};

time_delta is still 27bits, but RB_MAX_SMALL_DATA is increased to 112,
it is larger than 60 in my patch.

type_len  actual type                actual len
0         RINGBUF_TYPE_DATA,         array[0]
1 ~ 28    RINGBUF_TYPE_DATA          4*1 ~ 4*28
29        RINGBUF_TYPE_TIME_STAMP    
30        RINGBUF_TYPE_TIME_EXTEND
31        RINGBUF_TYPE_PADDING       array[0] if it's not the last event
                                     in the buffer_page(discarded event)
If you like this method, I will implement it.

Lai.


  reply	other threads:[~2009-04-15  9:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-13  3:00 [PATCH] ring_buffer: enlarge RB_MAX_SMALL_DATA Lai Jiangshan
2009-04-13 14:43 ` Steven Rostedt
2009-04-15  9:14   ` Lai Jiangshan [this message]
2009-04-15 10:32     ` Ingo Molnar
2009-04-15 13:45     ` Steven Rostedt
2009-04-15 14:12       ` Ingo Molnar
2009-04-15 14:23         ` 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=49E5A593.6090605@cn.fujitsu.com \
    --to=laijs@cn.fujitsu.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.org \
    --cc=srostedt@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.