[PATCH 0/5] perf: net_dropmonitor: Make it work and make it fast

[PATCH 0/5] perf: net_dropmonitor: Make it work and make it fast

[Ben Hutchings]

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

Somewhat surprisingly, the net_dropmonitor reporting script doesn't work
at all.  This series fixes it and then makes it slightly more efficient.

Ben.

Ben Hutchings (5):
  perf: net_dropmonitor: Fix trace parameter order
  perf: net_dropmonitor: Fix symbol-relative addresses
  perf: net_dropmonitor: Do not assume ordering of dictionaries
  perf: net_dropmonitor: Use bisection in symbol lookup
  perf: net_dropmonitor: Remove progress indicator

 tools/perf/scripts/python/net_dropmonitor.py |   39 ++++++++++++++------------
 1 file changed, 21 insertions(+), 18 deletions(-)



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 1/5] perf: net_dropmonitor: Fix trace parameter order

[Ben Hutchings]

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

This works much better if we don't treat protocol numbers as addresses.

Cc: stable@vger.kernel.org
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 tools/perf/scripts/python/net_dropmonitor.py |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/scripts/python/net_dropmonitor.py b/tools/perf/scripts/python/net_dropmonitor.py
index a4ffc95..adbfbf0 100755
--- a/tools/perf/scripts/python/net_dropmonitor.py
+++ b/tools/perf/scripts/python/net_dropmonitor.py
@@ -64,7 +64,7 @@ def trace_end():
 
 # called from perf, when it finds a correspoinding event
 def skb__kfree_skb(name, context, cpu, sec, nsec, pid, comm,
-			skbaddr, protocol, location):
+		   skbaddr, location, protocol):
 	slocation = str(location)
 	try:
 		drop_log[slocation] = drop_log[slocation] + 1



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 2/5] perf: net_dropmonitor: Fix symbol-relative addresses

[Ben Hutchings]

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

The comparison between traced and symbol addresses is backwards: if
the traced address doesn't exactly match a symbol (which we don't
expect it to), we'll show the next symbol and the offset to it,
whereas we should show the previous symbol and the offset from it.

Cc: stable@vger.kernel.org
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 tools/perf/scripts/python/net_dropmonitor.py |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/scripts/python/net_dropmonitor.py b/tools/perf/scripts/python/net_dropmonitor.py
index adbfbf0..4c11605 100755
--- a/tools/perf/scripts/python/net_dropmonitor.py
+++ b/tools/perf/scripts/python/net_dropmonitor.py
@@ -40,9 +40,9 @@ def get_kallsyms_table():
 
 def get_sym(sloc):
 	loc = int(sloc)
-	for i in kallsyms:
-		if (i['loc'] >= loc):
-			return (i['name'], i['loc']-loc)
+	for i in kallsyms[::-1]:
+		if loc >= i['loc']:
+			return (i['name'], loc - i['loc'])
 	return (None, 0)
 
 def print_drop_table():



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 3/5] perf: net_dropmonitor: Do not assume ordering of dictionaries

[Ben Hutchings]

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

The sort order of dictionaries in Python is undocumented.  Use
tuples instead, which are documented to be lexically ordered.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 tools/perf/scripts/python/net_dropmonitor.py |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/scripts/python/net_dropmonitor.py b/tools/perf/scripts/python/net_dropmonitor.py
index 4c11605..6acdc82e 100755
--- a/tools/perf/scripts/python/net_dropmonitor.py
+++ b/tools/perf/scripts/python/net_dropmonitor.py
@@ -32,7 +32,7 @@ def get_kallsyms_table():
 		j = j +1
 		if ((j % 100) == 0):
 			print "\r" + str(j) + "/" + str(linecount),
-		kallsyms.append({ 'loc': loc, 'name' : name})
+		kallsyms.append((loc, name))
 
 	print "\r" + str(j) + "/" + str(linecount)
 	kallsyms.sort()
@@ -40,9 +40,9 @@ def get_kallsyms_table():
 
 def get_sym(sloc):
 	loc = int(sloc)
-	for i in kallsyms[::-1]:
-		if loc >= i['loc']:
-			return (i['name'], loc - i['loc'])
+	for symloc, name in kallsyms[::-1]:
+		if loc >= symloc:
+			return (name, loc - symloc)
 	return (None, 0)
 
 def print_drop_table():



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 4/5] perf: net_dropmonitor: Use bisection in symbol lookup

[Ben Hutchings]

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

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 tools/perf/scripts/python/net_dropmonitor.py |   22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/tools/perf/scripts/python/net_dropmonitor.py b/tools/perf/scripts/python/net_dropmonitor.py
index 6acdc82e..32fcee0 100755
--- a/tools/perf/scripts/python/net_dropmonitor.py
+++ b/tools/perf/scripts/python/net_dropmonitor.py
@@ -40,10 +40,24 @@ def get_kallsyms_table():
 
 def get_sym(sloc):
 	loc = int(sloc)
