Skip to content

Commit

Permalink
Tools lint cleanup (iovisor#764)
Browse files Browse the repository at this point in the history
* argdist: linter cleanup

* cpudist: linter cleanup

* execsnoop: linter cleanup

* funclatency: linter cleanup

* gethostlatency: linter cleanup

* hardirqs: linter cleanup

* memleak: linter cleanup

* mountsnoop: linter cleanup

* offcputime: linter cleanup

* softirqs: linter cleanup

* solisten: linter cleanup and u+x mode

* stacksnoop: linter cleanup

* tplist: linter cleanup

* trace: linter cleanup
  • Loading branch information
goldshtn authored and 4ast committed Oct 18, 2016
1 parent 78a3341 commit f41ae86
Show file tree
Hide file tree
Showing 14 changed files with 147 additions and 127 deletions.
60 changes: 33 additions & 27 deletions tools/argdist.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

class Probe(object):
next_probe_index = 0
aliases = { "$PID": "bpf_get_current_pid_tgid()" }
aliases = {"$PID": "bpf_get_current_pid_tgid()"}

def _substitute_aliases(self, expr):
if expr is None:
Expand All @@ -37,8 +37,8 @@ def _parse_signature(self):
# supported right now.
index = param.rfind('*')
index = index if index != -1 else param.rfind(' ')
param_type = param[0:index+1].strip()
param_name = param[index+1:].strip()
param_type = param[0:index + 1].strip()
param_name = param[index + 1:].strip()
self.param_types[param_name] = param_type

def _generate_entry(self):
Expand All @@ -65,7 +65,7 @@ def _generate_entry(self):
collect += """
u64 __time = bpf_ktime_get_ns();
%s.update(&pid, &__time);
""" % param_hash
""" % param_hash
else:
collect += "%s.update(&pid, &%s);\n" % \
(param_hash, pname)
Expand All @@ -88,8 +88,8 @@ def _generate_entry_probe(self):
self.param_types["__latency"] = "u64" # nanoseconds
for pname in self.args_to_probe:
if pname not in self.param_types:
raise ValueError("$entry(%s): no such param" \
% arg)
raise ValueError("$entry(%s): no such param" %
arg)

self.hashname_prefix = "%s_param_" % self.probe_hash_name
text = ""
Expand Down Expand Up @@ -156,8 +156,8 @@ def _validate_specifier(self):
if len(parts) > 6:
self._bail("extraneous ':'-separated parts detected")
if parts[0] not in ["r", "p", "t", "u"]:
self._bail("probe type must be 'p', 'r', 't', or 'u' " +
"but got '%s'" % parts[0])
self._bail("probe type must be 'p', 'r', 't', or 'u'" +
" but got '%s'" % parts[0])
if re.match(r"\w+\(.*\)", parts[2]) is None:
self._bail(("function signature '%s' has an invalid " +
"format") % parts[2])
Expand Down Expand Up @@ -263,8 +263,8 @@ def _generate_usdt_arg_assignment(self, i):
expr = self.exprs[i]
if self.probe_type == "u" and expr[0:3] == "arg":
return (" u64 %s = 0;\n" +
" bpf_usdt_readarg(%s, ctx, &%s);\n") % \
(expr, expr[3], expr)
" bpf_usdt_readarg(%s, ctx, &%s);\n") \
% (expr, expr[3], expr)
else:
return ""

Expand Down Expand Up @@ -294,7 +294,7 @@ def _generate_hash_decl(self):
def _generate_key_assignment(self):
if self.type == "hist":
return self._generate_usdt_arg_assignment(0) + \
("%s __key = %s;\n" % \
("%s __key = %s;\n" %
(self.expr_types[0], self.exprs[0]))
else:
text = "struct %s_key_t __key = {};\n" % \
Expand Down Expand Up @@ -323,10 +323,11 @@ def generate_text(self):
program = ""
probe_text = """
DATA_DECL
""" + (
"TRACEPOINT_PROBE(%s, %s)" % (self.tp_category, self.tp_event) \
if self.probe_type == "t" \
else "int PROBENAME(struct pt_regs *ctx SIGNATURE)") + """
""" + (
"TRACEPOINT_PROBE(%s, %s)" %
(self.tp_category, self.tp_event)
if self.probe_type == "t"
else "int PROBENAME(struct pt_regs *ctx SIGNATURE)") + """
{
PID_FILTER
PREFIX
Expand All @@ -353,7 +354,8 @@ def generate_text(self):
# signatures. Other probes force it to ().
signature = ", " + self.signature

program += probe_text.replace("PROBENAME", self.probe_func_name)
program += probe_text.replace("PROBENAME",
self.probe_func_name)
program = program.replace("SIGNATURE", signature)
program = program.replace("PID_FILTER",
self._generate_pid_filter())
Expand Down Expand Up @@ -400,7 +402,8 @@ def _attach_k(self):

def attach(self, bpf):
self.bpf = bpf
if self.probe_type == "u": return;
if self.probe_type == "u":
return
if self.is_user:
self._attach_u()
else:
Expand Down Expand Up @@ -447,7 +450,7 @@ def display(self, top):
if self.type == "freq":
print(self.label or self.raw_spec)
print("\t%-10s %s" % ("COUNT", "EVENT"))
sdata = sorted(data.items(), key=lambda kv: kv[1].value)
sdata = sorted(data.items(), key=lambda p: p[1].value)
if top is not None:
sdata = sdata[-top:]
for key, value in sdata:
Expand All @@ -461,11 +464,11 @@ def display(self, top):
self._v2s(key.v0)
else:
key_str = self._display_key(key)
print("\t%-10s %s" % \
print("\t%-10s %s" %
(str(value.value), key_str))
elif self.type == "hist":
label = self.label or (self._display_expr(0)
if not self.is_default_expr else "retval")
if not self.is_default_expr else "retval")
data.print_log2_hist(val_type=label)
if not self.cumulative:
data.clear()
Expand All @@ -478,8 +481,8 @@ class Tool(object):
Probe specifier syntax:
{p,r,t,u}:{[library],category}:function(signature)[:type[,type...]:expr[,expr...][:filter]][#label]
Where:
p,r,t,u -- probe at function entry, function exit, kernel tracepoint,
or USDT probe
p,r,t,u -- probe at function entry, function exit, kernel
tracepoint, or USDT probe
in exit probes: can use $retval, $entry(param), $latency
library -- the library that contains the function
(leave empty for kernel functions)
Expand All @@ -506,7 +509,7 @@ class Tool(object):
argdist -H 'r::__kmalloc(size_t size):u64:$latency/$entry(size)#ns per byte'
Print a histogram of nanoseconds per byte from kmalloc allocations
argdist -C 'p::__kmalloc(size_t size, gfp_t flags):size_t:size:flags&GFP_ATOMIC'
argdist -C 'p::__kmalloc(size_t sz, gfp_t flags):size_t:sz:flags&GFP_ATOMIC'
Print frequency count of kmalloc allocation sizes that have GFP_ATOMIC
argdist -p 1005 -C 'p:c:write(int fd):int:fd' -T 5
Expand All @@ -520,7 +523,8 @@ class Tool(object):
argdist -C 'r::__vfs_read():u32:$PID:$latency > 100000'
Print frequency of reads by process where the latency was >0.1ms
argdist -H 'r::__vfs_read(void *file, void *buf, size_t count):size_t:$entry(count):$latency > 1000000'
argdist -H 'r::__vfs_read(void *file, void *buf, size_t count):size_t
$entry(count):$latency > 1000000'
Print a histogram of read sizes that were longer than 1ms
argdist -H \\
Expand Down Expand Up @@ -569,7 +573,8 @@ def __init__(self):
parser.add_argument("-v", "--verbose", action="store_true",
help="print resulting BPF program code before executing")
parser.add_argument("-c", "--cumulative", action="store_true",
help="do not clear histograms and freq counts at each interval")
help="do not clear histograms and freq counts at " +
"each interval")
parser.add_argument("-T", "--top", type=int,
help="number of top results to show (not applicable to " +
"histograms)")
Expand Down Expand Up @@ -610,8 +615,9 @@ def _generate_program(self):
for probe in self.probes:
bpf_source += probe.generate_text()
if self.args.verbose:
for text in [probe.usdt_ctx.get_text() \
for probe in self.probes if probe.usdt_ctx]:
for text in [probe.usdt_ctx.get_text()
for probe in self.probes
if probe.usdt_ctx]:
print(text)
print(bpf_source)
usdt_contexts = [probe.usdt_ctx
Expand Down
1 change: 0 additions & 1 deletion tools/cpudist.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,4 +183,3 @@ def pid_to_comm(pid):
countdown -= 1
if exiting or countdown == 0:
exit()

6 changes: 3 additions & 3 deletions tools/execsnoop.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
const char __user *const __user *__argv,
const char __user *const __user *__envp)
{
// create data here and pass to submit_arg to save space on the stack (#555)
// create data here and pass to submit_arg to save stack space (#555)
struct data_t data = {};
data.pid = bpf_get_current_pid_tgid() >> 32;
bpf_get_current_comm(&data.comm, sizeof(data.comm));
Expand Down Expand Up @@ -167,8 +167,8 @@ class EventType(object):
argv = defaultdict(list)

# TODO: This is best-effort PPID matching. Short-lived processes may exit
# before we get a chance to read the PPID. This should be replaced with fetching
# PPID via C when available (#364).
# before we get a chance to read the PPID. This should be replaced with
# fetching PPID via C when available (#364).
def get_ppid(pid):
try:
with open("/proc/%d/status" % pid) as status:
Expand Down
7 changes: 4 additions & 3 deletions tools/funclatency.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
#
# Run "funclatency -h" for full usage.
#
# The pattern is a string with optional '*' wildcards, similar to file globbing.
# If you'd prefer to use regular expressions, use the -r option.
# The pattern is a string with optional '*' wildcards, similar to file
# globbing. If you'd prefer to use regular expressions, use the -r option.
#
# Currently nested or recursive functions are not supported properly, and
# timestamps will be overwritten, creating dubious output. Try to match single
Expand Down Expand Up @@ -202,7 +202,8 @@ def signal_ignore(signal, frame):
matched = b.num_open_kprobes()
else:
b.attach_uprobe(name=library, sym_re=pattern, fn_name="trace_func_entry")
b.attach_uretprobe(name=library, sym_re=pattern, fn_name="trace_func_return")
b.attach_uretprobe(name=library, sym_re=pattern,
fn_name="trace_func_return")
matched = b.num_open_uprobes()

if matched == 0:
Expand Down
3 changes: 2 additions & 1 deletion tools/gethostlatency.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
u32 pid = bpf_get_current_pid_tgid();
if (bpf_get_current_comm(&val.comm, sizeof(val.comm)) == 0) {
bpf_probe_read(&val.host, sizeof(val.host), (void *)PT_REGS_PARM1(ctx));
bpf_probe_read(&val.host, sizeof(val.host),
(void *)PT_REGS_PARM1(ctx));
val.pid = bpf_get_current_pid_tgid();
val.ts = bpf_ktime_get_ns();
start.update(&pid, &val);
Expand Down
10 changes: 5 additions & 5 deletions tools/hardirqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from time import sleep, strftime
import argparse

### arguments
# arguments
examples = """examples:
./hardirqs # sum hard irq event time
./hardirqs -d # show hard irq event time as histograms
Expand Down Expand Up @@ -49,7 +49,7 @@
label = "usecs"
debug = 0

### define BPF program
# define BPF program
bpf_text = """
#include <uapi/linux/ptrace.h>
#include <linux/irq.h>
Expand Down Expand Up @@ -106,7 +106,7 @@
}
"""

### code substitutions
# code substitutions
if args.dist:
bpf_text = bpf_text.replace('STORE',
'irq_key_t key = {.slot = bpf_log2l(delta)};' +
Expand All @@ -121,7 +121,7 @@
if debug:
print(bpf_text)

### load BPF program
# load BPF program
b = BPF(text=bpf_text)

# these should really use irq:irq_handler_entry/exit tracepoints:
Expand All @@ -130,7 +130,7 @@

print("Tracing hard irq event time... Hit Ctrl-C to end.")

### output
# output
exiting = 0 if args.interval else 1
dist = b.get_table("dist")
while (1):
Expand Down
6 changes: 4 additions & 2 deletions tools/memleak.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,13 @@ def print_outstanding():
alloc_info[info.stack_id].update(info.size)
else:
stack = list(stack_traces.walk(info.stack_id, decoder))
alloc_info[info.stack_id] = Allocation(stack, info.size)
alloc_info[info.stack_id] = Allocation(stack,
info.size)
if args.show_allocs:
print("\taddr = %x size = %s" %
(address.value, info.size))
to_show = sorted(alloc_info.values(), key=lambda a: a.size)[-top_stacks:]
to_show = sorted(alloc_info.values(),
key=lambda a: a.size)[-top_stacks:]
for alloc in to_show:
print("\t%d bytes in %d allocations from stack\n\t\t%s" %
(alloc.size, alloc.count, "\n\t\t".join(alloc.stack)))
Expand Down
3 changes: 2 additions & 1 deletion tools/mountsnoop.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,8 @@ def print_event(mounts, umounts, cpu, data, size):
event.type == EventType.EVENT_UMOUNT_RET):
if event.type == EventType.EVENT_MOUNT_RET:
syscall = mounts.pop(event.pid)
call = 'mount({source}, {target}, {type}, {flags}, {data}) = {retval}'.format(
call = ('mount({source}, {target}, {type}, {flags}, {data}) ' +
'= {retval}').format(
source=decode_mount_string(syscall['source']),
target=decode_mount_string(syscall['target']),
type=decode_mount_string(syscall['type']),
Expand Down
23 changes: 13 additions & 10 deletions tools/offcputime.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ def positive_nonzero_int(val):
./offcputime # trace off-CPU stack time until Ctrl-C
./offcputime 5 # trace for 5 seconds only
./offcputime -f 5 # 5 seconds, and output in folded format
./offcputime -m 1000 # trace only events that last more than 1000 usec.
./offcputime -M 10000 # trace only events that last less than 10000 usec.
./offcputime -m 1000 # trace only events that last more than 1000 usec
./offcputime -M 10000 # trace only events that last less than 10000 usec
./offcputime -p 185 # only trace threads for PID 185
./offcputime -t 188 # only trace thread 188
./offcputime -u # only trace user threads (no kernel)
Expand Down Expand Up @@ -82,10 +82,12 @@ def positive_nonzero_int(val):
help="duration of trace, in seconds")
parser.add_argument("-m", "--min-block-time", default=1,
type=positive_nonzero_int,
help="the amount of time in microseconds over which we store traces (default 1)")
parser.add_argument("-M", "--max-block-time", default=(1<<64)-1,
help="the amount of time in microseconds over which we " +
"store traces (default 1)")
parser.add_argument("-M", "--max-block-time", default=(1 << 64) - 1,
type=positive_nonzero_int,
help="the amount of time in microseconds under which we store traces (default U64_MAX)")
help="the amount of time in microseconds under which we " +
"store traces (default U64_MAX)")
args = parser.parse_args()
if args.pid and args.tgid:
parser.error("specify only one of -p and -t")
Expand Down Expand Up @@ -198,13 +200,14 @@ def signal_ignore(signal, frame):
bpf_text = bpf_text.replace('USER_STACK_GET', user_stack_get)
bpf_text = bpf_text.replace('KERNEL_STACK_GET', kernel_stack_get)

need_delimiter = args.delimited and not (args.kernel_stacks_only or args.user_stacks_only)
need_delimiter = args.delimited and not (args.kernel_stacks_only or
args.user_stacks_only)

# check for an edge case; the code below will handle this case correctly
# but ultimately nothing will be displayed
if args.kernel_threads_only and args.user_stacks_only:
print("ERROR: Displaying user stacks for kernel threads " \
"doesn't make sense.", file=stderr)
print("ERROR: Displaying user stacks for kernel threads " +
"doesn't make sense.", file=stderr)
exit(1)

# initialize BPF
Expand Down Expand Up @@ -240,8 +243,8 @@ def signal_ignore(signal, frame):
for k, v in sorted(counts.items(), key=lambda counts: counts[1].value):
# handle get_stackid erorrs
if (not args.user_stacks_only and k.kernel_stack_id < 0) or \
(not args.kernel_stacks_only and k.user_stack_id < 0 and \
k.user_stack_id != -errno.EFAULT):
(not args.kernel_stacks_only and k.user_stack_id < 0 and
k.user_stack_id != -errno.EFAULT):
missing_stacks += 1
# check for an ENOMEM error
if k.kernel_stack_id == -errno.ENOMEM or \
Expand Down
Loading

0 comments on commit f41ae86

Please sign in to comment.