Page MenuHomeFreeBSD

Partial refactoring of uma_zalloc_arg().
ClosedPublic

Authored by jeff on Wed, Nov 20, 11:04 PM.

Details

Summary

uma_zalloc_arg/zfree_arg have grown out of control. I would like to add another new feature and it may require a custom trip through uma_zalloc and I didn't want to add a bunch of new flag checks.

I attempted to make more of a fastpath without a total hack. This should dramatically reduce the amount of text involved in normal allocations. I moved some of the zitem checks closer to the allocation so they are more encapsulated. I still feel like a fair bit of this should be abstracted into inlines for clarity.

This diff should be functionally identical. I don't think it's perfect but I think it's a step in the right direction. I will be posting another for uma_zfree_arg() soon.

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

jeff created this revision.Wed, Nov 20, 11:04 PM
jeff edited the summary of this revision. (Show Details)Wed, Nov 20, 11:09 PM
jeff added reviewers: kib, markj, glebius.
jeff set the repository for this revision to rS FreeBSD src repository.
markj accepted this revision.Thu, Nov 21, 8:54 PM
markj added inline comments.
sys/vm/uma_core.c
2542 ↗(On Diff #64651)

Missing parens around the return value.

2551 ↗(On Diff #64651)

Add a CRITICAL_ASSERT()?

2585 ↗(On Diff #64651)

Make lockfail a bool while you're here?

2631 ↗(On Diff #64651)

"let's"

3080 ↗(On Diff #64651)

The fragment

<update uz_items>
if (zone->uz_sleepers > 0 && zone->uz_items < zone->uz_max_items)
    wakeup_one(zone);

is pasted all over the place. This might be a good opportunity to fix that.

This revision is now accepted and ready to land.Thu, Nov 21, 8:54 PM
jeff updated this revision to Diff 64700.Thu, Nov 21, 10:40 PM

Address review feedback.

This revision now requires review to proceed.Thu, Nov 21, 10:40 PM
markj accepted this revision.Fri, Nov 22, 3:35 PM
This revision is now accepted and ready to land.Fri, Nov 22, 3:35 PM
jeff added a reviewer: rlibby.Sat, Nov 23, 7:39 PM

First pass, just looked at stuff related to item_ctor.

I wish I'd committed D20722 a couple months ago, although it will be simpler to rewrite now with this as base.

sys/vm/uma_core.c
2449–2451 ↗(On Diff #64700)

Prediction is wrong, assuming you mean to predict no failure, and not no constructor.

2452–2454 ↗(On Diff #64700)

This is nicer here... I left it out in D20722 because the three call sites have three different responses to error. It would be nice to unify them.

3076–3078 ↗(On Diff #64700)

This now double counts the failure (once in item_ctor(), once under fail:).

jeff added a comment.Sat, Nov 23, 9:00 PM

I am not going to object to you committing yours first and me rebasing on top of that.

In D22470#492498, @jeff wrote:

I am not going to object to you committing yours first and me rebasing on top of that.

Okay. I will fix up the procedure signatures to match yours and update right now.

jeff added a comment.Mon, Nov 25, 7:02 AM

I am going to commit this soon if there are no objections.

sys/vm/uma_core.c
3080 ↗(On Diff #64651)

I think it should be fixed but I will do that in a follow-up commit.

2449–2451 ↗(On Diff #64700)

Constructors are rare except on debug builds.

jeff updated this revision to Diff 64909.Tue, Nov 26, 8:48 PM

Review feedback. Cosmetic code adjustments.

This revision now requires review to proceed.Tue, Nov 26, 8:48 PM
This revision was not accepted when it landed; it landed in state Needs Review.Tue, Nov 26, 10:17 PM
This revision was automatically updated to reflect the committed changes.