Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

f2fsslower: track IO operations that are slower than a given threshold in F2FS file system #4782

Merged
merged 4 commits into from
Dec 19, 2023

Conversation

zhangting9756
Copy link
Contributor

add new tool tracing IO operation slower than a given threshold for f2fs file system in Andriod and Linux

…fied functions in the F2FS file system and tracks IO operations that are slower than a given threshold.
@chenhengqi
Copy link
Collaborator

Copy link
Collaborator

@chenhengqi chenhengqi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reword the subject line and commit message properly.

from time import strftime

# symbols
kallsyms = "/proc/kallsyms"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move close to where we use it.

// calculate delta
u64 ts = bpf_ktime_get_ns();
u64 delta_us = (ts - valp->ts) / 1000;
entryinfo.delete(&id);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do NOT delete the element prematurely. This should be done after perf submit calls.

Comment on lines 208 to 223
int trace_read_return(struct pt_regs *ctx)
{
return trace_return(ctx, TRACE_READ);
}
int trace_write_return(struct pt_regs *ctx)
{
return trace_return(ctx, TRACE_WRITE);
}
int trace_open_return(struct pt_regs *ctx)
{
return trace_return(ctx, TRACE_OPEN);
}
int trace_fsync_return(struct pt_regs *ctx)
{
return trace_return(ctx, TRACE_FSYNC);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an empty line between functions.


# Common file functions. See earlier comment about generic_file_read_iter().
if BPF.get_kprobe_functions(b'f2fs_file_read_iter'):
b.attach_kprobe(event="f2fs_file_read_iter", fn_name="trace_read_entry")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attach_kretprobe here we can save an if block below.

else:
b.perf_buffer_poll()
except KeyboardInterrupt:
exit()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No newline below ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've modified the code based on review advice and invite you review again,thank you

@zhangting9756 zhangting9756 changed the title f2fsslower:This tool attaches to the entry and return points of speci… f2fsslower: track IO operations that are slower than a given threshold in F2FS file system Nov 9, 2023
Comment on lines 200 to 201
if (qs.len == 0)
return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entryinfo.delete(&id); before returning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing again,code had modifed based on comments

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not what I asked for. 🤦

entryinfo.delete(&id); can be done only if valp is not referenced any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, code had modified for resource clean-up before returning

@zhangting9756
Copy link
Contributor Author

@chenhengqi @yonghong-song @weibang6liu will you review this bcc tool and commit?

@yonghong-song
Copy link
Collaborator

We have f2fsslower in libbpf-tools. So it makes sense to also have a bcc version. @chenhengqi Since you have reviewed this parch before, could you take a look at the patch to check whether there are any remaining issues or not? Thanks!

@chenhengqi chenhengqi merged commit 2ac4a63 into iovisor:master Dec 19, 2023
1 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants