Page MenuHomeFreeBSD

malloc(9): extend contigmalloc(9) by a "slab cookie"
Needs ReviewPublic

Authored by bz on Sun, Jun 30, 7:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jul 1, 9:33 PM
Unknown Object (File)
Mon, Jul 1, 3:36 PM

Details

Reviewers
jhb
markj
kib
Summary

Extend kern_malloc.c internals to also cover contigmalloc(9) by a
"slab cookie" and not just malloc/malloc_large. This allows us to
call free(9) even on contigmalloc(9) addresses.
Extend the contigmalloc(9) man page with a small additional note that
free(9) also works now.

The implementation also adds additional chacks now under INVARIANTS;
e.g. validity of a cookie (allocation type) in various places as well
as size and allocation type checks for contigfree(9).

The way this is done (free working for contigmalloc) will hide the
UMA/VM bits from a consumer which may otherwise need to know whether
the original allocation was by malloc or contigmalloc by looking at
the cookie (likely via an accessor function). This simplifies
the implementation of consumers of mixed environments a lot.

This is preliminary work to allow LinuxKPI to be adjusted to better
play by the rules Linux puts out for various allocations.
Most of this was described/explained to me by jhb.

One may observe that realloc(9) is currently unchanged (and contrary
to [contig]malloc/[contig]free an implementation may need access
the "slab cookie" information given it will likely be implementation
dependent which allocation to use if size changes beyond the usable
size of the initial allocation).

Described by: jhb
Sponsored by: The FreeBSD Foundation
MFC after: 3 days

Test Plan

I've been running with this + LinuxKPI changes for a good month
and a bit now.
I wonder whom to ask to micro-benchmark this?

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 58429
Build 55317: arc lint + arc unit

Event Timeline

bz requested review of this revision.Sun, Jun 30, 7:27 PM
share/man/man9/contigmalloc.9
121

Can't we always use free() now? If so, I would prefer to eliminate contigfree().

sys/kern/kern_malloc.c
468

KASSERT strings don't need a newline.

496

This line is too long. It would be good to introduce a macro to provide this cookie instead of manually constructing it.

543

Panic strings shouldn't end with a newline or a period. I see a couple of preexisting strings which have them but that's just a bug.

977

If you introduce a macro to extract the cookie type, the code will be a bit neater and then you don't need the slab_cookie helper variable.

978

It would be preferable to keep the branch hint. I believe you can annotate case statements the same way, i.e., case __predict_true(SLAB_COOKIE_SLAB_PTR):.

1051

There's no need to do anything special for KASAN here, it'll be handled in the kmem_* layer.

1216

I'd suggest simply replacing this case with __assert_unreachable();. You lose the panic string, but the info there is not very useful anyway.