Factor out common kerneldump code into dump_start()/dump_finish().

[markj]

Summary

This is a step towards generic support for kernel dump compression.
dump_start() and dump_finish() are responsible for writing kernel dump
headers, optionally writing the crypto key, and initializing "dumplo",
i.e. the offset into the dump device at which we start writing the
dump's contents.

Also remove the unused dump_write_pad(), and make some functions
static now that they're no longer used outside of kern_shutdown.c.

No functional change intended.

Test Plan

minidumps, full dumps, encrypted minidumps, all on amd64

[def]

sys/kern/kern_shutdown.c

1238

dump_start() returns ENOSPC now while in the past we set error to E2BIG in case of a lack of space on amd64 and arm64. On these architectures in the ENOSPC case we try to create a minidump again.

We should remove E2BIG cases as it's dead code and check in case of ENOSPC if the first kernel dump header was successfully written. I think it doesn't make sense to try to write a crash dump again if we fail at the beginning.

[markj]

sys/kern/kern_shutdown.c

1238

Oops, thanks for catching that.

Note that dump_check_bounds() returns ENOSPC. I think it would be simpler to change this to return E2BIG instead?

[def]

sys/kern/kern_shutdown.c

1218

Can we move it to sys/sys/kerneldump.h and rename to KERNELDUMP_METADATA_SIZE so we can use it also in sys/ddb/ddb/db_textdump.c?

1238

Do you mean to change dump_start() to return E2BIG if this check fails? This would be a good idea. Note that this also requires changes in MD code.

1240

Currently dump_start() modifies dumplop even if it fails. I would introduce a temporary dumplo variable to update *dumplop if dump_start() is successful. It would be less bug prone if we'll want to reuse *dumplop in case of a failure in the future. It would be also consistent with dumplo being updated after each dump_write() call in MD code.

1260

For consistency with dump_start() I would pass a pointer to dumplo to dump_finish() and update it after a kernel dump header is successfully written.

[markj]

sys/kern/kern_shutdown.c

1238

Indeed.

1260

I'm not sure why it's important to be consistent in this way. A dumplop argument would point into a stack frame that's about to be freed anyway. Further, I'll note that the dumplo/dumplo arguments are removed in a later revision.

[markj]

• Address review feedback.

[def]

sys/arm/arm/minidump_machdep.c

345

I was thinking about

else if (error == E2BIG || error == ENOSPC)

here and in other architectures as dump_write() can still return ENOSPC which might happen when a number of pages being dumped has increased during dumping.

sys/kern/kern_shutdown.c

1260

OK.

[markj]

• Combine error handling for E2BIG and ENOSPC.

[def]

sys/amd64/amd64/minidump_machdep.c

444

ENOSPC case is handled before. :)

sys/arm64/arm64/minidump_machdep.c

420

ENOSPC case is handled before.