-	for symloc, name in kallsyms[::-1]:
-		if loc >= symloc:
-			return (name, loc - symloc)
-	return (None, 0)
+
+	# Invariant: kallsyms[i][0] <= loc for all 0 <= i <= start
+	#            kallsyms[i][0] > loc for all end <= i < len(kallsyms)
+	start, end = -1, len(kallsyms)
+	while end != start + 1:
+		pivot = (start + end) // 2
+		if loc < kallsyms[pivot][0]:
+			end = pivot
+		else:
+			start = pivot
+
+	# Now (start == -1 or kallsyms[start][0] <= loc)
+	# and (start == len(kallsyms) - 1 or loc < kallsyms[start + 1][0])
+	if start >= 0:
+		symloc, name = kallsyms[start]
+		return (name, loc - symloc)
+	else:
+		return (None, 0)
 
 def print_drop_table():
 	print "%25s %25s %25s" % ("LOCATION", "OFFSET", "COUNT")



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH 5/5] perf: net_dropmonitor: Remove progress indicator

[Ben Hutchings]

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

We can read /proc/kallsyms in a fraction of a second, so why waste
a further fraction of a second showing progress?

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 tools/perf/scripts/python/net_dropmonitor.py |   13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/tools/perf/scripts/python/net_dropmonitor.py b/tools/perf/scripts/python/net_dropmonitor.py
index 32fcee0..b574059 100755
--- a/tools/perf/scripts/python/net_dropmonitor.py
+++ b/tools/perf/scripts/python/net_dropmonitor.py
@@ -15,28 +15,17 @@ kallsyms = []
 
 def get_kallsyms_table():
 	global kallsyms
+
 	try:
 		f = open("/proc/kallsyms", "r")
-		linecount = 0
-		for line in f:
-			linecount = linecount+1
-		f.seek(0)
 	except:
 		return
 
-
-	j = 0
 	for line in f:
 		loc = int(line.split()[0], 16)
 		name = line.split()[2]
-		j = j +1
-		if ((j % 100) == 0):
-			print "\r" + str(j) + "/" + str(linecount),
 		kallsyms.append((loc, name))
-
-	print "\r" + str(j) + "/" + str(linecount)
 	kallsyms.sort()
-	return
 
 def get_sym(sloc):
 	loc = int(sloc)


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 0/5] perf: net_dropmonitor: Make it work and make it fast

[Neil Horman]

On Tue, May 21, 2013 at 01:42:56AM +0100, Ben Hutchings wrote:
> Somewhat surprisingly, the net_dropmonitor reporting script doesn't work
> at all.  This series fixes it and then makes it slightly more efficient.
> 
> Ben.
> 
It worked fine when I first submitted it.  I wonder if there was a paramter
order change at some point that I wasn't CC'ed on.

Either way, this all looks good.  Thanks Ben.

Acked-by: Neil Horman <nhorman@tuxdriver.com>

> Ben Hutchings (5):
>   perf: net_dropmonitor: Fix trace parameter order
>   perf: net_dropmonitor: Fix symbol-relative addresses
>   perf: net_dropmonitor: Do not assume ordering of dictionaries
>   perf: net_dropmonitor: Use bisection in symbol lookup
>   perf: net_dropmonitor: Remove progress indicator
> 
>  tools/perf/scripts/python/net_dropmonitor.py |   39 ++++++++++++++------------
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> 

Re: [PATCH 0/5] perf: net_dropmonitor: Make it work and make it fast

[Ben Hutchings]

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

On Tue, 2013-05-21 at 09:33 -0400, Neil Horman wrote:
> On Tue, May 21, 2013 at 01:42:56AM +0100, Ben Hutchings wrote:
> > Somewhat surprisingly, the net_dropmonitor reporting script doesn't work
> > at all.  This series fixes it and then makes it slightly more efficient.
> > 
> > Ben.
> > 
> It worked fine when I first submitted it.  I wonder if there was a paramter
> order change at some point that I wasn't CC'ed on.

The parameter order for the trace event changed in 2.6.39, but that was
before this script went in.

Ben.

> Either way, this all looks good.  Thanks Ben.
> 
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> 
> > Ben Hutchings (5):
> >   perf: net_dropmonitor: Fix trace parameter order
> >   perf: net_dropmonitor: Fix symbol-relative addresses
> >   perf: net_dropmonitor: Do not assume ordering of dictionaries
> >   perf: net_dropmonitor: Use bisection in symbol lookup
> >   perf: net_dropmonitor: Remove progress indicator
> > 
> >  tools/perf/scripts/python/net_dropmonitor.py |   39 ++++++++++++++------------
> >  1 file changed, 21 insertions(+), 18 deletions(-)
> > 
> > 
> 
> 

