[PATCH -tip] Fastboot: fix initcalls disposition in bootgraph.pl

[PATCH -tip] Fastboot: fix initcalls disposition in bootgraph.pl

[Frederic Weisbecker]

[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]

When bootgraph.pl parses a file, it gives one row
for each initcall's pid. But only few of them will
be displayed => the longest.
This patch corrects it by giving only a rows for pids
which have initcalls that will be displayed.
You can see the result in attachment, before and after the patch.

Ingo, could you please revert the commit b4bc656397071b18c99e85d9181f076e44de8a99 which was a false solution
to solve this problem. Thanks.


(I'm sorry if the patch doesn't look smart. I've
never touch a line in perl until now).

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 scripts/bootgraph.pl |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/scripts/bootgraph.pl b/scripts/bootgraph.pl
index 2243353..5e7316e 100644
--- a/scripts/bootgraph.pl
+++ b/scripts/bootgraph.pl
@@ -37,13 +37,13 @@
 # 	dmesg | perl scripts/bootgraph.pl > output.svg
 #
 
-my @rows;
-my %start, %end, %row;
+my %start, %end;
 my $done = 0;
-my $rowcount = 0;
 my $maxtime = 0;
 my $firsttime = 100;
 my $count = 0;
+my %pids;
+
 while (<>) {
 	my $line = $_;
 	if ($line =~ /([0-9\.]+)\] calling  ([a-zA-Z0-9\_]+)\+/) {
@@ -54,14 +54,8 @@ while (<>) {
 				$firsttime = $1;
 			}
 		}
-		$row{$func} = 1;
 		if ($line =~ /\@ ([0-9]+)/) {
-			my $pid = $1;
-			if (!defined($rows[$pid])) {
-				$rowcount = $rowcount + 1;
-				$rows[$pid] = $rowcount;
-			}
-			$row{$func} = $rows[$pid];
+			$pids{$func} = $1;
 		}
 		$count = $count + 1;
 	}
@@ -109,17 +103,25 @@ $styles[11] = "fill:rgb(128,255,255);fill-opacity:0.5;stroke-width:1;stroke:rgb(
 my $mult = 950.0 / ($maxtime - $firsttime);
 my $threshold = ($maxtime - $firsttime) / 60.0;
 my $stylecounter = 0;
