Page MenuHomeFreeBSD

Don't bother freeing UMA zones after the shutdown process starts.
ClosedPublic

Authored by markj on Jan 7 2020, 4:50 PM.

Details

Summary

Some kernel subsystems, notably ZFS, will destroy UMA zones from a
shutdown eventhandler. For slabs that are mapped into KVA this can be
very expensive and so it needlessly delays the shutdown process.

Add a new state to the "booted" variable, BOOT_SHUTDOWN. Once
kern_reboot() starts invoking shutdown handlers, turn uma_zdestroy()
into a no-op. In the future I plan to implement periodic preening of
cached items in excess of the WSS, which would alleviate the problem
somewhat, but this patch is simple and can be backported to the stable
branches.

PR: 242427

Test Plan

I wrote a kernel module that creates a zone, allocates and frees ~2 million items,
and then destroys the zone to trigger reclamation. With a slab size of 4096,
uma_zdestroy() takes about half a second to complete, whereas with a slab size
of 8192 it takes about 16 seconds.

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.Jan 7 2020, 4:50 PM
markj edited the summary of this revision. (Show Details)Jan 7 2020, 4:55 PM
markj edited the test plan for this revision. (Show Details)
markj added reviewers: jeff, kib, rlibby.
markj added inline comments.Jan 7 2020, 4:57 PM
sys/vm/uma_core.c
2830 ↗(On Diff #66443)

We could have a debug tunable to disable this behaviour.

kib accepted this revision.Jan 7 2020, 5:11 PM
This revision is now accepted and ready to land.Jan 7 2020, 5:11 PM
rlibby added a comment.Jan 8 2020, 5:31 AM

Code looks good.

Should we maybe gate this on whether the zone defines a zfini or zrelease function? I'm a little concerned that it could be possible for a client to be affected by that difference. Then if a client defines one but really wants to leak anyway, it can always leak on its own.

markj added a comment.Jan 8 2020, 4:06 PM

Code looks good.
Should we maybe gate this on whether the zone defines a zfini or zrelease function? I'm a little concerned that it could be possible for a client to be affected by that difference. Then if a client defines one but really wants to leak anyway, it can always leak on its own.

Yes, I think that makes sense. Perhaps a non-default uk_freef should also prevent this short-circuiting.

I don't think a client can really do anything from zfini since it doesn't have a return value.

markj updated this revision to Diff 66496.Jan 8 2020, 4:37 PM

Apply rlibby's suggestion.

This revision now requires review to proceed.Jan 8 2020, 4:37 PM
rlibby accepted this revision.Jan 8 2020, 5:40 PM
This revision is now accepted and ready to land.Jan 8 2020, 5:40 PM
jeff accepted this revision.Jan 9 2020, 6:09 PM
This revision was automatically updated to reflect the committed changes.