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

collector/ftrace: Use top-level buffer instance #719

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

douglas-raillard-arm
Copy link
Collaborator

Abandon the use of non-toplevel buffer instance (-B parameter), as they are not worth the hassle. A number of frace features do not interact well with them such as:

  • bprint event emitted by trace_printk(), that always lands in the top-level buffer no matter what.
  • synthetic events that can also only reasonably be emitted in the top-level buffer as of Linux 6.13

Abandon the use of non-toplevel buffer instance (-B parameter), as they
are not worth the hassle. A number of frace features do not interact
well with them such as:
* bprint event emitted by trace_printk(), that always lands in the
  top-level buffer no matter what.
* synthetic events that can also only reasonably be emitted in the
  top-level buffer as of Linux 6.13
Copy link
Collaborator

@marcbonnici marcbonnici left a comment

Choose a reason for hiding this comment

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

Stupid questions, but I just want to check I've understood correctly from the details you listed in #717 for the motivation of this PR.

Reverting the use of non-toplevel buffers's main impact is that some events e.g. bprint which are not possible to use with non top-level buffers become usable again. The downside is that we "lose" the ability for concurrent instruments to be isolated with separate buffers however in practice this didn't seem to achieve the desired effect?
However from an end user's perspective the interface shouldn't change so we don't have to worry about compatibility issues if we revert this behaviour again?

Did I understand this correctly?

Thanks

@douglas-raillard-arm
Copy link
Collaborator Author

events e.g. bprint which are not possible to use with non top-level buffers become usable again.

They were still usable, but in the top-level instance. You can collect more than one instance in a single trace.dat, so that is what happened.

The downside is that we "lose" the ability for concurrent instruments to be isolated with separate buffers however in practice this didn't seem to achieve the desired effect?

Yes, isolation was the only upside I was chasing when I introduced that. After a couple of years of trying it out, it's basically 100% (mild) pain for 0% benefit I could see, and today, the pain turned into an actual blocker because some kernel feature (synthetic ftrace events) does not play well with instances because of some API design issue (I submitted some patches, but even if they get in, it will be years before all android kernels get fixed ...)

However from an end user's perspective the interface shouldn't change so we don't have to worry about compatibility issues if we revert this behaviour again?

Technically you can see the difference in trace-cmd report output. Instead of having this:

devlib:        trace-cmd-620   [003]  2440.867617: irq_disable:          caller=enter_from_kernel_mode+0x38 parent=el1_interrupt+0x24

you'll get that:

       trace-cmd-620   [003]  2440.867617: irq_disable:          caller=enter_from_kernel_mode+0x38 parent=el1_interrupt+0x24

If someone had an alternative parser for trace.dat, they would also be able to witness the difference. However, I'd be really surprised if a tool could cope with instances without handling the top-level one (usually it's more the opposite), and on top of that (almost) everything ended up in the "devlib" instance, so it's not like it was used as some sort of sorting mechanism. And since you could not choose the buffer in the devlib API, people can't have grown fancy use cases.

So if anything, reverting to using the top-level instance means the traces produced by devlib are more "vanilla" and less likely to trip other tools (although most tooling around trace.dat is made by the same people making ftrace, so support for buffers is normally good).

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.

2 participants