Page MenuHomeFreeBSD

Split out dumper allocation from list insertion
ClosedPublic

Authored by mhorne on Jan 27 2022, 9:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 7:00 PM
Unknown Object (File)
Feb 12 2024, 8:21 AM
Unknown Object (File)
Feb 12 2024, 8:21 AM
Unknown Object (File)
Feb 12 2024, 8:21 AM
Unknown Object (File)
Feb 12 2024, 8:20 AM
Unknown Object (File)
Feb 12 2024, 12:11 AM
Unknown Object (File)
Jan 14 2024, 8:15 AM
Unknown Object (File)
Jan 12 2024, 10:12 AM

Details

Summary

Add a new function, dumper_create(), to allocate a dumper.
dumper_insert() will call this function and retains the existing
behaviour.

This is desirable for performing live dumps of the system. Here, there
is a need to allocate and configure a dumper structure that is invoked
outside of the typical debugger context. Therefore, it should be
excluded from the list of panic-time dumpers.

free_single_dumper() is made public and renamed to dumper_destroy().

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/kern/kern_shutdown.c
1265

This is very unusual to allocate the item and change the output argument, despite returning an error. The requirement that you must

  • set the *dip value to NULL before call
  • call dumper_destroy() regardless of the return value

are error-prone IMO.

Make dumper_create() handle its own cleanup in failure cases.

sys/kern/kern_shutdown.c
1265

Yeah, I clearly did not look this over carefully enough before posting. Even D34069 did not expect to call dumper_destroy() in the failure case.

The new version should handle things more sensibly, although I did not understand your comment about the requirement that *dip be set to NULL.

kib added inline comments.
sys/kern/kern_shutdown.c
1265

To reliably detect the need for cleanup after failure in the old code, you have to set *dip to NULL before calling dumper_create(). Otherwise you cannot distinguish case of error before allocation vs. error after.

This revision is now accepted and ready to land.Feb 3 2022, 1:53 PM
markj added inline comments.
sys/kern/kern_shutdown.c
1243

I wouldn't bother checking for dip == NULL if it only happens due to programmer error.