+my %rows;
+my $rowscount = 1;
 while (($key,$value) = each %start) {
 	my $duration = $end{$key} - $start{$key};
 
 	if ($duration >= $threshold) {
 		my $s, $s2, $e, $y;
+		$pid = $pids{$key};
+
+		if (!defined($rows{$pid})) {
+			$rows{$pid} = $rowscount;
+			$rowscount = $rowscount + 1;
+		}
 		$s = ($value - $firsttime) * $mult;
 		$s2 = $s + 6;
 		$e = ($end{$key} - $firsttime) * $mult;
 		$w = $e - $s;
 
-		$y = $row{$key} * 150;
+		$y = $rows{$pid} * 150;
 		$y2 = $y + 4;
 
 		$style = $styles[$stylecounter];

[-- Attachment #2: before.svg --]
[-- Type: image/svg+xml, Size: 2315 bytes --]

[-- Attachment #3: after.svg --]
[-- Type: image/svg+xml, Size: 2309 bytes --]

Re: [PATCH -tip] Fastboot: fix initcalls disposition in bootgraph.pl

[Arjan van de Ven]

On Sat, 04 Oct 2008 21:35:48 +0200
Frederic Weisbecker <fweisbec@gmail.com> wrote:

> When bootgraph.pl parses a file, it gives one row
> for each initcall's pid. But only few of them will
> be displayed => the longest.
> This patch corrects it by giving only a rows for pids
> which have initcalls that will be displayed.
> You can see the result in attachment, before and after the patch.
> 
> Ingo, could you please revert the commit
> b4bc656397071b18c99e85d9181f076e44de8a99 which was a false solution
> to solve this problem. Thanks.


I wonder about this.. there's only supposed to be 2....
your graph is quite different.

Re: [PATCH -tip] Fastboot: fix initcalls disposition in bootgraph.pl

[Frédéric Weisbecker]

2008/10/4 Arjan van de Ven <arjan@infradead.org>:
> On Sat, 04 Oct 2008 21:35:48 +0200
> Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
>> When bootgraph.pl parses a file, it gives one row
>> for each initcall's pid. But only few of them will
>> be displayed => the longest.
>> This patch corrects it by giving only a rows for pids
>> which have initcalls that will be displayed.
>> You can see the result in attachment, before and after the patch.
>>
>> Ingo, could you please revert the commit
>> b4bc656397071b18c99e85d9181f076e44de8a99 which was a false solution
>> to solve this problem. Thanks.
>
>
> I wonder about this.. there's only supposed to be 2....
> your graph is quite different.
>

I'm tracing all of the initcalls in my machine (even the modules).
If you look at both svg sources, you will see all of the 5 initcalls displayed.
But for the "before the patch" one, it depends on the software you are
using to visualize it.
With firefox you only see two of them. With the default gnome
visualization software (don't remember its name)
you can see all of them but in a very extensive image because a lot of
rows are unused.

With the "after the patch one", there is no issues....

Re: [PATCH -tip] Fastboot: fix initcalls disposition in bootgraph.pl

[Arjan van de Ven]

On Sat, 4 Oct 2008 22:28:49 +0200
"Frédéric Weisbecker" <fweisbec@gmail.com> wrote:

> I'm tracing all of the initcalls in my machine (even the modules).
> If you look at both svg sources, you will see all of the 5 initcalls
> displayed. But for the "before the patch" one, it depends on the
> software you are using to visualize it.
> With firefox you only see two of them. With the default gnome
> visualization software (don't remember its name)
> you can see all of them but in a very extensive image because a lot of
> rows are unused.
> 
> With the "after the patch one", there is no issues....

fair enough

Acked-by: Arjan van de Ven <arjan@linux.intel.com>

-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH -tip] Fastboot: fix initcalls disposition in bootgraph.pl

[Ingo Molnar]

* Arjan van de Ven <arjan@infradead.org> wrote:

> On Sat, 4 Oct 2008 22:28:49 +0200
> "Frédéric Weisbecker" <fweisbec@gmail.com> wrote:
> 
> > I'm tracing all of the initcalls in my machine (even the modules).
> > If you look at both svg sources, you will see all of the 5 initcalls
> > displayed. But for the "before the patch" one, it depends on the
> > software you are using to visualize it.
> > With firefox you only see two of them. With the default gnome
> > visualization software (don't remember its name)
> > you can see all of them but in a very extensive image because a lot of
> > rows are unused.
> > 
> > With the "after the patch one", there is no issues....
> 
> fair enough
> 
> Acked-by: Arjan van de Ven <arjan@linux.intel.com>

applied to tip/tracing/core, thanks!

Frédéric, is the revert still needed, or is latest tip/master OK ?

	Ingo

Re: [PATCH -tip] Fastboot: fix initcalls disposition in bootgraph.pl

[Frédéric Weisbecker]

2008/10/5 Ingo Molnar <mingo@elte.hu>:
> Frédéric, is the revert still needed, or is latest tip/master OK ?

Yes it should be reverted. Tracing is still stopped at the end of kernel_init().

Thanks!

Re: [PATCH -tip] Fastboot: fix initcalls disposition in bootgraph.pl

[Frederic Weisbecker]

Frédéric Weisbecker wrote:
> 2008/10/6 Ingo Molnar <mingo@elte.hu>:
>> * Frédéric Weisbecker <fweisbec@gmail.com> wrote:
>>
>>> 2008/10/5 Ingo Molnar <mingo@elte.hu>:
>>>> Frédéric, is the revert still needed, or is latest tip/master OK ?
>>> Yes it should be reverted. Tracing is still stopped at the end of
>>> kernel_init().
>> lost track - exactly which commit ID should be reverted?
>>
>>        Ingo
>>
> 
> Hmm actually, the "stop_boot_trace" feature may be needed in  the
> future. If I trace the shed events too,
> I should be able to limit the tracing to avoid too much entries. I
> will see later.
> Perhaps just a patch to remove the trace stopping after builtin initcalls.
> I will submit it soon.
> 
> Sorry, just forget this revert request.
> 
> Thanks.
> 

From: Frederic Weisbecker <fweisbec@gmail.com>
Subject: [PATCH -tip] Tracing/fastboot: Revert the stopping of the boot tracer

Since the patch that stopped the tracing after builtin-initcalls wasn't
a real solution of a bootgraph.pl's bug, we may continue to trace boot
after builtin initcalls. But we want to keep the stop_boot_trace function
for further uses.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
diff --git a/init/main.c b/init/main.c
index 6371981..f039e7a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -975,7 +975,6 @@ static int __init kernel_init(void * unused)
 	 * we're essentially up and running. Get rid of the
 	 * initmem segments and start the user-mode stuff..
 	 */
-	stop_boot_trace();
 	init_post();
 	return 0;
 }