Page MenuHomeFreeBSD

Simplify swi for bus_dma.
ClosedPublic

Authored by jhb on Dec 14 2021, 7:06 PM.
Tags
None
Referenced Files
F86906897: D33447.id100673.diff
Thu, Jun 27, 7:36 AM
F86906893: D33447.id100673.diff
Thu, Jun 27, 7:36 AM
Unknown Object (File)
Sat, Jun 22, 8:22 PM
Unknown Object (File)
Sat, Jun 22, 4:14 AM
Unknown Object (File)
Sat, Jun 22, 4:06 AM
Unknown Object (File)
Sat, Jun 22, 1:36 AM
Unknown Object (File)
Sat, Jun 22, 12:54 AM
Unknown Object (File)
Fri, Jun 21, 2:09 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
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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
1751

We usually annotate these args with __unused.

sys/x86/x86/busdma_bounce.c
1339

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
1339

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.