I could break this up into multiple functions but it would serve no real purpose other than style. It is inherently very linear and verbose.

I think this needs to be +2. +1 for underscore and + 1 for null terminator.

I renamed this because it confused even me.

It might have been nicer to have both, avoiding ugly unused params.

static void zone_foreach(void (*zfunc)(uma_zone_t));
static void zone_foreach_arg(void (*zfunc)(uma_zone_t, void *), void *);

May want to add __unused ? Etc below.

This scheme has a couple problems:

• The name count algorithm doesn't munge the names, so we may not have detected a collision earlier, but collide here (e.g. "foo bar" and "foo_bar" collide).
• The counting also doesn't strictly work:

z1 = zcreate("foo");   // "foo"
z2 = zcreate("foo");   // "foo_2"
zdestroy(z1);
z3 = zcreate("foo");   // "foo_2" (undetected collision)
z4 = zcreate("foo_2"); // "foo_2" (munging again)

These can lead to trying to reuse the sysctl name, which then would lead to a crash in this code.

We could consider just setting namecnt here in zone_alloc_sysctl according to the first name we are able to successfully insert into the tree. It doesn't look like the sysctl API is very hepful for us in doing that though. We could use sysctl_add_oid but we'd want to suppress the sysctl_warn_reuse spew (maybe with a CTLFLAG?).

sysctl won't crash if there is a duplicate. It will print a warning about a re-used entry. I don't think we need to detect super pathological cases. You are right that I should be using count - 1.

Yes, sysctl won't crash on collision, but it'll return NULL, which we don't check for here.

I agree we don't need to chase down pathological cases, just make them harmless.

For re-used nodes sysctl returns the original oid. It just doesn't return it for leafs. We never use the return value from a leaf oid add.

I originally tested this without any collision detection so I'm fairly sure it doesn't panic.

const

Yep, I'm on board now.

Hmm, it's a union, should this be

if (zone->uz_lockptr != &zone->uz_lock && zone->uz_keg != NULL) {
        keg = zone->uz_keg;

?

s/uz_domain[0]/uz_domain[i]/

s/minum/minimum/

Just a thought. We don't NULL uz_ctlname/uz_oid. That's fine, we do bzero them. But should we, for consistency? I expect the compiler to remove the stores of zero after bzero anyway.

We ought to sysctl_remove_oid earlier, to avoid a race on teardown. The sysctl handlers inspect the keg and the counters.

Here and below, do we actually need the ZONE_LOCK() for the counter fetch? And, since we are not actually inspecting the uz_cpu cache buckets, do we really need it for that either?

s/uz_allocs/uz_frees/

Unfortunately there's still a reference in userspace to clean up... tools/tools/umastat/umastat.c.