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

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(-)

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

Cc: stable@vger.kernel.org
Signed-off-by: Ben Hutchings <redacted>
---
 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

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 <redacted>
---
 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():

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

Signed-off-by: Ben Hutchings <redacted>
---
 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():

Signed-off-by: Ben Hutchings <redacted>
---
 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")

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 <redacted>
---
 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)

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(-)

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:

quoted

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>

quoted

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.

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:

quoted

On Tue, May 21, 2013 at 01:42:56AM +0100, Ben Hutchings wrote:

quoted

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.

quoted

Either way, this all looks good.  Thanks Ben.

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

quoted

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.

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:

quoted

On Tue, May 21, 2013 at 01:42:56AM +0100, Ben Hutchings wrote:

quoted

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

quoted

Either way, this all looks good.  Thanks Ben.

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

quoted

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.

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:

quoted

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.