Re: [PATCH] markup_oops.pl: add options to improve cross-sompilation environments
Américo Wang
> Sorry guys, the prev mail still have some format trouble. I send a new mail
> for it.
>
> The markup_oops.pl have 3 troubles to support cross-compiler environment:
> 1. It use objdump directly.
> 2. It use modinfo to get the message of module.
> 3. It use hex function that cannot support 64-bit number in 32-bit arch.
>
> This patch add 3 options to markup_oops.pl:
> 1. -c CROSS_COMPILE Specify the prefix used for toolchain.
> 2. -m MODULE_DIRNAME Specify the module directory name.
> 3. Change hex function to Math::BigInt->from_hex.
>
> After this patch, parse the x8664 oops in x86, we can:
> cat amd64m | perl ~/kernel/tmp/m.pl -c /home/teawater/kernel/bin/x8664- -m
> ./e.ko vmlinux
>
> Thanks,
> Hui
>
> Signed-off-by: Hui Zhu <teaw...@gmail.com>
> Cc: Andrew Morton <ak...@linux-foundation.org>
> Cc: Arjan van de Ven <ar...@linux.intel.com>
> Cc: Sam Ravnborg <s...@ravnborg.org>
> Cc: oz...@pardus.org.tr
> Cc: Matthew Wilcox <wi...@linux.intel.com>
>
> ---
> scripts/markup_oops.pl | 71
> ++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 61 insertions(+), 10 deletions(-)
>
> --- a/scripts/markup_oops.pl
> +++ b/scripts/markup_oops.pl
> @@ -15,8 +15,46 @@ use Math::BigInt;
> # Arjan van de Ven <ar...@linux.intel.com>
>
>
> -my $vmlinux_name = $ARGV[0];
> -if (!defined($vmlinux_name)) {
> +my $cross_compile = "";
> +my $vmlinux_name = "";
> +my $modulefile = "";
> +
> +# Get options
> +my $option = 0;
> +for (my $i = 0; $i <= $#ARGV; $i++) {
> + if ($option == 0) {
> + if ($ARGV[$i] eq "-c") {
> + $option = 1;
> + }
> + elsif ($ARGV[$i] eq "-m") {
> + $option = 2;
> + }
> + elsif ($ARGV[$i] eq "-h") {
> + usage();
> + exit;
> + }
> + elsif ($i == $#ARGV) {
> + $vmlinux_name = $ARGV[$i];
> + }
> + else {
> + usage();
> + exit;
> + }
> + }
> + elsif ($option == 1) {
> + $cross_compile = $ARGV[$i];
> + $option = 0;
> + }
> + elsif ($option == 2) {
> + $modulefile = $ARGV[$i];
> + $option = 0;
> + }
> +}
> +
> +if ($vmlinux_name ne "") {
> + $vmlinux_name = $ARGV[$#ARGV];
> +}
Why not using the Perl module 'Getopt' to do this?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Hui Zhu
> On Tue, Jan 26, 2010 at 11:11 AM, Hui Zhu <hui...@windriver.com> wrote:
>> Sorry guys, the prev mail still have some format trouble. I send a new mail
>> for it.
>> +
>> +if ($vmlinux_name ne "") {
>> + $vmlinux_name = $ARGV[$#ARGV];
>> +}
>
> Why not using the Perl module 'Getopt' to do this?
>
Thanks for remind me about it. The following patch use Getopt.
The markup_oops.pl have 3 troubles to support cross-compiler environment:
1. It use objdump directly.
2. It use modinfo to get the message of module.
3. It use hex function that cannot support 64-bit number in 32-bit arch.
This patch add 3 options to markup_oops.pl:
1. -c CROSS_COMPILE Specify the prefix used for toolchain.
2. -m MODULE_DIRNAME Specify the module directory name.
3. Change hex function to Math::BigInt->from_hex.
After this patch, parse the x8664 oops in x86, we can:
cat amd64m | perl ~/kernel/tmp/m.pl -c /home/teawater/kernel/bin/x8664- -m ./e.ko vmlinux
Thanks,
Hui
Signed-off-by: Hui Zhu <teaw...@gmail.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
Cc: Arjan van de Ven <ar...@linux.intel.com>
Cc: Sam Ravnborg <s...@ravnborg.org>
Cc: oz...@pardus.org.tr
Cc: Matthew Wilcox <wi...@linux.intel.com>
---
scripts/markup_oops.pl | 45 +++++++++++++++++++++++++++++++++++----------
1 file changed, 35 insertions(+), 10 deletions(-)
--- a/scripts/markup_oops.pl
+++ b/scripts/markup_oops.pl
@@ -2,6 +2,7 @@
use File::Basename;
use Math::BigInt;
+use Getopt::Long;
# Copyright 2008, Intel Corporation
#
@@ -15,7 +16,17 @@ use Math::BigInt;
# Arjan van de Ven <ar...@linux.intel.com>
-my $vmlinux_name = $ARGV[0];
+my $cross_compile = "";
+my $vmlinux_name = "";
+my $modulefile = "";
+
+# Get options
+Getopt::Long::GetOptions(
+ 'cross_compile|c=s' => \$cross_compile,
+ 'modulefile|m=s' => \$modulefile,
+ 'help|h' => \&usage,
+);
+my $vmlinux_name = $ARGV[$#ARGV];
if (!defined($vmlinux_name)) {
my $kerver = `uname -r`;
chomp($kerver);
@@ -23,6 +34,7 @@ if (!defined($vmlinux_name)) {
print "No vmlinux specified, assuming $vmlinux_name\n";
}
my $filename = $vmlinux_name;
+
#
# Step 1: Parse the oops to find the EIP value
#
@@ -177,26 +189,26 @@ my $decodestart = Math::BigInt->from_hex
my $decodestop = Math::BigInt->from_hex("0x$target") + 8192;
if ($target eq "0") {
print "No oops found!\n";
- print "Usage: \n";
- print " dmesg | perl scripts/markup_oops.pl vmlinux\n";
- exit;
+ usage();
}
# if it's a module, we need to find the .ko file and calculate a load offset
if ($module ne "") {
- my $modulefile = `modinfo $module | grep '^filename:' | awk '{ print \$2 }'`;
- chomp($modulefile);
+ if ($modulefile eq "") {
+ my $modulefile = `modinfo $module | grep '^filename:' | awk '{ print \$2 }'`;
+ chomp($modulefile);
+ }
$filename = $modulefile;
if ($filename eq "") {
print "Module .ko file for $module not found. Aborting\n";
exit;
}
# ok so we found the module, now we need to calculate the vma offset
- open(FILE, "objdump -dS $filename |") || die "Cannot start objdump";
+ open(FILE, $cross_compile."objdump -dS $filename |") || die "Cannot start objdump";
while (<FILE>) {
if ($_ =~ /^([0-9a-f]+) \<$function\>\:/) {
my $fu = $1;
- $vmaoffset = hex($target) - hex($fu) - hex($func_offset);
+ $vmaoffset = Math::BigInt->from_hex("0x$target") - Math::BigInt->from_hex("0x$fu") - Math::BigInt->from_hex("0x$func_offset");
}
}
close(FILE);
@@ -212,7 +224,7 @@ sub InRange {
my ($address, $target) = @_;
my $ad = "0x".$address;
my $ta = "0x".$target;
- my $delta = hex($ad) - hex($ta);
+ my $delta = Math::BigInt->from_hex($ad) - Math::BigInt->from_hex($ta);
if (($delta > -4096) && ($delta < 4096)) {
return 1;
@@ -225,7 +237,7 @@ sub InRange {
# first, parse the input into the lines array, but to keep size down,
# we only do this for 4Kb around the sweet spot
-open(FILE, "objdump -dS --adjust-vma=$vmaoffset --start-address=$decodestart --stop-address=$decodestop $filename |") || die "Cannot start objdump";
+open(FILE, $cross_compile."objdump -dS --adjust-vma=$vmaoffset --start-address=$decodestart --stop-address=$decodestop $filename |") || die "Cannot start objdump";
while (<FILE>) {
my $line = $_;
@@ -344,3 +356,16 @@ while ($i < $finish) {
$i = $i +1;
}
+sub usage {
+ print <<EOT;
+Usage:
+ dmesg | perl $0 [OPTION] [VMLINUX]
+
+OPTION:
+ -c, -cross_compile CROSS_COMPILE Specify the prefix used for toolchain.
+ -m, -modulefile MODULE_DIRNAME Specify the module directory name.
+ -h, -help Help.
+EOT
+ exit;
+}
+
Américo Wang
Please make it "cross-compile", because underline is not common
in command line, so "--cross_compile" is worse than "--cross-compile".
> + 'modulefile|m=s' => \$modulefile,
"module" should be enough to express the meaning, I think.
> + 'help|h' => \&usage,
> +);
> +my $vmlinux_name = $ARGV[$#ARGV];
> if (!defined($vmlinux_name)) {
> my $kerver = `uname -r`;
> chomp($kerver);
> @@ -23,6 +34,7 @@ if (!defined($vmlinux_name)) {
> print "No vmlinux specified, assuming $vmlinux_name\n";
> }
> my $filename = $vmlinux_name;
> +
Adding this empty line probably is not what you really want.
> #
> # Step 1: Parse the oops to find the EIP value
> #
{snip}
>
> +sub usage {
> + print <<EOT;
> +Usage:
> + dmesg | perl $0 [OPTION] [VMLINUX]
> +
> +OPTION:
> + -c, -cross_compile CROSS_COMPILE Specify the prefix used for
> toolchain.
> + -m, -modulefile MODULE_DIRNAME Specify the module directory name.
> + -h, -help Help.
> +EOT
> + exit;
> +}
> +
GNU style is preferred here, please use "--foo" instead of "-foo".
Thanks.
Hui Zhu
Thanks for your mail.
Américo Wang:
>
>
> "module" should be enough to express the meaning, I think.
>
>
>> + 'help|h' => \&usage,
>> +);
>> +my $vmlinux_name = $ARGV[$#ARGV];
>> if (!defined($vmlinux_name)) {
>> my $kerver = `uname -r`;
>> chomp($kerver);
>> @@ -23,6 +34,7 @@ if (!defined($vmlinux_name)) {
>> print "No vmlinux specified, assuming $vmlinux_name\n";
>> }
>> my $filename = $vmlinux_name;
>> +
>
>
> Adding this empty line probably is not what you really want.
>
>> #
>> # Step 1: Parse the oops to find the EIP value
>> #
>
> {snip}
>
About this part, the code before "Parse the oops to find the EIP value" is for "Get options".
I change this part to:
my $filename = $vmlinux_name;
-#
-# Step 1: Parse the oops to find the EIP value
-#
+
+# Parse the oops to find the EIP value
What do you think about it?
And I changed the other part according to your mail.
Best regards,
Hui
The markup_oops.pl have 3 troubles to support cross-compiler environment:
1. It use objdump directly.
2. It use modinfo to get the message of module.
3. It use hex function that cannot support 64-bit number in 32-bit arch.
This patch add 3 options to markup_oops.pl:
1. -c CROSS_COMPILE Specify the prefix used for toolchain.
2. -m MODULE_DIRNAME Specify the module directory name.
3. Change hex function to Math::BigInt->from_hex.
After this patch, parse the x8664 oops in x86, we can:
cat amd64m | perl ~/kernel/tmp/m.pl -c /home/teawater/kernel/bin/x8664- -m ./e.ko vmlinux
Thanks,
Hui
Signed-off-by: Hui Zhu <teaw...@gmail.com>
Cc: Andrew Morton <ak...@linux-foundation.org>
Cc: Arjan van de Ven <ar...@linux.intel.com>
Cc: Sam Ravnborg <s...@ravnborg.org>
Cc: oz...@pardus.org.tr
Cc: Matthew Wilcox <wi...@linux.intel.com>
---
scripts/markup_oops.pl | 49 ++++++++++++++++++++++++++++++++++++-------------
1 file changed, 36 insertions(+), 13 deletions(-)
--- a/scripts/markup_oops.pl
+++ b/scripts/markup_oops.pl
@@ -2,6 +2,7 @@
use File::Basename;
use Math::BigInt;
+use Getopt::Long;
# Copyright 2008, Intel Corporation
#
@@ -15,7 +16,17 @@ use Math::BigInt;
# Arjan van de Ven <ar...@linux.intel.com>
-my $vmlinux_name = $ARGV[0];
+my $cross_compile = "";
+my $vmlinux_name = "";
+my $modulefile = "";
+
+# Get options
+Getopt::Long::GetOptions(
+ 'cross-compile|c=s' => \$cross_compile,
+ 'module|m=s' => \$modulefile,
+ 'help|h' => \&usage,
+);
+my $vmlinux_name = $ARGV[$#ARGV];
if (!defined($vmlinux_name)) {
my $kerver = `uname -r`;
chomp($kerver);
@@ -23,9 +34,8 @@ if (!defined($vmlinux_name)) {
print "No vmlinux specified, assuming $vmlinux_name\n";
}
my $filename = $vmlinux_name;
-#
-# Step 1: Parse the oops to find the EIP value
-#
+
+# Parse the oops to find the EIP value
my $target = "0";
my $function;
@@ -177,26 +187,26 @@ my $decodestart = Math::BigInt->from_hex
@@ -212,7 +222,7 @@ sub InRange {
my ($address, $target) = @_;
my $ad = "0x".$address;
my $ta = "0x".$target;
- my $delta = hex($ad) - hex($ta);
+ my $delta = Math::BigInt->from_hex($ad) - Math::BigInt->from_hex($ta);
if (($delta > -4096) && ($delta < 4096)) {
return 1;
@@ -225,7 +235,7 @@ sub InRange {
# first, parse the input into the lines array, but to keep size down,
# we only do this for 4Kb around the sweet spot
-open(FILE, "objdump -dS --adjust-vma=$vmaoffset --start-address=$decodestart --stop-address=$decodestop $filename |") || die "Cannot start objdump";
+open(FILE, $cross_compile."objdump -dS --adjust-vma=$vmaoffset --start-address=$decodestart --stop-address=$decodestop $filename |") || die "Cannot start objdump";
while (<FILE>) {
my $line = $_;
@@ -344,3 +354,16 @@ while ($i < $finish) {
$i = $i +1;
}
+sub usage {
+ print <<EOT;
+Usage:
+ dmesg | perl $0 [OPTION] [VMLINUX]
+
+OPTION:
+ -c, --cross-compile CROSS_COMPILE Specify the prefix used for toolchain.
+ -m, --module MODULE_DIRNAME Specify the module directory name.
+ -h, --help Help.
+EOT
+ exit;
+}
+
Américo Wang
>
> What do you think about it?
>
> And I changed the other part according to your mail.
It looks fine for me now, I assume you already tested it.
>
> Best regards,
> Hui
>
> The markup_oops.pl have 3 troubles to support cross-compiler environment:
> 1. It use objdump directly.
> 2. It use modinfo to get the message of module.
> 3. It use hex function that cannot support 64-bit number in 32-bit arch.
>
> This patch add 3 options to markup_oops.pl:
> 1. -c CROSS_COMPILE Specify the prefix used for toolchain.
> 2. -m MODULE_DIRNAME Specify the module directory name.
> 3. Change hex function to Math::BigInt->from_hex.
>
> After this patch, parse the x8664 oops in x86, we can:
> cat amd64m | perl ~/kernel/tmp/m.pl -c /home/teawater/kernel/bin/x8664- -m
> ./e.ko vmlinux
>
> Thanks,
> Hui
>
> Signed-off-by: Hui Zhu <teaw...@gmail.com>
> Cc: Andrew Morton <ak...@linux-foundation.org>
> Cc: Arjan van de Ven <ar...@linux.intel.com>
> Cc: Sam Ravnborg <s...@ravnborg.org>
> Cc: oz...@pardus.org.tr
> Cc: Matthew Wilcox <wi...@linux.intel.com>
>
Acked-by: WANG Cong <xiyou.w...@gmail.com>
Thanks!
Hui Zhu
On Tue, Jan 26, 2010 at 17:15, Am�rico Wang <xiyou.w...@gmail.com> wrote:
> On Tue, Jan 26, 2010 at 5:13 PM, Hui Zhu <hui...@windriver.com> wrote:
>>
>> What do you think about it?
>>
>> And I changed the other part according to your mail.
>
>
> It looks fine for me now, I assume you already tested it.
It was tested. :)
>
>
>>
>> Best regards,
>> Hui
>>
>> The markup_oops.pl have 3 troubles to support cross-compiler environment:
>> 1. �It use objdump directly.
>> 2. �It use modinfo to get the message of module.
>> 3. �It use hex function that cannot support 64-bit number in 32-bit arch.
>>
>> This patch add 3 options to markup_oops.pl:
>> 1. -c CROSS_COMPILE � � Specify the prefix used for toolchain.
>> 2. -m MODULE_DIRNAME � �Specify the module directory name.
>> 3. Change hex function to Math::BigInt->from_hex.
>>
>> After this patch, parse the x8664 oops in x86, we can:
>> cat amd64m | perl ~/kernel/tmp/m.pl -c /home/teawater/kernel/bin/x8664- -m
>> ./e.ko vmlinux
>>
>> Thanks,
>> Hui
>>
>> Signed-off-by: Hui Zhu <teaw...@gmail.com>
>> Cc: Andrew Morton <ak...@linux-foundation.org>
>> Cc: Arjan van de Ven <ar...@linux.intel.com>
>> Cc: Sam Ravnborg <s...@ravnborg.org>
>> Cc: oz...@pardus.org.tr
>> Cc: Matthew Wilcox <wi...@linux.intel.com>
>>
>
> Acked-by: WANG Cong <xiyou.w...@gmail.com>
>
Thanks,
Hui
Michal Marek
> +# Get options
> +Getopt::Long::GetOptions(
> + 'cross-compile|c=s' => \$cross_compile,
> + 'module|m=s' => \$modulefile,
> + 'help|h' => \&usage,
> +);
You should check the return code of GetOptions() and abort on invalid
options.
> +my $vmlinux_name = $ARGV[$#ARGV];
GetOptions() deletes the recognized options from @ARGV, so you can say
$ARGV[0] as before (and maybe check if there aren't any superfluous
arguments).
> # if it's a module, we need to find the .ko file and calculate a load
> offset
> if ($module ne "") {
> - my $modulefile = `modinfo $module | grep '^filename:' | awk '{
> print \$2 }'`;
> - chomp($modulefile);
> + if ($modulefile eq "") {
> + my $modulefile = `modinfo $module | grep '^filename:' | awk '{
> print \$2 }'`;
I know you didn't add this, but while at it, could you replace the
pipeline with just `modinfo -F filename $module`?
> +sub usage {
> + print <<EOT;
> +Usage:
> + dmesg | perl $0 [OPTION] [VMLINUX]
> +
> +OPTION:
> + -c, --cross-compile CROSS_COMPILE Specify the prefix used for
> toolchain.
> + -m, --module MODULE_DIRNAME Specify the module directory name.
Here and in the changelog you talk about "module directory name", but in
fact this is the module filename.
Thanks,
Michal
Hui Zhu
Thanks Michael.
I make a patch according to your mail.
Best regards,
Hui
1. Fix a little format issue.
2. Check the return of "Getopt::Long::GetOptions". Output usage and exit if it get error.
3. Change $ARGV[$#ARGV] to $ARGV[0].
4. Change the code which get $modulefile from modinfo. Replace the pipeline with `modinfo -F filename $module`.
4. Change usage from "Specify the module directory name" to "Specify the module filename".
Signed-off-by: Hui Zhu <teaw...@gmail.com>
---
scripts/markup_oops.pl | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
--- a/scripts/markup_oops.pl
+++ b/scripts/markup_oops.pl
@@ -23,10 +23,10 @@ my $modulefile = "";
# Get options
Getopt::Long::GetOptions(
'cross-compile|c=s' => \$cross_compile,
- 'module|m=s' => \$modulefile,
+ 'module|m=s' => \$modulefile,
'help|h' => \&usage,
-);
-my $vmlinux_name = $ARGV[$#ARGV];
+) || usage ();
+my $vmlinux_name = $ARGV[0];
if (!defined($vmlinux_name)) {
my $kerver = `uname -r`;
chomp($kerver);
@@ -193,7 +193,7 @@ if ($target eq "0") {
# if it's a module, we need to find the .ko file and calculate a load offset
if ($module ne "") {
if ($modulefile eq "") {
- my $modulefile = `modinfo $module | grep '^filename:' | awk '{ print \$2 }'`;
+ $modulefile = `modinfo -F filename $module`;
chomp($modulefile);
}
$filename = $modulefile;
@@ -361,7 +361,7 @@ Usage:
OPTION:
-c, --cross-compile CROSS_COMPILE Specify the prefix used for toolchain.
- -m, --module MODULE_DIRNAME Specify the module directory name.
+ -m, --module MODULE_DIRNAME Specify the module filename.
-h, --help Help.
EOT
exit;
Michal Marek
> I make a patch according to your mail.
>
> Best regards,
> Hui
>
> 1. Fix a little format issue.
> 2. Check the return of "Getopt::Long::GetOptions". Output usage and
> exit if it get error.
> 3. Change $ARGV[$#ARGV] to $ARGV[0].
> 4. Change the code which get $modulefile from modinfo. Replace the
> pipeline with `modinfo -F filename $module`.
> 4. Change usage from "Specify the module directory name" to "Specify the
> module filename".
>
> Signed-off-by: Hui Zhu <teaw...@gmail.com>
Thanks, I applied the previous and this patch. Just to make sure I
didn't miss any of your patches, can you check that
http://repo.or.cz/w/linux-kbuild.git/blob/refs/heads/for-next:/scripts/markup_oops.pl
has all your changes? If something is missing, please resend it, thanks.
Michal
Hui Zhu
Looks you miss this one:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ef2b9b054580ef835078d8aa411bd06542cd5c1c
Thanks,
Hui
Michal Marek
> Hi Michael,
>
> Looks you miss this one:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ef2b9b054580ef835078d8aa411bd06542cd5c1c
That one is already in Linus' tree, so this is OK. BTW please don't
top-post.
Michal
> On Sat, Feb 6, 2010 at 05:38, Michal Marek <mma...@suse.cz> wrote:
>> Thanks, I applied the previous and this patch. Just to make sure I
>> didn't miss any of your patches, can you check that
>> http://repo.or.cz/w/linux-kbuild.git/blob/refs/heads/for-next:/scripts/markup_oops.pl
>> has all your changes? If something is missing, please resend it, thanks.
Hui Zhu
>
> On 8.2.2010 03:55, Hui Zhu wrote:
> > Hi Michael,
> >
> > Looks you miss this one:
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ef2b9b054580ef835078d8aa411bd06542cd5c1c
>
>
> That one is already in Linus' tree, so this is OK. BTW please don't
> top-post.
>
Got it. Thanks Michael.
Hui