Page MenuHomeFreeBSD

Add a manual page for boottrace
ClosedPublic

Authored by 0mp on Dec 5 2021, 5:28 PM.

Diff Detail

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

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
share/man/man4/boottrace.4
52

"holistic" may confuse EFL readers (it sent me to MW to check its exact meaning).

63

Taste (minor): I'd use "fills up" or "gets full".

64

Nitpick: I think panics happen more often than they're (user-)initiated. If you mean both, perhaps "Upon initiating a shutdown or reboot, or if a kernel panic occurs or is requested by the user, ..."

66

As the amateur sysadmin for my own computer, my vote is for "yes".

80

(Or whichever value(s) will enable tracing.)

85

Do you mean "Return" (to userland) here? Or send to syslog?

102
104

Not an actual edit suggestion, but I'm curious: why does that only exist for shutdown?

128

Do you mean "userland" here, at any stage including shutdown? Or the "init+rc" stage only?

173

This looks out of place here to me. Am I missing something?

201

Or "for them".

Thank you very much @0mp for picking this up.

Would it be an idea to refer to this manual page from boot(9), boot(8), and/or some other relevant manual pages - to make it easier to discover?

It would be an idea, a good one in fact :)

Note that boot(9) is now kern_reboot(9).

share/man/man4/boottrace.4
34

This is the current intended semantic of 'run-time' events; everything logged after the completion of rc scripts (or to be precise, when init(8) transitions to multi-user) but before shutdown has begun. For example, loading a kmod on the command line will create an event in the run-time event buffer.

We consider execution of rc scripts to be part of 'boot-time', but the choice is somewhat arbitrary.

50

It was mentioned a few lines above that rc(8) is covered by boottrace.

75

The format of which part was different? I can clarify any specific bits which there is confusion about.

189

I don't think this section adds much, at least I would want to re-write the text comparing boottrace to tslog and I wouldn't classify it as a 'bug'.

With https://reviews.freebsd.org/D33275#inline-206447 in mind, why is it called "boottrace" specifically and not something that conveys more accurately which stages it covers?

share/man/man4/boottrace.4
202

Maybe "completely" or "mostly" instead depending on its amount of self-containedness?

With https://reviews.freebsd.org/D33275#inline-206447 in mind, why is it called "boottrace" specifically and not something that conveys more accurately which stages it covers?

I am not the original author, so I cannot say exactly how the name was decided upon. It likely comes from the motivation of being able to trace the boot process, but the feature was extended to shutdown and some run-time events, possibly after the fact.

Since shutdown is the converse of system boot, I don't think it's counterintuitive that it would be covered by a feature named 'boottrace', but I agree that it's not obvious from the name alone. I understand the desire for a name that properly encapsulates all of the functionality, but I cannot think of one that does this without becoming more vague. Unless someone can propose a name that is clearly superior, I will not go through the pain of such a large rename.

share/man/man4/boottrace.4
113

This variable is of no interest to end users, I suggest it be dropped from the man page. The only reason to use it is if one was working on the boottrace code, at which point they would find the tunable anyway.

0mp marked 11 inline comments as done.

Address most of the comments

0mp marked 2 inline comments as done and an inline comment as not done.Feb 2 2022, 12:43 PM
0mp added inline comments.
share/man/man4/boottrace.4
34

I'm marking this discussion as resolved for now. The semantic seems to be good enough.

50

I think that it is alright to add some more details here. We didn't mention the event annotations earlier. I thought about making it a bit concise for a moment but I didn't find a better way to express it.

64

I'd not go into details of panics here. This should be covered in a separate document explaining what a kernel panic is and how it can be occur.

I agree that "upon initialization" may sound like a user-initiated action. I cannot think of a better way to write this sentence, though.

Please let me know if you have anything on your mind.

66

Makes sense. Also, many other many other manpages do that, eg. vte(4).

173

It was a part of an old template. I removed it.

I made some comments here - but it looks to me like both my comments have or will be addressed shortly?

  • Remove the BUGS section. We just need to create a TSLOG page at some point. Differences between TSLOG and boottrace should be mentioned in a commit message of boottrace.
  • Improve formatting

Manual page LGTM at this stage - will rereview if it changes.

