Create a small program that, when invoked, will create start and stop
boottrace entries via sysctl, and execute the desired command. As
opposed to three lines of shell script, this utility allows the resource
usage to be correctly calculated when each sysctl is invoked, since the
execution of the desired program (rc script) will be part of the same
process.
Details
Details
- Reviewers
kevans trasz 0mp - Group Reviewers
manpages - Commits
- rG13ec1e3155c7: boottrace(8): small wrapper utility
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 41480 Build 38369: arc lint + arc unit
Event Timeline
| usr.bin/boottrace/boottrace.c | ||
|---|---|---|
| 56 | So if this utility is expected to be called by the RC framework, where do we draw the line between boot times and run times? Here you've chosen the RC scripts are run times, but they are a big part of boot, no? | |
| usr.bin/boottrace/boottrace.c | ||
|---|---|---|
| 56 | I suppose it is up to interpretation, whether the execution of startup scripts is considered part of "boot" or not. That said, I believe the original patch uses the start of multi-user as the separation point, which is reasonable enough. I will update this patch to use BOOTTRACE() instead. | |
| usr.bin/boottrace/boottrace.c | ||
|---|---|---|
| 56 | I am honestly fine either way, just wanted to have the justification documented somewhere. | |
Comment Actions
Log events as BOOTTRACE rather than RUNTTRACE. We will use the switch to multi-user mode as the mark between boot and run-time tracing.
Comment Actions
It would be nice to have some sort of man page, even a short one to briefly describe the purpose of this utility.
| usr.bin/boottrace/boottrace.c | ||
|---|---|---|
| 70 | Shouldn't we return the exit status of the executed utility instead of 0? | |
| usr.bin/boottrace/boottrace.c | ||
|---|---|---|
| 63 | I guess it's probably rare, but should the error be logged via boottrace as well? | |
| 63 | Ah, sorry, I guess it still gets the done trace below; disregard. | |
| 70 | If the idea is that we could expect to be able to arbitrarily throw whatever command under boottrace in isolation, I think you're right that it should maintain that kind of transparency. | |
Comment Actions
I'd also encourage you to run mandoc -Tlint, as you're missing some cross-references.
| usr.bin/boottrace/Makefile | ||
|---|---|---|
| 3 | You also need to install the manual page. | |
| usr.bin/boottrace/boottrace.1 | ||
| 77–85 ↗ | (On Diff #102168) | This part of the Examples subheader is too wide for terminals that are 80 characters in width, which needs to be addressed as all manual pages should be viewable in singleuser mode without any framebuffer in use. |
Comment Actions
The only output I see is:
$ mandoc -Tlint usr.bin/boottrace/boottrace.1 mandoc: usr.bin/boottrace/boottrace.1:33:6: STYLE: referenced manual not found: Xr boottrace 4
That page is in a separate review (D33275). Did you mean something else?
| usr.bin/boottrace/Makefile | ||
|---|---|---|
| 3 | This is the default value inferred by bsd.prog.mk. | |
| usr.bin/boottrace/boottrace.1 | ||
| 77–85 ↗ | (On Diff #102168) | I can shrink it further I think, but do you have suggestions on how this is typically handled? |
| usr.bin/boottrace/Makefile | ||
|---|---|---|
| 3 | I seem to recall having had to add MAN= to other utilities before, because the manual page wasn't installed properly everywhere - but I can't off-hand think of any examples (although I suspect it had something to do with man.cgi on the website?) | |
| usr.bin/boottrace/boottrace.1 | ||
| 77–85 ↗ | (On Diff #102168) | Nothing that's written down - I'd suggest using tmux or screen to create a 80 character wide pane/window and go from there. I don't believe there's anything in the base system that exceeds 80 characters? |
Comment Actions
Just a couple of minor nits. Otherwise, seems ready to land.
BTW, shouldn't boottrace live in /usr/sbin? I've not checked but I guess you need root to write to the sysctl variables anyway. (Then the manual page has to be moved to section 8).
Also, maybe I am missing something, but shouldn't boottrace actually be installed in /sbin? Otherwise, it may not be available until the /usr partition is mounted.
| usr.bin/boottrace/boottrace.1 | ||
|---|---|---|
| 31 ↗ | (On Diff #102327) | Idea: should we mention that it's about tracing time/duration here? |
| 71 ↗ | (On Diff #102327) | If boottrace can only be executed by root, then the prompt should be #. |
| 75 ↗ | (On Diff #102327) | The word "sysctl" should not be stylized with Va. I am attaching a possible fix. |
| 77–85 ↗ | (On Diff #102168) | I think that this is not an issue. With less(1) you can scroll horizontally if needed. Also, the boottrace log just happens to have a lot of columns, which naturally makes it long. Some tools like netstat or ps print a a limited, narrow version of their outputs and allow the user to request a detailed, wide version with a special flag. I don't think we need this kind of functionality for boottrace sysctls. |
| usr.bin/boottrace/boottrace.c | ||
| 5 | Bump to 2022. | |
| 64 | Maybe instead of 1, we could use a higher number like 127 in a similar fashion to timeout(1)? Then there is a lower chance of conflicting exit codes in boottrace and the utility. | |
Comment Actions
Good point, I renamed it to boottrace(8) and placed it under /usr/sbin in the final commit. There are other tools used by rc.subr that depend on /usr, so I think it is fine to do so here too (it can handle the case where the tool cannot be found).
| usr.bin/boottrace/boottrace.c | ||
|---|---|---|
| 64 | Seems to be a common enough pattern to do the following: So, yes I have done this in the committed version. | |