Skip to content

ftrace_log: Add override for tracefs location#126

Open
credp wants to merge 2 commits intoscheduler-tools:masterfrom
credp:tracefs_path_set
Open

ftrace_log: Add override for tracefs location#126
credp wants to merge 2 commits intoscheduler-tools:masterfrom
credp:tracefs_path_set

Conversation

@credp
Copy link
Contributor

@credp credp commented Jul 6, 2022

It's possible to mount tracefs in other locations - the Google Pixel 6,
for example, mounts tracefs at /sys/kernel/tracing rather than the
typical location inside debugfs at /sys/kernel/debug/tracing and disallows
debugfs from being mounted when running any GKI kernel.

This change allows configurations to select the location where rt-app
will attempt to configure tracing when enabled. If it is not supplied,
then the intended behaviour is that the existing default should be used.

There should not be any impact on existing json files running on existing
systems.

Signed-off-by: "Chris Redpath" chris.redpath@arm.com

It's possible to mount tracefs in other locations - the Google Pixel 6,
for example, mounts tracefs at /sys/kernel/tracing rather than the
typical location inside debugfs at /sys/kernel/debug/tracing and disallows
debugfs from being mounted when running any GKI kernel.

This change allows configurations to select the location where rt-app
will attempt to configure tracing when enabled. If it is not supplied,
then the intended behaviour is that the existing default should be used.

There should not be any impact on existing json files running on existing
systems.

Signed-off-by: "Chris Redpath" <chris.redpath@arm.com>
@credp credp force-pushed the tracefs_path_set branch 3 times, most recently from c5ff4f0 to 282918a Compare July 13, 2022 19:02
@credp credp requested a review from vingu-linaro July 13, 2022 19:03
@credp
Copy link
Contributor Author

credp commented Jul 13, 2022

@DouglasRaillard Here you go - it's tested on my VM but the P6 will be a bit longer as I am starting from scratch on it again.

@douglas-raillard-arm
Copy link

@credp it builds but there is a warning to fix:

rt-app_parse_config.c:1210:33: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
 1210 |                 opts->ftracedir = ftrace_dir();
      |                                 ^

@credp
Copy link
Contributor Author

credp commented Jul 14, 2022

@credp it builds but there is a warning to fix:

rt-app_parse_config.c:1210:33: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
 1210 |                 opts->ftracedir = ftrace_dir();
      |                                 ^

Ooh, thanks. I changed the ftrace_dir to use strdup instead of using a static string buffer and forgot to fix this bit up. Will repost

@credp credp force-pushed the tracefs_path_set branch from 282918a to 2800b8e Compare July 14, 2022 09:57
@credp credp force-pushed the tracefs_path_set branch from 2800b8e to 463123a Compare July 14, 2022 10:41
If you don't provide a tracing location in the global config key section,
rt-app usually looks in /sys/kernel/debug for the tracing folder.

As part of this, switch the internals to use the full tracing path
instead of relying upon adding /tracing/ to the path of files used.

This patch allows rt-app to examine the content of /etc/mtab, and apply
the following ordering:

* If tracefs is mounted, use that location
* If tracefs is not found anywhere, then look for debugfs
  - If we use debugfs, append "/tracing" to the path
* If we find neither, just use /sys/kernel/debug/tracing

Using the default means we couldn't find tracefs or debugfs mounted
and so is likely to fail later if you are using ftrace, but should have
no impact if you're not tracing.

All of the path selection is overridden if you provide the config key.

This should allow rt-app to work on any platform, at least from tracing
POV.

Signed-off-by: Chris Redpath <chris.redpath@arm.com>
@credp credp force-pushed the tracefs_path_set branch from 463123a to 7f2a4c2 Compare July 14, 2022 10:56
return tmp;
}

return strdup(DEFAULT_FTRACE_DIR);
Copy link
Member

Choose a reason for hiding this comment

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

do we really need to return a default path ? there is no chance that this can work, isn't ?

Should be better return null and escape early in the main thread

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I return NULL here, it means that if the mtab code fails we cannot use ftrace unless we provide a path in the global section. That's probably OK, I don't mind that if you dont.

Copy link
Member

Choose a reason for hiding this comment

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

I wondering if there is a usecase where mtab would fail but the default path could work

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 don't think so, and if that does happen you can always add ftracedir to your global options file to force it to a specific location. I'll push a new squashed rev up once I've tested it a bit.

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