Page MenuHomeFreeBSD

uma: Respect uk_reserve in keg_drain()
ClosedPublic

Authored by markj on Oct 13 2020, 8:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 22, 7:16 AM
Unknown Object (File)
Fri, Mar 22, 7:16 AM
Unknown Object (File)
Fri, Mar 22, 7:16 AM
Unknown Object (File)
Fri, Mar 22, 7:16 AM
Unknown Object (File)
Fri, Mar 22, 7:16 AM
Unknown Object (File)
Fri, Mar 22, 7:16 AM
Unknown Object (File)
Jan 16 2024, 3:15 AM
Unknown Object (File)
Dec 23 2023, 1:11 AM
Subscribers

Details

Summary

When we set a reserve of free items for a zone, we must take care to avoid
reclamining them. Modify keg_drain() to simply respect the reserved pool. For
now we take uk_reserve to be a per-NUMA domain quantity, since that's what
existing code does.

While here remove an always-false uk_freef == NULL check (kegs that
shouldn't be drained should set _NOFREE instead), and make sure that the
keg_drain() KTR statement does not reference an uninitialized variable.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 34220
Build 31366: arc lint + arc unit

Event Timeline

markj requested review of this revision.Oct 13 2020, 8:57 PM
markj created this revision.
sys/vm/uma_core.c
1437–1438

If it might be the case that the reserve could be large, this could prevent some pointless list surgery when we can't reclaim anything:

	n = dom->ud_free_slabs;
	if (n == 0 || dom->ud_free_items - keg->uk_ipers < keg->uk_reserve)
		goto unlock;
	LIST_SWAP(&freeslabs, &dom->ud_free_slab, uma_slab, us_link);
1444

But there are also free items in the partial slabs, and those work fine for reserve allocations. This loop needs to include some kind of comparison of ud_free_items vs uk_reserve. Maybe:

	free_items = dom->ud_free_items - n * keg->uk_ipers;
	while (free_items < keg->uk_reserve) {
		...
		n--;
		free_items += keg->uk_ipers;
	}
1449

This reverses the order. Do we care? If so, we can do tail inserts by keeping track of the tail and using LIST_INSERT_AFTER.

1450–1452

Rather than remove from hash above and re-insert here, can't we just move the hash remove block to below the re-insert block (and walk &freeslabs instead)?

markj marked 2 inline comments as done.
  • Address feedback.
  • Add a uk_reserve sysctl for each keg.
sys/vm/uma_core.c
1437–1438

I think we can go a bit further and determine whether we need to keep more slabs than we'd free, and in that case pull individual slabs off of the domain's free slab list.

1449

I can't think of any reason this would hurt anything.

Don't assume that the number of free items is larger than the reserve.

Fix the problem by avoiding underflow instead.

sys/vm/uma_core.c
1437–1438

Here's a suggestion for rephrasing that into one loop where we calculate the number of slabs to keep up front. Take or leave according to your taste.

	partial = dom->ud_free_items - dom->ud_free_slabs * keg->uk_ipers;
	keep_slabs = 0;
	if (partial < keg->uk_reserve) {
		keep_slabs = min(dom->ud_free_slabs,
		    howmany(keg->uk_reserve - partial, keg->uk_ipers));
	}
	n = dom->ud_free_slabs - keep_slabs;

	for (i = min(keep_slabs, n); i > 0; i--) {
		slab = LIST_FIRST(&dom->ud_free_slab);
		LIST_REMOVE(slab, us_link);
		LIST_INSERT_HEAD(&freeslabs, slab, us_link);
	}

	if (keep_slabs < n)
		LIST_SWAP(&freeslabs, &dom->ud_free_slab, uma_slab, us_link);
1445

>= ? If it's equal to reserve + ipers, we can subtract ipers and still meet reserve.

sys/vm/uma_core.c
1437–1438

Thanks, that's much nicer than what I wrote.

Simplify keg_drain_domain().

This revision is now accepted and ready to land.Oct 16 2020, 4:48 PM
sys/vm/uma_core.c
1433

Suggested comment to describe what follows: Are the free items in partially allocated slabs sufficient to meet the reserve? If not, compute the number of fully free slabs that must be kept.

1441

When I first read this "min" operation, my immediate was "what in the world is going on here." I think that this deserves a comment explaining that this is an optimization.

By the way, in case you are going to use the summary in the commit message, "reclamining" is misspelled.

markj marked 2 inline comments as done.

Add comments per Alan's notes.

This revision now requires review to proceed.Oct 16 2020, 7:55 PM
This revision is now accepted and ready to land.Oct 16 2020, 8:47 PM
This revision was automatically updated to reflect the committed changes.