Page MenuHomeFreeBSD

Add a central location for exclusion checks. We check here if function is excluded from FBT instrumentation
ClosedPublic

Authored by br on Jun 24 2015, 2:25 PM.

Details

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

br retitled this revision from to Add a central location for exclusion checks. We check here if function is excluded from FBT instrumentation.
br updated this object.
br edited the test plan for this revision. (Show Details)
br added a reviewer: DTrace.
br added a child revision: D2738: DTrace support for AArch64.
emaste added inline comments.
sys/cddl/dev/fbt/fbt.c
129–130 ↗(On Diff #6431)

A comment perhaps?

132–137 ↗(On Diff #6431)

Should this be limited to arm/arm64, or apply to all archs?

andrew added inline comments.
sys/cddl/dev/fbt/fbt.c
133–134 ↗(On Diff #6431)

How about something like the following?

When DTrace is built into the kernel we need to exclude the FBT functions from instrumentation.

sys/cddl/dev/fbt/fbt.c
132–137 ↗(On Diff #6431)

This will apply to all archs.

markj added inline comments.
sys/cddl/dev/fbt/fbt.c
132–137 ↗(On Diff #6431)

This isn't foolproof: what if one enables both fbt::systrace_probe:entry and a syscall probe?

I think we've discussed this problem before, but there isn't an obvious/straightforward solution. It seems to me that fixing this properly will involve something along the lines of: identify object files we want to avoid tracing, modify the build to dump their symbols and generate a table that's linked into fbt.ko or the kernel, and use the table to do lookups in fbt_excluded().

I don't object to this change, but it doesn't seem like the right approach to me. Perhaps it should at least be conditionalized on #ifndef _KLD_MODULE?

sys/cddl/dev/fbt/fbt.c
132–137 ↗(On Diff #6431)

Perhaps it should at least be conditionalized on #ifndef _KLD_MODULE?

Seems like that would be a reasonable interim solution.

This revision was automatically updated to reflect the committed changes.