uma: deduplicate uma_small_alloc
ClosedPublic
Actions

Authored by bnovkov on May 3 2024, 8:09 PM.

Details

Reviewers

markj
mhorne
kib
andrew
manu

Commits

rGda76d349b6b1: uma: Deduplicate uma_small_alloc

Summary

This patch refactors the UMA small alloc code and removes most UMA machine-dependent code.

The existing machine-dependent uma_small_alloc code was almost identical across all architectures, except for powerpc where using the direct map addresses involved extra steps in some cases.

The MI/MD split was replaced by a default uma_small_alloc implementation that can be overridden by architecture-specific code by defining the UMA_MD_SMALL_ALLOC symbol.
Furthermore, UMA_USE_DMAP was introduced to replace most UMA_MD_SMALL_ALLOC uses.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bnovkov created this revision.May 3 2024, 8:09 PM

Herald added a reviewer: andrew. · View Herald TranscriptMay 3 2024, 8:09 PM

Herald added a reviewer: andrew. · View Herald Transcript

Herald added a reviewer: manu. · View Herald Transcript

Herald added subscribers: riscv, olce, jhibbits and 2 others. · View Herald Transcript

bnovkov requested review of this revision.May 3 2024, 8:09 PM

bnovkov added a child revision: D45044: uma: retire UMA_MD_SMALL_ALLOC.May 3 2024, 8:31 PM

kib added inline comments.May 4 2024, 9:04 PM

sys/powerpc/powerpc/uma_machdep.c
41 ↗	(On Diff #138121)	Do you really need this guard? It seems to be always true.

bnovkov added inline comments.May 8 2024, 5:12 PM

sys/powerpc/powerpc/uma_machdep.c
41 ↗	(On Diff #138121)	I added it since powerpc's vmparam.h does not define it for 32 bit Book-E (lines 131-133). However, I'm not that well acquainted with powerpc so I don't know if that combination is even possible.

jhibbits added inline comments.May 8 2024, 5:30 PM

sys/powerpc/powerpc/uma_machdep.c
41 ↗	(On Diff #138121)	Reading the code, it looks like we can unconditionally enable UMA_MD_SMALL_ALLOC for all powerpc. I think it wasn't set before is that we assumed a DMAP, which 32-bit Book-E cannot have. The code path has evolved to handle it, though.

markj accepted this revision.May 13 2024, 1:16 PM

markj added inline comments.

sys/vm/uma_core.c
2083	This can be `static`, right? Or does that make supporting `UMA_MD_SMALL_ALLOC` difficult?
2084	since you're moving this code anyway. The old u_int*_t typedefs are deprecated.
2168

This revision is now accepted and ready to land.May 13 2024, 1:16 PM

Address multiple comments:

enabled UMA_MD_SMALL_ALLOC unconditionally
removed redundant include guard in ppc uma_machdep
removed obsolete u_int8_t types

This revision now requires review to proceed.May 13 2024, 7:15 PM

bnovkov marked 2 inline comments as done.May 13 2024, 7:18 PM

bnovkov added inline comments.

sys/vm/uma_core.c
2083	I don't think so, it would cause a 'static declaration follows non-static declaration' type of error on platforms that don't define `UMA_MD_SMALL_ALLOC`.