Page MenuHomeFreeBSD

Simplify swi for bus_dma.
ClosedPublic

Authored by jhb on Dec 14 2021, 7:06 PM.
Tags
None
Referenced Files
F102592096: D33447.diff
Thu, Nov 14, 12:38 PM
Unknown Object (File)
Sat, Oct 26, 10:18 PM
Unknown Object (File)
Thu, Oct 24, 7:37 AM
Unknown Object (File)
Sat, Oct 19, 11:19 AM
Unknown Object (File)
Sep 28 2024, 2:08 PM
Unknown Object (File)
Sep 24 2024, 3:54 AM
Unknown Object (File)
Sep 23 2024, 12:38 AM
Unknown Object (File)
Sep 20 2024, 1:52 AM

Details

Summary

When a DMA request using bounce pages completes, a swi is triggered to
schedule pending DMA requests using the just-freed bounce pages. For
a long time this bus_dma swi has been tied to a "virtual memory" swi
(swi_vm). However, all of the swi_vm implementations are the same and
consist of checking a flag (busdma_swi_pending) which is always true
and if set calling busdma_swi. I suspect this dates back to the
pre-SMPng days and that the intention was for swi_vm to serve as a
mux. However, in the current scheme there's no need for the mux.

Instead, remove swi_vm and vm_ih. Each bus_dma implementation that
uses bounce pages is responsible for creating its own swi (busdma_ih)
which it now schedules directly. This swi invokes busdma_swi directly
removing the need for busdma_swi_pending.

One consequence is that the swi now works on RISC-V which had previously
failed to invoke busdma_swi from swi_vm.

Sponsored by: Netflix

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 43351
Build 40239: arc lint + arc unit

Event Timeline

jhb requested review of this revision.Dec 14 2021, 7:06 PM

I have booted this on amd64 and it has passed make tinderbox

Although a bit long, the change looks straight forward...

This revision is now accepted and ready to land.Dec 14 2021, 7:12 PM
In D33447#756112, @imp wrote:

Although a bit long, the change looks straight forward...

There's a lot of duplicated code in the bus_dma backends. It might be nice to have a 'subr_busdma_bounce.c' at least someday, but that's a larger body of work.

In D33447#756115, @jhb wrote:
In D33447#756112, @imp wrote:

Although a bit long, the change looks straight forward...

There's a lot of duplicated code in the bus_dma backends. It might be nice to have a 'subr_busdma_bounce.c' at least someday, but that's a larger body of work.

Reading your initial description, I thought you were biting off that :). But I agree, that would be harder, especially sine these routines have evolved for years and it's not always clear that the differences are important to an architecture, or a bug in them all that's only fixed on a few...

kib added inline comments.
sys/arm/arm/busdma_machdep.c
1747

We usually annotate these args with __unused.

sys/x86/x86/busdma_bounce.c
1337

Would it make sense to schedule swi outside the bounce_lock owning region?

  • Address some feedback from kib@.
This revision now requires review to proceed.Dec 28 2021, 7:30 PM
jhb marked 2 inline comments as done.Dec 28 2021, 7:31 PM
jhb added inline comments.
sys/x86/x86/busdma_bounce.c
1337

An even better change I might do is to free the pages from a map as a batch so we only do one wakeup instead of one per page.

This revision is now accepted and ready to land.Dec 28 2021, 7:35 PM
This revision was automatically updated to reflect the committed changes.
jhb marked an inline comment as done.