-- 
Ben Hutchings
friends: People who know you well, but like you anyway.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 0/5] perf: net_dropmonitor: Make it work and make it fast

[Neil Horman]

On Tue, May 21, 2013 at 02:54:00PM +0100, Ben Hutchings wrote:
> On Tue, 2013-05-21 at 09:33 -0400, Neil Horman wrote:
> > On Tue, May 21, 2013 at 01:42:56AM +0100, Ben Hutchings wrote:
> > > Somewhat surprisingly, the net_dropmonitor reporting script doesn't work
> > > at all.  This series fixes it and then makes it slightly more efficient.
> > > 
> > > Ben.
> > > 
> > It worked fine when I first submitted it.  I wonder if there was a paramter
> > order change at some point that I wasn't CC'ed on.
> 
> The parameter order for the trace event changed in 2.6.39, but that was
> before this script went in.
> 
Hm, I wonder if I developed it on 2.6.38, that sounds like around the time it
went it.
Neil

> Ben.
> 
> > Either way, this all looks good.  Thanks Ben.
> > 
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > 
> > > Ben Hutchings (5):
> > >   perf: net_dropmonitor: Fix trace parameter order
> > >   perf: net_dropmonitor: Fix symbol-relative addresses
> > >   perf: net_dropmonitor: Do not assume ordering of dictionaries
> > >   perf: net_dropmonitor: Use bisection in symbol lookup
> > >   perf: net_dropmonitor: Remove progress indicator
> > > 
> > >  tools/perf/scripts/python/net_dropmonitor.py |   39 ++++++++++++++------------
> > >  1 file changed, 21 insertions(+), 18 deletions(-)
> > > 
> > > 
> > 
> > 
> 
> -- 
> Ben Hutchings
> friends: People who know you well, but like you anyway.

Re: [PATCH 0/5] perf: net_dropmonitor: Make it work and make it fast

[Neil Horman]

On Tue, May 21, 2013 at 02:54:00PM +0100, Ben Hutchings wrote:
> On Tue, 2013-05-21 at 09:33 -0400, Neil Horman wrote:
> > On Tue, May 21, 2013 at 01:42:56AM +0100, Ben Hutchings wrote:
> > > Somewhat surprisingly, the net_dropmonitor reporting script doesn't work
> > > at all.  This series fixes it and then makes it slightly more efficient.
> > > 
> > > Ben.
> > > 
> > It worked fine when I first submitted it.  I wonder if there was a paramter
> > order change at some point that I wasn't CC'ed on.
> 
> The parameter order for the trace event changed in 2.6.39, but that was
> before this script went in.
> 
> Ben.
> 
Yeah, thats how this happened.  I posted right before 2.6.39, which was around
July 4th:
https://lkml.org/lkml/2011/7/4/273

So I posted it, and shortly thereafter the perf infrastructure changed.  Acme
didnt actually pull the patch in until Well into August, and so it was borked
from there.  I never noticed as I had been using the netlink based utility since
that time.

Thanks for catching this!
Neil

> > Either way, this all looks good.  Thanks Ben.
> > 
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > 
> > > Ben Hutchings (5):
> > >   perf: net_dropmonitor: Fix trace parameter order
> > >   perf: net_dropmonitor: Fix symbol-relative addresses
> > >   perf: net_dropmonitor: Do not assume ordering of dictionaries
> > >   perf: net_dropmonitor: Use bisection in symbol lookup
> > >   perf: net_dropmonitor: Remove progress indicator
> > > 
> > >  tools/perf/scripts/python/net_dropmonitor.py |   39 ++++++++++++++------------
> > >  1 file changed, 21 insertions(+), 18 deletions(-)
> > > 
> > > 
> > 
> > 
> 
> -- 
> Ben Hutchings
> friends: People who know you well, but like you anyway.

Re: [PATCH 0/5] perf: net_dropmonitor: Make it work and make it fast

[David Miller]

From: Neil Horman <nhorman@tuxdriver.com>
Date: Tue, 21 May 2013 09:33:56 -0400

> On Tue, May 21, 2013 at 01:42:56AM +0100, Ben Hutchings wrote:
>> Somewhat surprisingly, the net_dropmonitor reporting script doesn't work
>> at all.  This series fixes it and then makes it slightly more efficient.
>> 
>> Ben.
>> 
> It worked fine when I first submitted it.  I wonder if there was a paramter
> order change at some point that I wasn't CC'ed on.
> 
> Either way, this all looks good.  Thanks Ben.
> 
> Acked-by: Neil Horman <nhorman@tuxdriver.com>

Series applied, thanks.