0mp marked 9 inline comments as done.Feb 17 2022, 5:39 PM
0mp added inline comments.
share/man/man4/boottrace.4
75

I cannot find that anymore. I think I was confusing the ${procname}: name format used by boottrace somewhere internally I suppose and the format of kern.boottrace.log.

104

That's a good question. I'll ask @mhorne about it.

113

Done!

128

I'll ask @mhorne about it.

189

I removed it. We should have a dedicated manual page for comparison of tracing facilities. Also, we need a tslog manual page.

0mp retitled this revision from WIP: Add a manual page for boottrace to Add a manual page for boottrace.Feb 18 2022, 8:29 AM
share/man/man4/boottrace.4
33–34

This seems like an unusual thing to state in the synopsis. Other pages simply list the relevant include file, sys/boottrace.h in this case. See watchdog(4), ng_split(4), tty(4) for other non-optional components that have this section.

69

It actually takes over when the system has completed booting (including rc execution) and init(8) transitions to multi-user mode.

106

Something like this to describe the expected format.

128

It logs everything that has been recorded in the boot-time and run-time event tables (whether the kernel or userspace generated the event). It doesn't include shutdown events, because this sysctl is not expected to be invoked at any point during shutdown. There is the kern.boottrace.shutdown_trace option to have those events printed before the system performs its final shutdown action(s).

156

@debdrup raised the point in D31929 that we should not exceed 80 characters. I was able to squish the output there to make it fit, but not sure what should be done here...

187

I think this should reference sysctl(8) instead. Also add an xref for boottrace(1).

share/man/man4/boottrace.4
156

@debdrup raised the point in D31929 that we should not exceed 80 characters. I was able to squish the output there to make it fit, but not sure what should be done here...

There's a lot of empty space that can be elided, which should make it possible to fit within the 80 character limit.

This revision is now accepted and ready to land.Feb 19 2022, 2:06 AM
0mp marked 12 inline comments as done.Feb 20 2022, 11:39 PM

I'll post an updated patch in a moment.

share/man/man4/boottrace.4
33–34

Good idea. I'm moving this part to the description section and adding that include statement.

69

Thanks. I'll include that in a patch shortly.

106

Oh I get it now. I'll add this bit of information to the manual.

156

I assume that this is the exact output format of that sysctl. I'd prefer to stay as close to the real output as possible.

However, if @mhorne managed to make the output of this sysctl narrower, then we should definitely update this example to reflect those changes.

187

good idea

0mp marked 5 inline comments as done.
  • add sys/boottrace.h to the synopsis
  • move the information that boottrace is unconditionally compiled into the kernel out from the synopsis
  • improve descriptions of event tables
  • explain the expected format of the string passed to the kern.boottrace.boottrace sysctl
  • add an example of how using "sysctl kern.boottrace.boottrace" to create events
  • update the See Also section
This revision now requires review to proceed.Feb 20 2022, 11:44 PM
  • Note that the process name is inferred also when the process name is empty.

Other than that question, LGTM.

share/man/man4/boottrace.4
121

Clarification request: is that the executable name, or the title set by setproctitle() or similar means?

share/man/man4/boottrace.4
121

Good question, I'll look into that.

  • Add a bit more details where the process name comes from
0mp marked an inline comment as done.Feb 28 2022, 12:13 PM
0mp added inline comments.
share/man/man4/boottrace.4
121

It is the executable name. See getprogname(3) for details.

mhorne added inline comments.
share/man/man4/boottrace.4
121

It is always the executable name, as neither setprogname() nor setproctitle() change the process's name from the kernel's POV.

208

It is now boottrace(8) :)

This revision is now accepted and ready to land.Feb 28 2022, 4:37 PM

With the qualifier removed and the manual page section changed, approving.

share/man/man4/boottrace.4
122

Remove misleading qualifier following mhorne's comment.

208
0mp marked 4 inline comments as done.
  • Update a description related to the process name
  • Update SEE ALSO
This revision now requires review to proceed.Feb 28 2022, 9:49 PM
share/man/man4/boottrace.4
121

Ach, I didn't know about that. Thanks!

This revision was not accepted when it landed; it landed in state Needs Review.Feb 28 2022, 9:52 PM
This revision was automatically updated to reflect the committed changes.