Page MenuHomeFreeBSD

minidump: De-duplicate is_dumpable()
ClosedPublic

Authored by mhorne on Sep 8 2021, 6:51 PM.
Tags
None
Referenced Files
F163242734: D31884.id95967.diff
Tue, Jul 21, 8:46 AM
F163209855: D31884.id94902.diff
Tue, Jul 21, 2:15 AM
F163204787: D31884.id95474.diff
Tue, Jul 21, 1:09 AM
F163200347: D31884.id94921.diff
Tue, Jul 21, 12:12 AM
Unknown Object (File)
Sat, Jul 18, 2:14 AM
Unknown Object (File)
Mon, Jul 13, 1:32 AM
Unknown Object (File)
Sun, Jul 12, 8:21 PM
Unknown Object (File)
Mon, Jul 6, 1:05 PM

Details

Summary

The function is identical in each minidump implementation, and it is
small, so move it to the vm_dumpset.h header. The only slight exception
is powerpc where the function is public, for use in moea64_scan_pmap().

Test Plan

Will tinderbox.

Diff Detail

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

Event Timeline

mhorne requested review of this revision.Sep 8 2021, 6:51 PM

Looks identical modulo int->bool changes, which don't matter.

This revision is now accepted and ready to land.Sep 8 2021, 6:55 PM
mhorne added reviewers: kib, markj.

This is a good cleanup. I don't really like the header pollution. We are already assuming that consumers have included vm_page.h (to check m->flags & PG_NODUMP), so why not vm_phys.h as well? Alternately we could have the implementation live in vm_phys.c and call it vm_phys_is_dumpable() or so.

This is a good cleanup. I don't really like the header pollution. We are already assuming that consumers have included vm_page.h (to check m->flags & PG_NODUMP), so why not vm_phys.h as well? Alternately we could have the implementation live in vm_phys.c and call it vm_phys_is_dumpable() or so.

Sure, I will drop the include from this header. I considered vm_phys.c as well, but currently nothing in that file touches dump_avail, so this seems the more natural place.

Drop include of vm_phys.h from vm_dumpset.h. Include it where it is needed instead.

This revision now requires review to proceed.Sep 9 2021, 2:51 PM

I am fine with this as is, but I think my minor preference is to have the function moved to vm_phys.c.

This revision is now accepted and ready to land.Sep 9 2021, 3:52 PM

I suspect vm_dumpset.h should be wrapped in #ifdef _KERNEL as well, just in case some other header starts including it.

Move function to vm_phys.c, rename it to vm_phys_is_dumpable().

This revision now requires review to proceed.Sep 21 2021, 6:53 PM
This revision is now accepted and ready to land.Sep 22 2021, 3:26 PM
This revision was automatically updated to reflect the committed changes.