Use a variant structure for hash and offpage zones to save space.
ClosedPublic
Actions

Authored by jeff on Nov 29 2019, 9:03 PM.

Details

Reviewers

rlibby
markj

Commits

rS355510: Use a variant slab structure for offpage zones. This saves space in

Summary

This is a relatively minor memory savings but I like the code refactoring that resulted from it.

On a freshly booted system I am using ~2MB of memory for off page slab structures. It may make sense to make a small version and a large version. I believe that was originally a feature of UMA.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jeff created this revision.Nov 29 2019, 9:03 PM

Harbormaster completed remote builds in B27855: Diff 65061.Nov 29 2019, 9:03 PM

jeff edited the summary of this revision. (Show Details)Nov 29 2019, 9:07 PM

jeff added reviewers: rlibby, markj.

jeff set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a subscriber: imp. · View Herald TranscriptNov 29 2019, 9:07 PM

markj added inline comments.Dec 2 2019, 5:20 PM

sys/vm/uma_core.c
2357 ↗	(On Diff #65061)	Fix indentation while here?
sys/vm/uma_int.h
147 ↗	(On Diff #65061)	Missing a period in the comment.
154 ↗	(On Diff #65061)	This looks like it should be de-indented by one tab.
263 ↗	(On Diff #65061)	Where is this used?
304 ↗	(On Diff #65061)	Missing parens around the return value. The parens around `slab` and the cast are redundant.
315 ↗	(On Diff #65061)	Missing parens around the return value.

jeff added inline comments.Dec 2 2019, 8:23 PM

sys/vm/uma_int.h
263 ↗	(On Diff #65061)	relic of an aborted attempt to make a minimum size slab and maximum size slab zone.

Address review feedback

Harbormaster completed remote builds in B27899: Diff 65146.Dec 3 2019, 12:06 AM

markj accepted this revision.Dec 3 2019, 4:27 PM

markj added inline comments.

sys/vm/uma_int.h
158 ↗	(On Diff #65146)	Is the purpose of changing from SLIST to LIST to make this cheaper?

This revision is now accepted and ready to land.Dec 3 2019, 4:27 PM

Looks good! Below are a couple style comments, and a couple other drive by comments.

sys/vm/uma_core.c
994 ↗	(On Diff #65146)	Is this not better described as `slab_data(slab, keg)`?
996–998 ↗	(On Diff #65146)	Counting backward may not be a good idea if it defeats prefetch.
1054–1055 ↗	(On Diff #65146)	Not sure if this is perf sensitive or how long the loop is, but instead of adjusting `uk_pages` and `uk_free` in the loop, we could just count (`n++`) and then do the adjustment below the loop.
1188–1191 ↗	(On Diff #65146)	Easier to read in the positive direction: if (keg->uk_flags & UMA_ZONE_OFFPAGE) ((uma_hash_slab_t)slab)->uhs_data = mem; else slab = (uma_slab_t )(mem + keg->uk_pgoff); Ditto in `slab_data()`.
sys/vm/uma_int.h
241 ↗	(On Diff #65146)	Curious, why is this a keg member? It seems it's always `(uk_flags & UMA_ZONE_OFFPAGE) ? slabzone : NULL`.
488 ↗	(On Diff #65146)	Cast is now unnecessary?