Page MenuHomeFreeBSD

Add support of 4Kn kernel dumps.
ClosedPublic

Authored by cem on Apr 6 2016, 3:38 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 5, 10:25 AM
Unknown Object (File)
Tue, Mar 5, 10:25 AM
Unknown Object (File)
Feb 22 2024, 12:03 AM
Unknown Object (File)
Jan 22 2024, 6:31 AM
Unknown Object (File)
Jan 22 2024, 3:02 AM
Unknown Object (File)
Dec 29 2023, 1:40 AM
Unknown Object (File)
Dec 29 2023, 1:40 AM
Unknown Object (File)
Dec 24 2023, 5:02 PM

Details

Summary

Make sure all I/O is of sector size of the dump dev. The kernel dump header
is 512 bytes and assumed that would fit in sector but will be a partial
sector on a 4Kn disk. Introducing a new function dump_write_pad that
will write out a sector with the contents of the header padded with zeros
to the size of a sector. For now assume the max size is 4096. Using
malloc to allocate memory causes issues with:
Dump map grown while dumping. Retrying...

Update savecore to deal with 4Kn dumps.

Fix mrsas(4) so dumping works and temporarily fix mrsas(4) to boot
by removing the tsleep and replace it with DELAY in mrsas_complete_cmd
so booting doesn't hang.

Test Plan

Tested on amd64 with and without minidumps to 4Kn disks and 512 disks.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 3274
Build 3308: arc lint + arc unit

Event Timeline

ambrisko retitled this revision from to Add support of 4Kn kernel dumps..
ambrisko updated this object.
ambrisko edited the test plan for this revision. (Show Details)

We talked about a CTASSERT for some assumption over email. Was that not needed?

sys/kern/kern_dump.c
62–63

This is fine for now. I think we could also malloc a buffer (at runtime, not during panic/dump) based on the observed di->blocksize when a dumper is registered.

126–127

We can move this out of the loop.

169

sz and size is potentially confusing :).

202

Does savecore really support page sizes that require padding relative to the native block size? I would just leave this as dump_write() for now.

sys/kern/kern_shutdown.c
895

I don't like having a large buf like this on the kernel stack, especially during panic/dump. For now, adding static would be fine. We can switch to using malloc memory, described above, in a later revision (if we care).

897

*size = roundup(length, di->blocksize);

904

As a minor optimization, we only need to zero (temp + length, sizeof(buf) - length).

I agree with all Conrad's comments. In addition, I have a few of my own:

sbin/savecore/savecore.c
496

"sizeof(kdhl)"

style(9): "sizeof's are written with parenthesis always."

Same issue in other places too.

sys/amd64/amd64/minidump_machdep.c
59

Shouldn't 'di' be passed in as an argument to DEV_ALIGN()?

sys/kern/kern_dump.c
59

These defines are duplicated in sys/amd64/amd64/minidump_machdep.c (and i386). They should probably be consolidated in a header somewhere; perhaps 'sys/x86/include/dump.h'?

cem requested changes to this revision.Apr 14 2016, 7:08 PM
cem edited edge metadata.

Ping :-). I'm happy to make these changes, run it by Rpokala again, and get this committed in time for 11.0 if you're busy, Doug. Thanks!

This revision now requires changes to proceed.Apr 14 2016, 7:08 PM
cem edited reviewers, added: ambrisko; removed: cem.
cem added inline comments.
sys/amd64/amd64/minidump_machdep.c
59

Neither of these macros is actually used in minidump_machdep; I just removed them.

sys/kern/kern_dump.c
59

They aren't used in minidump — I just removed them. They both seem to be a duplicate of roundup2() though.

DEV_ALIGN(x) = roundup2(x, di->blocksize);
MD_ALIGN(x) = roundup2(x, PAGE_SIZE); // PAGE_MASK = PAGE_SIZE-1

cem edited edge metadata.

Apply my and Rpokala's suggestions.

Replace static 4096 char 'buffer' with a malloc(di->blocksize), kept in a new
dumperinfo .blockbuf member (allocated at runtime, not dump time).

Apply a few minor cleanups in areas I was touching (removing unused macros from
minidump_machdep, some bzero/bcopy to memset/memcpy conversion, making
whitespace consistent in conf.h, some style(9) nits). Got rid of DEV_ALIGN
entirely (used only once) from kern_dump.c.

cem marked 8 inline comments as done.Apr 14 2016, 10:07 PM

One thing I forgot to note— I removed the mrsas(4) changes from this diff. I think they should be committed separately.

I've tested the new patch on a normal 512-byte hdd. savecore(8) works for minidumps and full dumps; KGDB seems to understand both kinds of core just fine.

rpokala edited edge metadata.

I just performed the following procedure:

  1. % dumpon ${DUMPDEV}
  2. sysctl debug.minidump=${MINIDUMP}
  3. break into debugger
  4. db> call doadump
  5. db> continue
  6. % savecore . ${DUMPDEV}
  7. % kgdb -q /boot/kernel/kernel vmcore.${N}
  8. (kgdb) bt

There are four cases:

  1. DUMPDEV is 512n; MINIDUMP=1
  2. DUMPDEV is 512n; MINIDUMP=0
  3. DUMPDEV is 4Kn; MINIDUMP=1
  4. DUMPDEV is 4Kn; MINIDUMP=0

In almost all cases, `kgdb' was able to extract a reasonable backtrace from the vmcore. The exception was 512n/MINIDUMP=0, which was untested due to insufficient space for a full dump.

Commit it! :-)

This revision is now accepted and ready to land.Apr 15 2016, 5:08 PM
This revision was automatically updated to reflect the committed changes.