Page MenuHomeFreeBSD

busdma: Avoid overallocation of bounce pages
Needs ReviewPublic

Authored by markj on Dec 23 2024, 9:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jan 8, 7:26 PM
Unknown Object (File)
Mon, Jan 6, 8:20 AM
Unknown Object (File)
Sat, Jan 4, 6:05 AM
Unknown Object (File)
Wed, Jan 1, 8:43 PM
Unknown Object (File)
Tue, Dec 31, 6:27 PM
Unknown Object (File)
Dec 27 2024, 11:37 AM
Unknown Object (File)
Dec 27 2024, 7:44 AM
Unknown Object (File)
Dec 27 2024, 5:52 AM

Details

Summary

A busdma tag maintains a "bounce zone", an allocator of pages that can
be used to transfer data when a DMA mapping requires bouncing. These
pages are generally reserved at DMA map creation time, or at tag
creation time (if BUS_DMA_ALLOCNOW is specified).

busdma tags may share a single bounce zone. If BUS_DMA_ALLOCNOW is not
specified when creating a tag, then the first mapping created for that
tag will reserve a page in the bounce zone, even though it may not be
needed. This can result in overallocation and leaks of memory if many
busdma tags and mappings are created, or if tags and mappings are
destroyed and re-created over time.

Commit 6fa7734d6fbbe attempted to fix this, but made the bounce page
reservation logic too restrictive, causing some drivers to fail to
reserve bounce pages entirely; it was reverted in commit eae22c44301.

Try again to fix the same problem: try to reserve pages if the zone does
not already exceed its maximum. While here, simplify logic used to set
the BUS_DMA_MI_ALLOC_COMP flag on a tag after bounce pages are
successfully reserved.

PR: 278569

Test Plan

I modified e1000 to require bouncing for buffers above 4GB, and verified that it reserved and used bounce pages during a stress test.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 61310
Build 58194: arc lint + arc unit

Event Timeline

markj requested review of this revision.Dec 23 2024, 9:01 PM

It would be tempting to hoist the pages calculation out of the this inner loop and just doing if ((bz->map_count + 1) * pages < bz->bz_total_pages && bz->bz_total_pages < maxpages) as the condition. You'd have to basically add 1 more to the map count if BUS_DMA_MIN_ALLOC_COMP was set (and then stop setting the flag in the else). That would avoid continuously reserving pages up to the bz_total_pages limit if you just keep destroying and freeing maps (e.g. if a NIC driver destroyed all its rings and maps for ifconfig down/up). However, if multiple tags share the same zone you can perhaps get starvation across tags. If we wanted to avoid that problem, we could instead track how many pages a tag has allocated so far, and then the expression would be if ((bz->map_count + 1) * pages < dmat->bounce_pages && bz->bz_total_pages < maxpages). That also nicely handles the BUS_DMA_MIN_ALLOC_COMP case and we could drop that flag entirely.

sys/arm64/arm64/busdma_bounce.c
437

This expression is different on arm than on non-arm due to commit 8160dc498371b25e1aa16cef76e47ae33f0a4d67. Perhaps as a followup we may want to apply this arm change to the other arches.

439

I do think we can remove these similar to the ones removed for non-arm below so that we always honor the bz->total_pages limit.