mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Gilboa Davara <gilboad@gmail.com>
To: Paulo Marques <pmarques@grupopie.com>
Cc: Satyam Sharma <satyam@infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Reduce __print_symbol/sprint_symbol stack usage.
Date: Fri, 21 Sep 2007 16:57:59 +0200	[thread overview]
Message-ID: <1190386679.30016.67.camel@gilboa-home-dev.localdomain> (raw)
In-Reply-To: <46F3D351.2090506@grupopie.com>

On Fri, 2007-09-21 at 15:21 +0100, Paulo Marques wrote:
> Gilboa Davara wrote:
> > Hello Paulo,
> 
> Hi, Gilboa
> 
> Usually the developer can separate the output by hand in the unlikely 
> case of simultaneous concurrent users of printk, so I don't think this 
> is really a big problem.

In general I agree, but as the number of cores increases dramatically,
hitting a warning/BUG/oops simultaneously on multiple cores (especially
during development) becomes more of a problem.

At least to me, It's not rare that when working on 8C or above machine,
an oops message becomes completely unreadable due to multiple debug
printks that trash the serial port output. 

Never the less, I do agree that this problem is more developer related
and should not hit an end user running a debug-free distribution kernel.

> 
> >> Of course this requires changing _all_ callers of print_symbol to use 
> >> the new interface, but these are less than 100 ;)
> > 
> > This is my first contribution to the Linux kernel. As such I rather
> > start small, and work my way up slowly. (Read: solve the immediate stack
> > over-run now, think about changing the symbol_display interface later)
> 
> Yes, but this is a sensitive area, so you can not just implement 
> something now that can cause regressions, and just fix it later.

True.
My aim is to create a patch that
A. Doesn't break the current interface/APIs while keeping the
patch/noise to a minimum.
B. Does not cause any type of regression.

> 
> Kernel panics are quite rare and the information provided by the stack 
> dump is _extremely_ precious.

I fully agree.

> 
> Even more, risking deadlocks caused by something that should only be 
> used to produce debug information is even worse.

Hopefully the latest patch moves one step further in removing your
concerns.

> 
> >> Comments?
> > 
> > I do agree that the current interface needs work.
> > 
> > ... But as I said, I rather start slowly and on small scale. (Though I
> > did find a rather problematic place to start at... ;))
> 
> Well, if we agree that this is the way to go, then the way to start 
> slowly would be to submit a patch that makes both interfaces available 
> for a while and changes the most stack-critical callers to the new 
> interface.
> 
> Then slowly keep submitting patches to change other callers 
> progressively until there are no more callers of the old interface. At 
> that point we can just drop it entirely.

If I cannot find a working solution that does meet my initial goals
(Minimal patch/no regression) I'll start from scratch - creating a new
trace call path.

- Gilboa


  reply	other threads:[~2007-09-21 14:58 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-15 11:35 [Minor patch] " Gilboa Davara
2007-09-15 13:02 ` Satyam Sharma
2007-09-15 15:15   ` Gilboa Davara
2007-09-15 18:08     ` [PATCH] " Gilboa Davara
2007-09-19  1:00       ` Satyam Sharma
2007-09-19 14:25         ` Paulo Marques
2007-09-21 12:45           ` Gilboa Davara
2007-09-21 14:21             ` Paulo Marques
2007-09-21 14:57               ` Gilboa Davara [this message]
2007-09-21 14:56           ` Steven Rostedt
2007-09-21 15:47             ` Paulo Marques
2007-09-21 12:31         ` Gilboa Davara
2007-09-21 14:28       ` [PATCH] Reduce __print_symbol/sprint_symbol stack usage. (v3) Gilboa Davara
2007-09-21 16:02         ` Paulo Marques
2007-09-21 16:19           ` Gilboa Davara
2007-09-21 14:47 ` [Minor patch] Reduce __print_symbol/sprint_symbol stack usage Steven Rostedt
2007-09-21 14:53   ` Gilboa Davara

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=1190386679.30016.67.camel@gilboa-home-dev.localdomain \
    --to=gilboad@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmarques@grupopie.com \
    --cc=satyam@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®