Page MenuHomeFreeBSD

Deduplicate bus_dma bounce code.
ClosedPublic

Authored by jhb on Dec 28 2021, 7:34 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sep 20 2024, 1:09 PM
Unknown Object (File)
Sep 18 2024, 9:12 AM
Unknown Object (File)
Sep 17 2024, 8:18 PM
Unknown Object (File)
Sep 10 2024, 6:50 PM
Unknown Object (File)
Sep 5 2024, 12:15 AM
Unknown Object (File)
Aug 31 2024, 5:23 PM
Unknown Object (File)
Aug 31 2024, 2:36 AM
Unknown Object (File)
Aug 28 2024, 7:25 PM

Details

Summary

Move mostly duplicated code in various MD bus_dma backends to support
bounce pages into sys/kern/subr_busdma_bounce.c. This file is
currently #include'd into the backends rather than compiled standalone
since it requires access to internal members of the opaque bus_dma
structures such as bus_dmamap_t and bus_dma_tag_t.

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhb requested review of this revision.Dec 28 2021, 7:34 PM

Using #include is not necessarily ideal, but it is at least a starting point. Having alloc_bounce_zone() accept dma tag properties as parameters and returning a struct bounce_zone * would avoid most of the need to know the internals of bus_dmatag_t, but bus_dma_map_t is a tougher nut to crack.

jhb added inline comments.
sys/kern/subr_busdma_bounce.c
51

These bits for MIPS will go away soon.

56

We could perhaps adopt the x86 alloc_bounce_page() API on all architectures and just always pass 0 as addr2 on non-x86 to remove the x86-specific #ifdef's.

112

Note that only 32-bit arm cleared map->pagesneeded to 0 in this case. I'm not sure if it is more or less correct to always do this.

164

Currently only x86 is NUMA aware, but other arches do support NUMA (arm64 and powerpc come to mind?). I made this conditional on dmat_domain so that those other arches could enable NUMA by defining it along with adding the relevant MD bits when creating tags

276

The one other MIPs-specific bit.

sys/kern/subr_busdma_bounce.c
38

Why not move M_BUSDMA declaration there?

40

Same, why not define it there? Is some arch too weird for the move of the definition?

Or may be we need some other 'common' source, perhaps?

56

I think we can use datapage[2] even without API adoption.

112

Could you please explain why? Imo it is reverse, we need to keep pagesneeded non-zero after failure?

sys/kern/subr_busdma_bounce.c
38

Some backends use it in other parts of the file.

40

Some other backends define their own nodes as well. Notably 32-bit arm I believe.

56

Yes, that is true, though it wouldn't really make the #ifdef's in add_bounce_page any better I think.

112

This is going to fail with ENOMEM out to the caller due to BUS_DMA_NOWAIT. The map doesn't need to hold persistent state in this case as the load has just failed and the map is not "in use".

sys/kern/subr_busdma_bounce.c
112

I looked into this more and this appears to be a bug on other platforms. In fact, even on arm if the load fails with EFBIG we fail to clear pagesneeded in the map, so the next request that reuses a map might try to allocate bounce pages it doesn't need (or worse, we allocate too few because the new request needs more bounce pages than the old one, but we never try to count since pagesneeded is already non-zero). Probably the better solution though is to ensure that all of the bus_dmamap_load*() routines always reset pagesneeded to 0 at the start. You do need to be careful that we don't reset it to zero when retrying the request from the SWI thread (not that it hurts to reset it, you just avoid using the cached value and end up recomputing it which is wasteful).

This revision is now accepted and ready to land.Jan 1 2022, 1:37 AM
This revision was automatically updated to reflect the committed changes.