public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <srostedt@redhat.com>
To: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Ingo Molnar <mingo@elte.hu>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ring_buffer: enlarge RB_MAX_SMALL_DATA
Date: Wed, 15 Apr 2009 09:45:01 -0400	[thread overview]
Message-ID: <1239803101.21121.69.camel@localhost.localdomain> (raw)
In-Reply-To: <49E5A593.6090605@cn.fujitsu.com>


On Wed, 2009-04-15 at 17:14 +0800, Lai Jiangshan wrote:

> 
> 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.

I like the idea a lot. The only issue I have with it is that these types
are used a lot in switch statements, and in fast paths.

But the compression may be a better win.

We could do this in the switches.

	switch (event->type_len) {
	case RINGBUF_TYPE_TIME_STAMP:
		/**/
		break;
	case RINGBUF_TYPE_TIME_EXTEND:
		/**/
		break;
	case RINGBUF_TYPE_PADDING:
		/**/
		break;
	default: /* RINGBUF_TYPE_DATA */

	}

But before any of this goes in. I would like to export the interface.
Yes it is suppose to be OK to be able to change the interface, but the
binary output made it into 30. I doubt this change will make it before
31. If that's the case. We must have the format listed. 30 is expected
to be used by many distros. There's already tools using ftrace. If we
make a change that can not be caught, then it will break many tools.

We could ask Linus to pull this into 30 because it is a API change.

-- Steve



  parent reply	other threads:[~2009-04-15 13:45 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
2009-04-15 10:32     ` Ingo Molnar
2009-04-15 13:45     ` Steven Rostedt [this message]
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=1239803101.21121.69.camel@localhost.localdomain \
    --to=srostedt@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /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