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

xfsslower: Fix compilation error due to rewriter update #1767

Merged
merged 1 commit into from
May 18, 2018

Conversation

pchaigno
Copy link
Contributor

@pchaigno pchaigno commented May 17, 2018

Since #1737, the bcc rewriter is able to track more external pointers going through maps. xfsslower was relying on the rewriter not being able to replace some dereferences. This commit takes this
into account and removes two unnecessary calls to bpf_probe_read.

Fixes #1766.

/cc @bobrik

@bobrik
Copy link
Contributor

bobrik commented May 17, 2018

I'm concerned that we have other tools with the same issue. Any thoughts on this:

Should we detect this and print a warning instead of breaking existing usage?

@pchaigno
Copy link
Contributor Author

@bobrik I'm answering in the original issue. Give me a minute to track the appropriate commits I need to reference. Could you double check that this does fix the issue for you too in the meantime?

bpf_probe_read(&de, sizeof(de), &valp->fp->f_path.dentry);
bpf_probe_read(&qs, sizeof(qs), (void *)&de->d_name);
struct dentry *de = valp->fp->f_path.dentry;
struct qstr qs = de->d_name;
Copy link
Contributor

Choose a reason for hiding this comment

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

You can do just this:

struct qstr qs = valp->fp->f_path.dentry->d_name;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes. Looks like we can :-)
That addresses the above comment too. Thanks!

Since ad2d0d9, the bcc rewriter is able to track more external
pointers going through maps.  xfsslower and zfsslower were relying on
the rewriter not being able to replace some dereferences.  This
commit takes this into account and removes two unnecessary calls to
bpf_probe_read.
@pchaigno
Copy link
Contributor Author

Looks like zfsslower has the exact same issue (could have guessed :/). Other tools look fine. I'll try to improve the smoke_tool tests to catch this kind of issues tomorrow.

@yonghong-song yonghong-song merged commit 19b7f74 into iovisor:master May 18, 2018
@pchaigno pchaigno deleted the fix-xfsslower branch May 18, 2018 06:42
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.

None yet

3 participants