tty info (^T): Add optional kernel stack(9) traces
ClosedPublic
Actions

Authored by cem on Oct 19 2018, 10:26 PM.

Details

Reviewers

ed
bdrewery
markj
oshogbo
trasz

Commits

rS339471: tty info (^T): Add optional kernel stack(9) traces

Summary

It is often useful for developers and administrators to determine a running
thread's stack for debugging purposes. With this feature, using ^T will
print that information

For now, the feature is disabled by default. Enable with sysctl
kern.tty_info_kstacks=1.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

cem created this revision.Oct 19 2018, 10:26 PM

Harbormaster completed remote builds in B20298: Diff 49323.Oct 19 2018, 10:26 PM

cem added a parent revision: D17620: Add flags variants to linker_files / stack(9) symbol resolution.Oct 19 2018, 10:30 PM

LGTM.

This revision is now accepted and ready to land.Oct 19 2018, 10:47 PM

ed added inline comments.Oct 20 2018, 10:50 AM

sys/kern/tty_info.c
234 ↗	(On Diff #49323)	`sbuf` has its own built-in printf logic, right? If so: would it make sense to refactor this code to use `sbuf` entirely? The interesting thing about doing that would be that afterwards, `ttyprintf()` would only be called to write raw, pre-formatted buffers of data to the TTY. This can be done more efficiently, without calling into `subr_prf.c` at all.

cem marked 3 inline comments as done.Oct 20 2018, 1:32 PM

cem added inline comments.

sys/kern/tty_info.c
234 ↗	(On Diff #49323)	It has it, yep. I guess I didn't realize this function was the only in-tree ttyprintf consumer. Given that, yeah, I agree it makes a lot of sense to rip out ttyprintf, convert this code to sbuf, and use the more efficient tty plain string-writing routine.
sys/sys/tty.h
138 ↗	(On Diff #49323)	@markj suggests we might just put this on the stack. I don't see any public stack_init KPI that doesn't allocate, but stack_create() is just an M_ZERO malloc, so perhaps zero-initialization would be adequate.
144 ↗	(On Diff #49323)	I'm less sure about dumping this one on the stack, especially given that PRINTF_BUFR_SIZE is somewhat flexible.

cem added a parent revision: D17624: Replace ttyprintf with sbuf_printf and tty drain routine.Oct 20 2018, 3:06 PM

Rebase on D17624

This revision now requires review to proceed.Oct 20 2018, 3:08 PM

Harbormaster completed remote builds in B20304: Diff 49332.Oct 20 2018, 3:08 PM

cem marked an inline comment as done.Oct 20 2018, 3:09 PM

cem marked an inline comment as done.

cem added inline comments.

sys/kern/tty_info.c
334–338 ↗	(On Diff #49332)	The stack(9) interface is a bit obnoxious here -- I'd like to be able to just invoke a single int-returning function, maybe with flags to allow grabbing a running stack, and not worry about checking SWAPPED/RUNNING or setting error to zero myself. But that is orthogonal to this enhancement.

markj added inline comments.Oct 20 2018, 5:08 PM

sys/kern/tty_info.c
333 ↗	(On Diff #49332)	Use stack_zero()?
334–338 ↗	(On Diff #49332)	I think I did it that way because each platform needs its own implementation and I only wanted to do it for x86. I don't disagree with the sentiment.