Page MenuHomeFreeBSD

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

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jeff created this revision.Fri, Nov 29, 9:03 PM
jeff edited the summary of this revision. (Show Details)Fri, Nov 29, 9:07 PM
jeff added reviewers: rlibby, markj.
jeff set the repository for this revision to rS FreeBSD src repository.
markj added inline comments.Mon, Dec 2, 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.Mon, Dec 2, 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.

jeff updated this revision to Diff 65146.Tue, Dec 3, 12:06 AM

Address review feedback

markj accepted this revision.Tue, Dec 3, 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.Tue, Dec 3, 4:27 PM
rlibby accepted this revision.Wed, Dec 4, 7:34 AM

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?

This revision was automatically updated to reflect the committed changes.