public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Vaibhav Nagarnaik <vnagarnaik@google.com>,
	Ingo Molnar <mingo@redhat.com>, Michael Rubin <mrubin@google.com>,
	David Sharp <dhsharp@google.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 03/15] tracing: Add a free on close control mechanism for buffer_size_kb
Date: Mon, 13 Jun 2011 13:49:04 +0200	[thread overview]
Message-ID: <20110613114904.GD10960@elte.hu> (raw)
In-Reply-To: <1307965185.9218.53.camel@gandalf.stny.rr.com>


* Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 2011-06-13 at 12:12 +0200, Ingo Molnar wrote:
> > * Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > +	switch (cmd) {
> > > +	case TRACE_RINGBUF_FREE_ON_CLOSE: {
> > > +		info->free_buffer_on_close = !!arg;
> > > +		ret = 0;
> > > +		break;
> > > +	}
> > >  	}
> > 
> > that doesn't look very tidy.
> > 
> > > @@ -3635,9 +3700,12 @@ static const struct file_operations tracing_pipe_fops = {
> > >  };
> > >  
> > >  static const struct file_operations tracing_entries_fops = {
> > > -	.open		= tracing_open_generic,
> > > +	.open		= tracing_entries_open,
> > >  	.read		= tracing_entries_read,
> > >  	.write		= tracing_entries_write,
> > > +	.unlocked_ioctl	= tracing_entries_ioctl,
> > > +	.compat_ioctl	= tracing_entries_ioctl,
> > 
> > i don't like it at all that you are adding to the ftrace ABI here. 
> > The *only* premise of the whole /debug/tracing/ muck was to allow it 
> > to be human-parseable and scripted - an ioctl is clearly outside that 
> > scope. Instead of increasing the mess in /debug/tracing/ we want 
> > clean tracing done via the perf ABI ...
> 
> Vaibhav originally suggested adding a "buffer_free" file that you 
> could write into and cause it to free the buffer. It would do this 
> on the release so you could also have an app (like Google needs) to 
> open this file and if the app dies, it will automatically free the 
> buffer closing it.
> 
> https://lkml.org/lkml/2011/3/17/366
> 
> I didn't really like adding another file to the debugfs system, and 
> recommended the ioctl. It seemed like a nice "unix" fit. But if you 
> want to go back to the Vaibhav's original method, which will stay 
> in the frame of "human-parseable and scripted". We could do that.

Yes, i think so - the *only* point of the /debug/tracing/ muck is 
that it's scriptable and human parseable. If Google wants to use it 
for more than that then they should help us enhance the perf syscall 
ABI for tracing ...

Thanks,

	Ingo

  reply	other threads:[~2011-06-13 11:49 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-09 17:27 [PATCH 00/15] [GIT PULL] tracing: various updates Steven Rostedt
2011-06-09 17:27 ` [PATCH 01/15] tracing: Schedule a delayed work to call wakeup() Steven Rostedt
2011-06-13 10:07   ` Ingo Molnar
2011-06-13 10:27     ` Pekka Enberg
2011-06-13 11:45       ` Steven Rostedt
2011-06-09 17:27 ` [PATCH 02/15] tracing: Use NUMA allocation for per-cpu ring buffer pages Steven Rostedt
2011-06-13 10:09   ` Ingo Molnar
2011-06-13 11:28     ` Steven Rostedt
2011-06-09 17:27 ` [PATCH 03/15] tracing: Add a free on close control mechanism for buffer_size_kb Steven Rostedt
2011-06-13 10:12   ` Ingo Molnar
2011-06-13 11:39     ` Steven Rostedt
2011-06-13 11:49       ` Ingo Molnar [this message]
2011-06-13 11:54         ` Steven Rostedt
2011-06-13 19:12           ` Vaibhav Nagarnaik
2011-06-13 20:01           ` Vaibhav Nagarnaik
2011-06-14  0:37             ` Steven Rostedt
2011-06-14  0:43               ` Vaibhav Nagarnaik
2011-06-09 17:27 ` [PATCH 04/15] ftrace: Fixed an include coding style issue Steven Rostedt
2011-06-09 17:27 ` [PATCH 05/15] async: " Steven Rostedt
2011-06-09 17:27 ` [PATCH 06/15] tracing, function_graph: Remove dependency of abstime and duration Steven Rostedt
2011-06-09 17:27 ` [PATCH 07/15] tracing, function_graph: Merge overhead and duration display Steven Rostedt
2011-06-09 17:27 ` [PATCH 08/15] tracing, function: Fix trace header to follow context-info option Steven Rostedt
2011-06-09 17:27 ` [PATCH 09/15] tracing, function_graph: Remove lock-depth from latency trace Steven Rostedt
2011-06-09 17:27 ` [PATCH 10/15] tracing, function_graph: Add context-info support for function_graph Steven Rostedt
2011-06-09 17:27 ` [PATCH 11/15] tracing: Convert to kstrtoul_from_user Steven Rostedt
2011-06-09 17:27 ` [PATCH 12/15] ring-buffer: Set __GFP_NORETRY flag for ring buffer allocating Steven Rostedt
2011-06-09 17:27 ` [PATCH 13/15] x86: Swap save_stack_trace_regs parameters Steven Rostedt
2011-06-13 10:18   ` Ingo Molnar
2011-06-13 11:43     ` Steven Rostedt
2011-06-09 17:27 ` [PATCH 14/15] stack_trace: Add weak save_stack_trace_regs() Steven Rostedt
2011-06-13 10:19   ` Ingo Molnar
2011-06-13 10:52     ` Masami Hiramatsu
2011-06-13 11:42       ` Ingo Molnar
2011-06-09 17:27 ` [PATCH 15/15] tracing/kprobes: Fix kprobe-tracer to support stack trace Steven Rostedt
2011-06-13 10:21   ` Ingo Molnar
2011-06-13 11:44     ` Steven Rostedt
2011-06-13 11:50       ` Ingo Molnar
2011-06-13 12:14         ` Steven Rostedt
2011-06-14  1:25           ` Masami Hiramatsu
2011-06-14  2:08             ` Steven Rostedt
2011-06-14 11:22               ` Peter Zijlstra

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=20110613114904.GD10960@elte.hu \
    --to=mingo@elte.hu \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=dhsharp@google.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mrubin@google.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=vnagarnaik@google.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