markup_oops.pl: add options to improve cross-sompilation environments

PATCH In review Hui Zhu 4 participants 13 messages Jan 26, 2010 – Feb 20, 2010

lore

Share & tools

lore

Hui Zhu

author

2010-01-26 03:00 UTC #0 lore raw

PATCH 1/1 markup_oops.pl: add options to improve cross-sompilation environments

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 <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Arjan van de Ven <[email protected]>

Cc Sam Ravnborg <[email protected]>

Cc [email protected]

Cc Matthew Wilcox <[email protected]>

---
scripts/markup_oops.pl | 71 ++++++++++++++++++++++++++++++++++++++++++-------

1 file changed, 61 insertions(+), 10 deletions(-)

scripts/markup_oops.pl +0 -0

@@ -15,8 +15,46 @@ use Math::BigInt;

# Arjan van de Ven <[email protected]>

-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];
+}

+else {
	my $kerver = `uname -r`;
	chomp($kerver);
	$vmlinux_name = "/lib/modules/$kerver/build/vmlinux";
@@ -177,26 +215,27 @@ 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";
+	usage();
	exit;
}

# 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 +251,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 +264,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 +383,15 @@ while ($i < $finish) {
	$i = $i +1;
}

+sub usage {
+ print <<EOT;
+Usage:
+ dmesg | perl $0 [OPTION] [VMLINUX]
+
+OPTION:

+  -c CROSS_COMPILE	Specify the prefix used for toolchain.
+  -m MODULE_DIRNAME	Specify the module directory name.
+  -h			Help

+EOT
+}
+

Américo Wang

2010-01-26 03:05 UTC #1 lore raw

Quoted 69 lines · Hui Zhu

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 <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Arjan van de Ven <[email protected]>
Cc Sam Ravnborg <[email protected]>
Cc [email protected]
Cc Matthew Wilcox <[email protected]>
---
scripts/markup_oops.pl | 71
++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 61 insertions(+), 10 deletions(-)
scripts/markup_oops.pl +29 -2
@@ -15,8 +15,46 @@ use Math::BigInt;
 #       Arjan van de Ven <[email protected]>
 
 
-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;

scripts/markup_oops.pl use Math::BigInt; · line 56

Quoted 10 lines

scripts/markup_oops.pl +10 -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?

Hui Zhu

author

2010-01-26 07:27 UTC #2 lore raw

Américo Wang:

Quoted 11 lines · Hui Zhu

