Page MenuHomeFreeBSD

[uma-multipage 1/3] uma: add UMA_ZONE_CONTIG, and a default contig_alloc
ClosedPublic

Authored by rlibby on Jan 17 2020, 5:44 PM.

Details

Summary

For now, copy the mbuf allocator.

Test Plan

kyua test -k /usr/tests/sys/Kyuafile

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

share/man/man9/zone.9
345 ↗(On Diff #66922)

drop ", but only"

477 ↗(On Diff #66922)

s/yields/allocates

memory is used redundantly. I would say 'if memory with special constraints such as attributes, alignment, or address ranges must be used..."

sys/vm/uma.h
238 ↗(On Diff #66922)

I think this could be phrased better as a constraint than a statement of fact.

"Objects must be physically contiguous"

Although that still seems to imply that somehow multiple objects are contiguous. We really want to express that a single object will be.

sys/vm/uma_core.c
2070 ↗(On Diff #66922)

There really shouldn't be any boot time contig allocations but I suppose it doesn't hurt to handle it.

rlibby added inline comments.
share/man/man9/zone.9
477 ↗(On Diff #66922)

Okay, and then drop the VM_MEMATTR_DEFAULT example?

sys/vm/uma.h
610–611 ↗(On Diff #66922)

This is gratuitous, but maybe related enough to sneak in here.

sys/vm/uma_core.c
1644–1646 ↗(On Diff #66922)

Also gratuitous, but related.

2070 ↗(On Diff #66922)

Yes... I thought to handle it since it's easy and I think it works fine with startup_alloc. If you prefer, I can add it to the KASSERT about early boot PCPU alloc instead.

jeff added inline comments.
share/man/man9/zone.9
477 ↗(On Diff #66922)

right.

sys/vm/uma.h
610–611 ↗(On Diff #66922)

I don't mind.

We could almost get rid of this field now.

This revision is now accepted and ready to land.Jan 17 2020, 7:55 PM
markj added inline comments.
share/man/man9/zone.9
344 ↗(On Diff #66922)

Per mandoc style, sentences should start on new lines.

sys/vm/uma.h
238 ↗(On Diff #66922)

Yeah, I'm having trouble phrasing this concisely with the right sense. I think your suggestion is okay. Here's another idea: "Each object must be internally physically contiguous."

sys/vm/uma.h
238 ↗(On Diff #66922)

Your proposed phrasing, specifically "Each object", is less likely to be misinterpreted.

No description that fits in one line will be unambiguous, If you are willing to use two lines, I would suggest: "Physical memory underlying an object must be contiguous"

share/man/man9/zone.9
345 ↗(On Diff #66922)

I would suggest: "The physical memory underlying each item in this zone must be contiguous. In particular, if an item spans a page boundary, then the underlying physical pages must be contiguous."

rlibby added inline comments.
sys/vm/uma.h
238 ↗(On Diff #66922)

Thanks, I'll use that.

rlibby marked an inline comment as done.

Review feedback: man page and comment wording. Also s/0x%#b/0x%b/.

This revision now requires review to proceed.Jan 18 2020, 6:46 PM

Rebase over r357392.

@jeff, note that I moved the assignment of uk_allocf = pcpu_page_alloc under the startup_alloc check, because I thought it read better that way. Please let me know if you'd like that restored, or if there was another reason for it being outside that check.

Drop early-boot PCPU assert. It works now.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 4 2020, 10:40 PM
This revision was automatically updated to reflect the committed changes.