All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Ken Chen <kenchen@google.com>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [patch] add /proc/pid/stack to dump task's stack trace
Date: Fri, 7 Nov 2008 09:32:49 +0100	[thread overview]
Message-ID: <20081107083249.GD4435@elte.hu> (raw)
In-Reply-To: <20081107082003.GA15800@x200.localdomain>


* Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Fri, Nov 07, 2008 at 08:59:25AM +0100, Ingo Molnar wrote:
> > 
> > * Ingo Molnar <mingo@elte.hu> wrote:
> > 
> > > > > + * buffer size used for proc read.  See proc_info_read().
> > > > > + * 4K page size but our output routines use some slack for overruns
> > > > > + */
> > > > > +#define PROC_BLOCK_SIZE	(3*1024)
> > > 
> > > That sounds like a proper limit - the hard limit for this particular 
> > > printout function is 4096-170, so we are well within the 
> > > PROC_BLOCK_SIZE range.
> > 
> > ok, i've added Ken's patch to tip/core/stacktrace and started testing 
> > it.
> > 
> > Alexey, i've added your Acked-by because you appeared to like the 
> > patch - let me know if i should remove it.
> 
> Of course, I don't like it!
> 
> Switch to seqfiles, add entry in TID table as well.
>
> The idea is good, though.

oh well - Ken, could you please switch it to seqfiles?

It should be something like this to convert the currently sweet and 
trivial function into a much more complex seqfile iterator splitup:

- the ::start method does the kmalloc of a loop state structure like 
  this:

  {
	struct stack_trace backtrace;
	unsigned long entries[MAX_STACK_TRACE_DEPTH];

	int iterator;
  }

  and saves the trace. (struct stack_trace trace can be on-stack, it's 
  only needed for the save_stack_trace() - and we keep the entries 
  after that.)

- the ::stop method does the kfree of the loop state.

- the ::show method prints a single line, based on ->entries[->iterator]

- the ::next method does ->iterator++, and returns NULL if iterator 
  reaches ->backtrace.nr_entries.

it will be more source code, larger kernel image, it will be more 
fragile and will be harder to review, and it wont actually matter in 
practice because 99.9999% of the backtraces we care about have a size 
smaller than 3K. (and where they get larger clipping them to the first 
3K is perfectly fine)

	Ingo

  reply	other threads:[~2008-11-07  8:33 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-06 19:01 [patch] add /proc/pid/stack to dump task's stack trace Ken Chen
2008-11-06 19:51 ` Alexey Dobriyan
2008-11-06 20:12   ` Ken Chen
2008-11-06 20:35     ` Ingo Molnar
2008-11-07  0:30       ` Ken Chen
2008-11-07  0:48         ` Alexey Dobriyan
2008-11-07  7:41           ` Ingo Molnar
2008-11-07  7:59             ` Ingo Molnar
2008-11-07  8:20               ` Alexey Dobriyan
2008-11-07  8:32                 ` Ingo Molnar [this message]
2008-11-07  8:49                   ` Alexey Dobriyan
2008-11-07  8:53                     ` Ingo Molnar
2008-11-07  9:03                       ` Ingo Molnar
2008-11-07  9:16                         ` Andrew Morton
2008-11-07  9:29                           ` Ingo Molnar
2008-11-07 17:51                             ` Alexey Dobriyan
2008-11-08 12:06                               ` Ingo Molnar
2008-11-10 23:54                             ` Andrew Morton
2008-11-11 10:00                               ` Ingo Molnar
2008-11-11 12:25                         ` Marcin Slusarz
2008-11-11 12:33                           ` Alexey Dobriyan
2008-11-11 13:20                           ` Ingo Molnar
2008-11-11 14:03                             ` Mikael Pettersson
2008-11-11 14:19                               ` Ingo Molnar
2008-11-07 18:38                   ` Ken Chen
2008-11-07 18:46                     ` Paul Menage
2008-11-08 12:10                     ` Ingo Molnar
2008-11-09 18:08                       ` Alexey Dobriyan
2008-11-10  8:41                         ` Ingo Molnar

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=20081107083249.GD4435@elte.hu \
    --to=mingo@elte.hu \
    --cc=a.p.zijlstra@chello.nl \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=kenchen@google.com \
    --cc=linux-kernel@vger.kernel.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
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.