Page MenuHomeFreeBSD

Include caching zones into zone_foreach() where appropriate.
ClosedPublic

Authored by kib on Tue, Nov 5, 3:15 PM.

Details

Summary

This is especially important for the counters allocation because EARLY_COUNTER does not work after temp_bsp_pcpu is switched from.

In collaboration with: pho

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

kib created this revision.Tue, Nov 5, 3:15 PM
emaste added a subscriber: emaste.Tue, Nov 5, 3:34 PM
kib edited the summary of this revision. (Show Details)Tue, Nov 5, 6:06 PM
kib added reviewers: alc, markj, glebius.
kib edited the summary of this revision. (Show Details)
kib added a subscriber: pho.
markj added inline comments.Tue, Nov 5, 6:19 PM
sys/vm/uma_core.c
261 ↗(On Diff #63963)

I think "cache_zones" would be clearer. All zones perform some caching, so "caching_zones" sounds redundant. "Cache zones" do nothing except provide caching.

905 ↗(On Diff #63963)

Why not do this for cache zones as well?

kib marked an inline comment as done.Tue, Nov 5, 8:20 PM
kib added inline comments.
kib updated this revision to Diff 63975.Tue, Nov 5, 8:20 PM

Rename argument.

markj added a comment.Tue, Nov 5, 9:03 PM

Taking a step back, I believe the bool parameter to zone_foreach() is too blunt. For example, during reclaim we call zone_foreach(zone_trim, true), but in the callout we do zone_foreach(zone_timeout, false). The zone_timeout() estimates the WSS for the zone, and the WSS is used to decide how much to trim, but with this change we are not calculating the WSS for cache zones.

Instead, each callback should be modified to handle cache zones:

  • cache_shink() and cache_drain_safe_cpu() need no modification.
  • zone_alloc_counters() needs no modification.
  • zone_reclaim() should skip the keg_drain() call for cache zones (uz_zflags & UMA_ZFLAG_CACHE). In fact, zone_dtor() calls zone_reclaim(), and the keg_drain() call will cause a panic when operating on a cache zone. This is a regression from r343026.
  • uma_print_zone() needs no modification.
  • zone_timeout() needs to be modified to check for UMA_ZFLAG_CACHE before locking the keg and resizing its hash table.
sys/vm/uma_core.c
905 ↗(On Diff #63963)

Oh, that assertion should be conditional on "ws". It makes no sense to impose the bucket limit in this case, since we are about to free those buckets as the next step in the reclaim.

I second Mark's latest suggestion.

kib updated this revision to Diff 63999.Wed, Nov 6, 2:54 PM
markj accepted this revision.Wed, Nov 6, 3:34 PM

Thanks.

This revision is now accepted and ready to land.Wed, Nov 6, 3:34 PM
glebius accepted this revision.Wed, Nov 6, 11:47 PM
alc accepted this revision.Thu, Nov 7, 8:21 AM
This revision was automatically updated to reflect the committed changes.