Page MenuHomeFreeBSD

Optimize keg_drain().
ClosedPublic

Authored by markj on Feb 6 2020, 3:48 AM.

Details

Summary

Avoid holding the keg lock for longer than necessary. This helps ensure
that trimming the zone doesn't block allocations for longer than really
needed. If a zone's bucket list has recently been drained and we are
draining the keg, it is likely that allocations will block until
keg_drain() has emptied the free slab list, and that may take a long
time.

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

markj created this revision.Feb 6 2020, 3:48 AM
rlibby accepted this revision.Feb 6 2020, 6:48 AM
rlibby added inline comments.
sys/vm/uma_core.c
1273 ↗(On Diff #67849)

You can delete this check now.

1283–1284 ↗(On Diff #67849)

This is outside the lock, so we don't really care, but I think there's no particular reason to maintain the list here vs abandoning it as we iterate with LIST_FOREACH_SAFE. Additionally, only this list maintenance writes the slab header on destruction here.

3426–3430 ↗(On Diff #67849)

It might be useful to leave a comment explaining that we know that slab was on the partial list, and NOT the free list (even if it is totally free, and so we aren't missing a ud_free_slabs-- here), and maybe to leave a similar comment about the slab returned by keg_fetch_slab.

This revision is now accepted and ready to land.Feb 6 2020, 6:48 AM
markj marked 3 inline comments as done.Feb 6 2020, 3:25 PM
markj updated this revision to Diff 67867.Feb 6 2020, 3:28 PM

Apply Ryan's feedback:

  • Remove an unneeded check in keg_drain().
  • Avoid unnecessarily dirtying slab headers in keg_drain().
  • Add some comments to explain how we know that no extra ud_free_slabs manipulation is needed.
This revision now requires review to proceed.Feb 6 2020, 3:28 PM
rlibby accepted this revision.Feb 6 2020, 3:48 PM

Fixups look good.

This revision is now accepted and ready to land.Feb 6 2020, 3:48 PM
jeff accepted this revision.Feb 6 2020, 9:05 PM
jeff added inline comments.
sys/vm/uma_core.c
4648–4651 ↗(On Diff #67867)

Let's make this sequence an inline since it's now three statements that are repeated in multiple places.

sys/vm/uma_int.h
330 ↗(On Diff #67867)

I'm I correct in there being 32bit left before we spill to a new line?

markj added inline comments.Feb 6 2020, 9:08 PM
sys/vm/uma_int.h
330 ↗(On Diff #67867)

No, ud_lock is in its own cache line and struct slabhead is a single pointer, so there is still a lot of padding left over.

This revision was automatically updated to reflect the committed changes.