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

rlibby created this revision.Jan 17 2020, 5:44 PM
jeff added inline comments.Jan 17 2020, 5:52 PM
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 a subscriber: alc.Jan 17 2020, 5:56 PM
rlibby marked an inline comment as done.Jan 17 2020, 6:08 PM
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 accepted this revision.Jan 17 2020, 7:55 PM
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 accepted this revision.Jan 17 2020, 8:18 PM
markj added inline comments.
share/man/man9/zone.9
344 ↗(On Diff #66922)

Per mandoc style, sentences should start on new lines.

rlibby added inline comments.Jan 18 2020, 5:05 PM
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."

alc added inline comments.Jan 18 2020, 5:33 PM
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"

alc added inline comments.Jan 18 2020, 5:52 PM
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 marked 2 inline comments as done.Jan 18 2020, 5:57 PM
rlibby added inline comments.
sys/vm/uma.h
238 ↗(On Diff #66922)

Thanks, I'll use that.

rlibby updated this revision to Diff 66971.Jan 18 2020, 6:46 PM
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
rlibby updated this revision to Diff 67724.Feb 3 2020, 5:49 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.

rlibby updated this revision to Diff 67783.Feb 4 2020, 7:47 PM

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.