iovisor / iovisor/bcc

ext4dist: needs cleanup

Open
#2,181 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
22.7k
Forks
4.1k
Avg merge
10d 4h
Merged PRs (30d)
3

Description

This code:

```
# Common file functions. See earlier comment about generic_file_read_iter().
# Comment by Joe Yin
# From Linux 4.10, the function .read_iter at the ext4_file_operations has
# changed to ext4_file_read_iter.
# So, I add get_kprobe_functions(b'ext4_file_read_iter'),it will first to attach ext4_file_read_iter,
# if fails and will attach the generic_file_read_iter which used to pre-4.10.

if BPF.get_kprobe_functions(b'ext4_file_read_iter'):
b.attach_kprobe(event="ext4_file_read_iter", fn_name="trace_entry")
else:
b.attach_kprobe(event="generic_file_read_iter", fn_name="trace_read_entry")
```

A) it's an incomplete change: the EXT4_FILE_OPERATIONS check in the BPF program should be dynamically included only if generic_file_read_iter() is traced, otherwise we're wasting cycles.

B) It's a mess:

```
pep8 --show-source --ignore=E123,E125,E126,E127,E128,E302 ext4dist.py
ext4dist.py:183:21: W291 trailing whitespace
# Comment by Joe Yin
^
ext4dist.py:184:75: W291 trailing whitespace
# From Linux 4.10, the function .read_iter at the ext4_file_operations has
^
ext4dist.py:186:80: E501 line too long (101 > 79 characters)
# So, I add get_kprobe_functions(b'ext4_file_read_iter'),it will first to attach ext4_file_read_iter,
^
ext4dist.py:190:1: E101 indentation contains mixed spaces and tabs
b.attach_kprobe(event="ext4_file_read_iter", fn_name="trace_entry")
^
ext4dist.py:190:1: W191 indentation contains tabs
b.attach_kprobe(event="ext4_file_read_iter", fn_name="trace_entry")
^
ext4dist.py:192:1: W191 indentation contains tabs
b.attach_kprobe(event="generic_file_read_iter", fn_name="trace_read_entry")
^
ext4dist.py:207:1: E101 indentation contains mixed spaces and tabs
try:
^
```

C) "Hi, this is what I did" comments do not follow the style of the rest of the tools, which use terse comments and a change history at the top:

```
[...]
# 12-Feb-2016 Brendan Gregg Created this.
```

I'd rather have seen the author put their name in that history with "Fixed for 4.10 ext4_file_read_iter".

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.