Review
Quoted 7 lines
Sorry guys, the prev mail still have some format trouble. I send a new mail for it.
(quoted diff) +5 -0
+}
+
+if ($vmlinux_name ne "") {
+       $vmlinux_name = $ARGV[$#ARGV];
+}
Why not using the Perl module 'Getopt' to do this?

Hi Américo,

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.

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 <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Arjan van de Ven <[email protected]>

Cc Sam Ravnborg <[email protected]>

Cc [email protected]

Cc Matthew Wilcox <[email protected]>

---
scripts/markup_oops.pl | 45 +++++++++++++++++++++++++++++++++++----------

1 file changed, 35 insertions(+), 10 deletions(-)

scripts/markup_oops.pl +0 -0

@@ -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 <[email protected]>

-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

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

2010-01-26 07:53 UTC #3 lore raw

Quoted 59 lines · Hui Zhu

Américo Wang:
Quoted 14 lines · Hui Zhu
Review
Quoted 9 lines
Sorry guys, the prev mail still have some format trouble. I send a new mail
for it.
(quoted diff) +5 -0
+}
+
+if ($vmlinux_name ne "") {
+       $vmlinux_name = $ARGV[$#ARGV];
+}
Why not using the Perl module 'Getopt' to do this?
Hi Américo,
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 <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Arjan van de Ven <[email protected]>
Cc Sam Ravnborg <[email protected]>
Cc [email protected]
Cc Matthew Wilcox <[email protected]>
---
scripts/markup_oops.pl | 45 +++++++++++++++++++++++++++++++++++----------
1 file changed, 35 insertions(+), 10 deletions(-)
scripts/markup_oops.pl +1 -0
@@ -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 <[email protected]>

scripts/markup_oops.pl use Math::BigInt; · line 25

Quoted 10 lines

scripts/markup_oops.pl +7 -1

 
 
-my $vmlinux_name = $ARGV[0];
+my $cross_compile = "";
+my $vmlinux_name = "";
+my $modulefile = "";
+
+# Get options
+Getopt::Long::GetOptions(
+       'cross_compile|c=s'     => \$cross_compile,

Please make it "cross-compile", because underline is not common in command line, so "--cross_compile" is worse than "--cross-compile".

scripts/markup_oops.pl use Math::BigInt; · line 26

Quoted 1 line

scripts/markup_oops.pl +1 -0
+       'modulefile|m=s'        => \$modulefile,

"module" should be enough to express the meaning, I think.

scripts/markup_oops.pl if (!defined($vmlinux_name)) { · line 37

Quoted 11 lines

scripts/markup_oops.pl +4 -0

+       '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.

scripts/markup_oops.pl if (!defined($vmlinux_name)) { · line 40

Quoted 3 lines

scripts/markup_oops.pl +0 -0

 #
 # Step 1: Parse the oops to find the EIP value
 #

{snip}

Quoted 11 lines

scripts/markup_oops.pl +7 -0
+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.

scripts/markup_oops.pl

Quoted 4 lines

scripts/markup_oops.pl +4 -0
+EOT
+       exit;
+}
+

GNU style is preferred here, please use "--foo" instead of "-foo".

Thanks.

Hui Zhu

author

2010-01-26 09:03 UTC #4 lore raw

Hi Américo,

Thanks for your mail.

Américo Wang:

Quoted 20 lines

"module" should be enough to express the meaning, I think.
Review if (!defined($vmlinux_name)) { · line 37
Quoted 11 lines
(quoted diff) +3 -0
+       'help|h'                => \&usage,
+);
+my $vmlinux_name = $ARGV[$#ARGV];
if (!defined($vmlinux_name)) {
       my $kerver = `uname -r`;
       chomp($kerver);
(quoted diff) +1 -0
@@ -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.

Quoted 6 lines

Review if (!defined($vmlinux_name)) { · line 40
Quoted 3 lines
(quoted diff) +0 -0
 #
 # 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.

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 <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Arjan van de Ven <[email protected]>

Cc Sam Ravnborg <[email protected]>

Cc [email protected]

Cc Matthew Wilcox <[email protected]>

---
scripts/markup_oops.pl | 49 ++++++++++++++++++++++++++++++++++++-------------

1 file changed, 36 insertions(+), 13 deletions(-)

scripts/markup_oops.pl +0 -0

@@ -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 <[email protected]>

-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 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 +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

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

Acked-by

2010-01-26 09:15 UTC #5 lore raw

Quoted 4 lines · Hui Zhu

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.

Quoted 21 lines

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

Quoted 7 lines

Signed-off-by Hui Zhu <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Arjan van de Ven <[email protected]>
Cc Sam Ravnborg <[email protected]>
Cc [email protected]
Cc Matthew Wilcox <[email protected]>

Acked-by WANG Cong <[email protected]>

Thanks!

Hui Zhu

2010-01-26 09:21 UTC #6 lore raw

Hi Américo,

Quoted 8 lines · Américo Wang

Quoted 4 lines · Hui Zhu
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. :)

Quoted 31 lines

Quoted 23 lines
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
Quoted 7 lines
Signed-off-by Hui Zhu <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Arjan van de Ven <[email protected]>
Cc Sam Ravnborg <[email protected]>
Cc [email protected]
Cc Matthew Wilcox <[email protected]>

Quoted 2 lines

Acked-by WANG Cong <[email protected]>

Thanks,
Hui

Michal Marek

2010-01-29 20:27 UTC #7 lore raw

In reply to Hui Zhu

Review

Quoted 6 lines · Hui Zhu

(quoted diff) +6 -0

+# 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.

Quoted 1 line

+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).

Quoted 9 lines

# 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 }'`;

(quoted diff) +2 -1

-    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?

Quoted 9 lines

(quoted diff) +7 -0
+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

author

2010-02-01 05:37 UTC #8 lore raw

Michal Marek:

Quoted 33 lines · Hui Zhu

Review
Quoted 6 lines
(quoted diff) +6 -0
+# 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.
Quoted 1 line
+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).
Quoted 9 lines
# 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 }'`;
(quoted diff) +2 -1
-    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?

Quoted 13 lines

Quoted 9 lines
(quoted diff) +7 -0
+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 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 <[email protected]>

---
scripts/markup_oops.pl | 10 +++++-----

1 file changed, 5 insertions(+), 5 deletions(-)

scripts/markup_oops.pl +0 -0

@@ -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

2010-02-05 21:39 UTC #9 lore raw

Quoted 14 lines · Hui Zhu

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".

Quoted 1 line

Signed-off-by Hui Zhu <[email protected]>

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

Quoted 43 lines

---
scripts/markup_oops.pl | 10 +++++-----

1 file changed, 5 insertions(+), 5 deletions(-)

scripts/markup_oops.pl +0 -0

@@ -23,10 +23,10 @@ my $modulefile = "";
 # Get options

Getopt::Long::GetOptions(

    'cross-compile|c=s'    => \$cross_compile,

scripts/markup_oops.pl +3 -3

-    '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);

scripts/markup_oops.pl +0 -0

@@ -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 }'`;

scripts/markup_oops.pl +1 -0

+        $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.

scripts/markup_oops.pl +1 -1

-  -m, --module MODULE_DIRNAME        Specify the module directory name.
+  -m, --module MODULE_DIRNAME        Specify the module filename.
  -h, --help                Help.

EOT

    exit;

Hui Zhu

2010-02-08 02:56 UTC #10 lore raw

Hi Michael,

Looks you miss this one:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ef2b9b054580ef835078d8aa411bd06542cd5c1c

Thanks,
Hui

Quoted 69 lines · Michal Marek

Quoted 14 lines · Hui Zhu
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".
Quoted 1 line
Signed-off-by Hui Zhu <[email protected]>
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
Quoted 43 lines
---
scripts/markup_oops.pl | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
scripts/markup_oops.pl +0 -0
@@ -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,
scripts/markup_oops.pl +2 -2
-);
-my $vmlinux_name = $ARGV[$#ARGV];
+) || usage ();
+my $vmlinux_name = $ARGV[0];
if (!defined($vmlinux_name)) {
my $kerver = uname -r;
chomp($kerver);
scripts/markup_oops.pl +0 -0
@@ -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;
scripts/markup_oops.pl +0 -0
@@ -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

2010-02-17 13:08 UTC #11 lore raw

Quoted 4 lines · Hui Zhu

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

Quoted 5 lines · Michal Marek

Quoted 4 lines
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

2010-02-20 02:16 UTC #12 lore raw

Quoted 11 lines · Michal Marek

Quoted 4 lines · Hui Zhu
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