LinuxKPI: implement krealloc() for memory being contiguous
ClosedPublic
Actions

Authored by bz on Sat, Mar 29, 6:34 PM.

Details

Reviewers

jhb
markj

Group Reviewers

linuxkpi

Commits

rG516eb937b274: LinuxKPI: implement krealloc() for memory being contiguous
rG580c891f5add: malloc: extend malloc_usable_size() for contigmalloc
rG1c95d401ebe5: LinuxKPI: implement krealloc() for memory being contiguous
rGc5cf4b64f4a9: malloc: extend malloc_usable_size() for contigmalloc

Summary

Extend malloc_usable_size() for contigmalloc; it seems the only
outside consumer is LinuxKPI ksize() which by itself has little to
no consumer either.

Implement krealloc_array() using krealloc(). Implement krealloc()
doing the various size checks ourselves and use realloc() or kmalloc()
depending on old and new allocation sizes.

This ensures that allocated memory stays physically contiguous.

Sponsored by: The FreeBSD Foundation
MFC after: 3 days
Suggested by: jhb (see D46657)

Test Plan

Someone please double-check the DEBUG_MEMGUARD/DEBUG_REDZONE
invariants in malloc_usable_size() for contigmalloc!

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bz created this revision.Sat, Mar 29, 6:34 PM

Herald added subscribers: olce, emaste, imp. · View Herald TranscriptSat, Mar 29, 6:34 PM

bz requested review of this revision.Sat, Mar 29, 6:34 PM

Harbormaster completed remote builds in B63198: Diff 152847.Sat, Mar 29, 6:34 PM

bz added a child revision: D49572: LinuxKPI; cleanup slab.h a bit; move more free() into slab.c.Sat, Mar 29, 6:36 PM

bz added a parent revision: D46657: LinuxKPI: switch mallocarray to an lpi implementation using __kmalloc().

The patch looks fine to me.

There is a typo in the descripton/commit message in "Implement kreallocc()`": the 'c' is duplicated in the function name.

markj added inline comments.Sun, Mar 30, 12:27 PM

sys/compat/linuxkpi/common/src/linux_slab.c
248 ↗	(On Diff #152847)	Isn't it sufficient to check `size <= PAGE_SIZE`? I think it'd be nice to add a comment explaining that realloc() doesn't handle contiguous allocations.

In D49571#1130320, @dumbbell wrote:

The patch looks fine to me.

There is a typo in the descripton/commit message in "Implement kreallocc()`": the 'c' is duplicated in the function name.

Fixed. Thanks.

Add the original ptr == NULL check and just call malloc in that case.
Add a comment about realloc and simplify the size check as suggested by @markj.

Harbormaster completed remote builds in B63242: Diff 152953.Tue, Apr 1, 11:43 AM

Argh, sorry. I'll re-upload the diff in a second.

Let's hope arc diff update Dxxx HEAD^1 works.

Harbormaster completed remote builds in B63245: Diff 152957.Tue, Apr 1, 11:49 AM

jhb accepted this revision.Tue, Apr 1, 5:30 PM

jhb added inline comments.

sys/compat/linuxkpi/common/src/linux_slab.c
254 ↗	(On Diff #152957)
sys/kern/kern_malloc.c
1145	I would be tempted to commit this separately FWIW, but it's not a big deal. I think of it as more of a fixup to the original commit to merge contigmalloc() into the malloc() API so that free() can be used with both.