Paths

Table of Contentst

Differential D39132

panic(9): some updates
ClosedPublic
Actions

Authored by mhorne on Mar 16 2023, 7:36 PM.

Tags

None

Referenced Files

	F102651630: D39132.diff
	Fri, Nov 15, 8:58 AM

	F102641140: D39132.id118947.diff
	Fri, Nov 15, 5:32 AM

	Unknown Object (File)
	Oct 16 2024, 5:39 AM

	Unknown Object (File)
	Oct 15 2024, 11:22 AM

	Unknown Object (File)
	Oct 15 2024, 11:22 AM

	Unknown Object (File)
	Oct 14 2024, 4:08 AM

	Unknown Object (File)
	Oct 12 2024, 8:54 PM

	Unknown Object (File)
	Oct 11 2024, 7:05 PM

View All 70 Files

Subscribers

Details

Reviewers

markj
kib
jhb
rpokala

Group Reviewers

Commits

rG86e2335d10e5: panic(9): some updates
rGd5e105bf7e4c: panic(9): some updates

Summary

Correct the include headers
Better description of what the panic() function does
Document KERNEL_PANICKED()
Add a section describing panic execution context
Add SEE ALSO

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 50425
Build 47316: arc lint + arc unit

Event Timeline

mhorne created this revision.Mar 16 2023, 7:36 PM

Herald added a subscriber: imp. · View Herald TranscriptMar 16 2023, 7:36 PM

mhorne requested review of this revision.Mar 16 2023, 7:36 PM

Harbormaster completed remote builds in B50425: Diff 118947.Mar 16 2023, 7:36 PM

mhorne added a parent revision: D39131: KASSERT(9): some updates.Mar 16 2023, 7:36 PM

mhorne added a child revision: D39133: kern_reboot(9): some updates.

mhorne added reviewers: markj, kib, jhb.Mar 16 2023, 7:39 PM

markj added inline comments.Mar 16 2023, 8:04 PM

share/man/man9/panic.9
73	I would just say that it disables interrupts. spinlock_enter() just happens to be the name of the MI function used for that purpose.
87	It'd be worth mentioning `debugger_on_panic` IMO.
96	Maybe "called recursively" would be more clear?
124
128

rpokala added a subscriber: rpokala.Mar 16 2023, 8:19 PM

rpokala added inline comments.

share/man/man9/panic.9
73	When .Fn panic or .Fn vpanic is entered, the calling thread takes its spinlock.
80	In the unlikely event of concurrent panics, only one panicking thread will proceed.
93	is called to restart the system, attempting a kernel dump in the process.

Handle feedback.

Harbormaster completed remote builds in B50435: Diff 118958.Mar 16 2023, 9:33 PM

mhorne added inline comments.Mar 16 2023, 9:35 PM

share/man/man9/panic.9
73	@rpokala I added a short paragraph that clarifies vpanic(), but I prefer to keep mentions of it out of the main body of the description. @markj yes, I thought that name was somewhat suspect.

rpokala accepted this revision.Mar 16 2023, 10:04 PM

This revision is now accepted and ready to land.Mar 16 2023, 10:04 PM

kib added inline comments.Mar 17 2023, 7:11 AM

share/man/man9/panic.9
48–49	Again, traditionally it was sys/systm.h, and we should offer either of the headers as the correct one.
77	I believe the 'first time' is somewhat confusing. The situation you describe is actually the concurrent calls.

Handle kib's comments.

This revision now requires review to proceed.Mar 17 2023, 6:24 PM

Harbormaster completed remote builds in B50449: Diff 119026.Mar 17 2023, 6:24 PM

kib accepted this revision.Mar 17 2023, 8:00 PM

This revision is now accepted and ready to land.Mar 17 2023, 8:00 PM

rpokala accepted this revision.Mar 18 2023, 12:33 AM

markj accepted this revision.Mar 18 2023, 2:45 PM

markj added inline comments.

share/man/man9/panic.9
71–88
74	Not related to the patch, just a general question: why do spinlock_enter()/_exit() enter a critical section at all? I would expect disabling interrupts to have the same effect, at least in non-INVARIANTS kernels.

kib added inline comments.Mar 18 2023, 3:32 PM

share/man/man9/panic.9
74	To prevent context switches in the code executed under spinlock. As an example, imagine that some code took a spinlock, then does critical_enter()/critical_exit(). If owe_preempt is set, we end up trying to reschedule.

markj added inline comments.Mar 18 2023, 3:39 PM

share/man/man9/panic.9
74	Oops, of course. Thanks.

Closed by commit rGd5e105bf7e4c: panic(9): some updates (authored by mhorne). · Explain WhyMar 20 2023, 8:13 PM

This revision was automatically updated to reflect the committed changes.

mhorne marked an inline comment as done.

mhorne added a commit: rGd5e105bf7e4c: panic(9): some updates.

mhorne added a commit: rG86e2335d10e5: panic(9): some updates.Mar 27 2023, 5:22 PM

Revision Contents
Changeset List

Path

Size

share/

man/

man9/

3 lines

100 lines

Diff 118947

share/man/man9/Makefile

Loading...

share/man/man9/panic.9

Loading...