linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Ying Han <yinghan@google.com>
To: Andi Kleen <andi@firstfloor.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Minchan Kim <minchan.kim@gmail.com>,
	Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>,
	Balbir Singh <balbir@linux.vnet.ibm.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH] Eliminate task stack trace duplication.
Date: Tue, 3 May 2011 13:09:02 -0700	[thread overview]
Message-ID: <BANLkTi=C9qpkfM1fjeCD_Z_-2rYUifiaUg@mail.gmail.com> (raw)
In-Reply-To: <m2iptref78.fsf@firstfloor.org>

On Tue, May 3, 2011 at 12:50 PM, Andi Kleen <andi@firstfloor.org> wrote:
> Ying Han <yinghan@google.com> writes:
>
>> The problem with small dmesg ring buffer like 512k is that only limited number
>> of task traces will be logged. Sometimes we lose important information only
>> because of too many duplicated stack traces.
>>
>> This patch tries to reduce the duplication of task stack trace in the dump
>> message by hashing the task stack. The hashtable is a 32k pre-allocated buffer
>> during bootup.
>
> Nice idea!  This makes it a lot more readable too.
>
> Can we compress the register values too? (e.g. by not printing that many
> 0s and replacing ffff8 with <k> or so)
>
> In fact I don't remember needing the register values for anything.
> Maybe they could be just not printed by default?

I can look into that, but i might not have time working on this patch
soon. If this is something
nice to have, maybe we can consider merge the first part in and then
add on top of it?

>
>>  #endif
>>       read_lock(&tasklist_lock);
>> +
>> +     spin_lock(&stack_hash_lock);
>
> The long hold lock scares me a little bit for a unstable system.
> Could you only hold it while hashing/unhashing?

The patch was initially developed on 2.6.26 kernel and I forward
ported to the latest kernel this time. We've been running the 2.6.26
kernel with the patch for quite long time, and haven't noticed the
problem w/ the locking.

>
> Also when you can't get it fall back to something else.

Can you clarify that?

Thank you for reviewing :)

--Ying
>
> -Andi
> --
> ak@linux.intel.com -- Speaking for myself only
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-05-03 20:09 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-03 17:35 [PATCH] Eliminate task stack trace duplication Ying Han
2011-05-03 18:23 ` Rik van Riel
2011-05-03 20:10   ` Ying Han
2011-05-03 19:50 ` Andi Kleen
2011-05-03 20:09   ` Ying Han [this message]
2011-05-03 21:10     ` Andi Kleen

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='BANLkTi=C9qpkfM1fjeCD_Z_-2rYUifiaUg@mail.gmail.com' \
    --to=yinghan@google.com \
    --cc=andi@firstfloor.org \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=minchan.kim@gmail.com \
    --cc=nishimura@mxp.nes.nec.co.jp \
    /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;
as well as URLs for NNTP newsgroup(s).