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

DTrace depends on an obsolete debugfs mount point #111

Open
nickalcock opened this issue Nov 6, 2024 · 3 comments
Open

DTrace depends on an obsolete debugfs mount point #111

nickalcock opened this issue Nov 6, 2024 · 3 comments
Assignees
Labels
local-review Fix not yet publically visible, but coming

Comments

@nickalcock
Copy link
Member

The old /sys/kernel/debug/tracing mount point for the tracefs has been obsolete for some time (not sure how long, sometime between kernel 4.3 and kernel 6.1). The new location is /sys/kernel/tracing, but some installations don't mount it yet so debugfs automounts the tracefs under the old location in that case: it even seems to be possible to get both directories, or to have both directories but one of them is empty. This means we have to cope with a debugfs that might be on /sys/kernel/debug/tracing or /sys/kernel/tracing on different systems or even on the same system at different times.

We should dynamically adapt to such things.

@nickalcock nickalcock self-assigned this Nov 6, 2024
@nickalcock nickalcock added the local-review Fix not yet publically visible, but coming label Nov 6, 2024
@nickalcock
Copy link
Member Author

Implemented a fix, under test now.

nickalcock added a commit that referenced this issue Nov 6, 2024
So as of kernel 6.3 (upstream commit 2455f0e124d317dd08d337a75), the
canonical tracefs location has moved to /sys/kernel/tracing.  Unfortunately
this requires an explicit mount, and a great many installations have not
done this and perhaps never will.  So if the debugfs is mounted on
/sys/kernel/debug, it automatically makes /sys/kernel/debug/tracing appear
as it used to, as another tracefs mount. And of course there's nothing
special about the "canonical location": it's up to the admin, who might
choose to remount the thing anywhere at all.

To make this even more fun, it's quite possible to end up with the tracefs
on /sys/kernel/debug/tracing, but an empty directory at /sys/kernel/tracing
(I got that during testing with no effort at all).

All this means that the existing DTrace hardwiring for tracefs/eventsfs
locations isn't good enough. Instead, hunt for a suitable tracefs mount with
getmntent(), and add a function to return an arbitrary path under that
directory (mimicking the things we used to do with EVENTSFS defines and the
like).  Return a pointer to static (per-dtrace_hdl) storage so that we don't
need to clog up all the callers with dynamic allocation: only one path is
ever needed at any one time anyway.

Tested with both in-practice-observed locations of debugfs.

Bug: #111
Signed-off-by: Nick Alcock <[email protected]>
@nickalcock
Copy link
Member Author

Pushed for experimentation to the nix/movable-tracefs branch (not yet reviewed, may do anything!)

nickalcock added a commit that referenced this issue Nov 11, 2024
So as of kernel 6.3 (upstream commit 2455f0e124d317dd08d337a75), the
canonical tracefs location has moved to /sys/kernel/tracing.  Unfortunately
this requires an explicit mount, and a great many installations have not
done this and perhaps never will.  So if the debugfs is mounted on
/sys/kernel/debug, it automatically makes /sys/kernel/debug/tracing appear
as it used to, as another tracefs mount.  And of course there's nothing
special about the "canonical location": it's up to the admin, who might
choose to remount the thing anywhere at all.

To make this even more fun, it's quite possible to end up with the tracefs
on /sys/kernel/debug/tracing, but an empty directory at /sys/kernel/tracing
(I got that during testing with no effort at all).

All this means that the existing DTrace hardwiring for tracefs/eventsfs
locations isn't good enough.  Instead, hunt for a suitable tracefs mount with
getmntent(), and add a function to open files under that directory, allowing
the path to be created using a printf-style format string (mimicking the
things we used to do with EVENTSFS defines and the like).  This is actually
all we need; there is no need to ever return these paths at all, so there
is no clogging up the code with free()s -- and actually there's a
noticeable simplification in most cases.

Tested with both in-practice-observed locations of debugfs, and the
obviously crazy and bad-in-a-format-string path of "/%s/%s/%n" to make sure
that is properly rejected.

Bug: #111
Signed-off-by: Nick Alcock <[email protected]>
@nickalcock
Copy link
Member Author

Revised thing pushed -- almost totally different, courtesy of a really good review from Kris. Still seems to work plus is much better :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
local-review Fix not yet publically visible, but coming
Projects
None yet
Development

No branches or pull requests

1 participant