uma: Factor out the implementations of uma_zfree_{arg,smr}()

[markj]

Summary

The two function both free an item to a UMA zone, but uma_zfree_arg()
does so in such as way as to ensure that the item will be the first one
returned by a subsequent allocation, while uma_zfree_smr() must defer
reuse of the item and therefore never frees to the per-CPU alloc bucket.

When KASAN is enabled, we actually want uma_zfree_arg() to behave like
uma_zfree_smr(): to improve the reliability of use-after-free detection,
reuse of the newly freed item should be deferred for some time.

Refactor a bit to make it easier to improve KASAN along these lines:
introduce two helper functions, cache_free_item() and cache_free_smr(),
which handle most of the work of interacting with the per-CPU caches.
A subsequent commit will let uma_zfree_arg() use cache_free_smr() when
KASAN is enabled.

No functional change intended.

[rlibby]

Just for review convenience, do you have a public git branch with the patch set?

[markj]

In D58268#1336646, @rlibby wrote:

Just for review convenience, do you have a public git branch with the patch set?

Sure, I pushed one here: https://github.com/markjdb/freebsd/tree/main-kasan-fixes

BTW git arc can fetch patches from phabricator and apply them to a local tree, though of course that can fail, e.g., if the patch in phabricator needs a rebase. The following invocation will fetch and apply all of the patches in this series:

$ git arc patch -c -s -r D58272

Kind of cumbersome versus fetching a git branch, but hey...

[rlibby]

Style nits, but logic looks good.

sys/vm/uma_core.c

4461

I think style prefers us to spell this atomic_load_32() > 0, like how it was in the old code. uz_sleepers is unsigned so it should not matter logically.

4475

Another style point. I appreciate that you are avoiding assigning cache in a way that is somewhat surprising as we're outside of the critical section but using curcpu, as the comment in the non-SMR path explains. But it also introduces a difference when one reads the SMR vs non-SMR path, but in fact no difference is intended there. I would suggest that if the reader is not intended to notice a difference between the two paths, we spell these constructs in the same way in both paths. (And, in the non-SMR path, the assignment is used because cache is used twice.)

4508

udata is always NULL... but I see cache_free_smr is just temporary in this patch set, so, don't care I guess.

4578–4580

Unclear why this moved, especially as you didn't do the same in uma_zfree_smr.

[rlibby]

In D58268#1336651, @markj wrote:

In D58268#1336646, @rlibby wrote:

BTW git arc can fetch patches from phabricator and apply them to a local tree, though of course that can fail, e.g., if the patch in phabricator needs a rebase. The following invocation will fetch and apply all of the patches in this series:

$ git arc patch -c -s -r D58272

Kind of cumbersome versus fetching a git branch, but hey...

Thanks, I didn't know there was a way to get it to handle a patch stack.

[markj]

sys/vm/uma_core.c

4508

Yeah, I could remove the parameter here and introduce it in the next patch, but it's probably not worth bothering...

[markj]

Address review feedback

[markj]

sys/vm/uma_core.c

4475

I'm not quite sure what you're asking for here. Do you mean that cache_free_smr() should assign to cache here and load the zone flags using that variable? Or should uma_zfree_smr() load the flags and pass them in? The latter is a bit messy since they're only used when NUMA is defined.

[rlibby]

Looks good.

sys/vm/uma_core.c

4475

Yeah, you're right. There's a reason for the difference. It's good how you have it, never mind.