Message-ID: <4F9A347C.5040308@hitachi.com>
Date:	Fri, 27 Apr 2012 14:54:04 +0900
From:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To:	Jason Wessel <jason.wessel@...driver.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...hat.com,
	rusty@...tcorp.com.au, Steven Rostedt <rostedt@...dmis.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	yrl.pp-manager.tt@...achi.com
Subject: Re: [RFC PATCH 0/8] backtrace/oops with source/line information

(2012/04/23 21:04), Jason Wessel wrote:
> On 04/23/2012 06:44 AM, Masami Hiramatsu wrote:
>> (2012/04/21 6:29), Jason Wessel wrote:
>>> Call to panic() with the patch set
>>> ----------------------------------
>>> Call Trace:
>>> [<ffffffff815f3003>] panic+0xbd/0x14 panic.c:111
>>> [<ffffffff815f31f4>] ? printk+0x68/0xd printk.c:765
>>> [<ffffffffa0000175>] panic_write+0x25/0x30 [test_panic] test_panic.c:189
>>> [<ffffffff8118aa96>] proc_file_write+0x76/0x21 generic.c:226
>>> [<ffffffff8118aa20>] ? __proc_create+0x130/0x21 generic.c:211
>>> [<ffffffff81185678>] proc_reg_write+0x88/0x21 inode.c:218
>>> [<ffffffff81125718>] vfs_write+0xc8/0x20 read_write.c:435
>>> [<ffffffff811258d1>] sys_write+0x51/0x19 read_write.c:457
>>> [<ffffffff815f84d9>] ia32_do_call+0x13/0xc ia32entry.S:427
>>
>>
>> This can be also useful for kprobes, users can put new events on the
>> specified lines not address! Ofcourse, I'd like to use this interface
>> for the disassembler patch too. ;)
> 
> The eventual goal would be to use this for kdb and kprobes as well.
> Really it is nothing more than a few more functions to access the data
> tables. I thought the first logical step would be to use this in
> backtrace and oops because that affects the widest scope of users of
> this feature.

Agreed, the oops message is the most important one which has to be
done.


> 
>>
>>> The key difference here is that you get the source line information in
>>> any kind of oops/panic/backtrace, including inside kernel modules. Of
>>> course this comes at the expense of some memory you do not get to use
>>> because these tables have to get stored in a place that is accessible
>>> after a crash. On a system with 4 gigs of ram however, the cost is
>>> nearly insignificant when you have to give up a few megs for the
>>> capability. The idea is to make it a bit easier to just jump into a
>>> source tree without having to use any tools to decode the dump (which
>>> I know every kernel developer out there already has anwyay :-)
>>
>> By the way, it seems that source file doesn't include directries,
>> is that possible to expand it? Since there are many files which have
>> same name but in different directries, it would be better to
>> print those path informations too.
> 
> 
> For now I didn't want to increase the kernel size any further, and it
> was not directly needed by the backtrace due to screen size
> constraints. It will not be hard to add the directories into the
> database tables. At the moment, the readelf code is just trowing the
> information away.

Ah, I see. I've thought that your patch directly includes
.debug_lines section.

> I also had noticed from my random sampling of functions / line
> locations that you end up with unique values if you use the function
> name + file name + line number. Obviously that is not the same thing
> as simply knowing the line number and file location, but if the
> memory footprint becomes too great a problem this is a possible
> compromise.

OK, BTW, how much bigger the kernel with line location info now?

>>> Using this patch is fairly straight forward. From a developer / end
>>> user perspective here are the steps:
>>> * set CONFIG_KALLSYMS_LINE_LOCATIONS=y
>>
>> I've found this config under Kallsyms resources, but I think
>> it is better to put it under CONFIG_DEBUG_INFO, because you
>> may need to enable CONFIG_DEBUG_INFO for build kernel with
>> .debug_* stuffs, and that will increase on-memory kernel
>> footprint.
> 
> At the moment the line locations feature is tied to the kallsyms code.
> I thought I had made it depend on the debug info, but it depends only
> on DEBUG_KERNEL at the moment so that will need to get fixed.
> 
>>>
>>> Finally the symbol lookup used by printk was changed to request the
>>> lines for location data when it exists.
>>
>> Ah, that's really what I want! :)
>> I think ftrace also can use this for pretty printing ip address.
>>
> 
> Because the address printing was generically changed, if the ftrace ip
> address print is using a sym_address lookup it should automatically
> appear.

OK, I'll try to use it. (looks easy to use:))

>>> If you are wondering why I pulled in an modified readelf? It is
>>> because most of the pre-built versions of readelf I tried were not
>>> working properly on 32 or 64 bit combinations of machines.
>>
>> Maybe you can report it to the readelf upstream.
> 
> It is entirely possible this has been fixed a long time ago. I used
> readelf 2.14 for all the work for the kernel because I needed a
> version with a GPL v2 license. All I had to go on was just trying the
> pre-built binaries from binutils from several host OS systems and
> cross compilers I had laying around. It was enough to tell me that we
> would need our own readelf in the tree if we wanted this feature to
> work reliably.

Hmm, I see.

>>> git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git
>> lines_for_locations
>>
>> BTW, correct git url is here, isn't it? :)
>> git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git
>>
> 
> 
> Thanks for pointing this out. It will also be corrected should this
> series get posted again. :-)
> 
> Cheers,
> Jason.

Thanks!

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@...achi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists