Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 20 Jun 2003 20:21:35 -0600
From:      Scott Long <scott_long@btc.adaptec.com>
To:        freebsd-arch@freebsd.org
Subject:   API change for bus_dma
Message-ID:  <3EF3C12F.9060303@btc.adaptec.com>

next in thread | raw e-mail | index | archive | help
All,

As I work towards locking down storage drivers, I'm also preening their
use of busdma.  A common theme among them is a misuse of
bus_dmamap_load() and the associated callback mechanism.  For most, the
consequence is harmless as long as the card can support the amount of
physical memory in the system (systems with IOMMU's not withstanding).
However, in cases such as PAE where busdma might have to use bounce
buffers, most drivers don't handle the possibility of bus_dmamap_load()
returning EINPROGRESS.  The consequence of this is twofold:
bus_dmamap_load() returns without the callback being called, but the
driver doesn't detect this and merrily goes on its way.  Later on the
callback does get called, and any state that was shared with it gets
corrupted.  This is a problem even for drivers that are under Giant.

The solution for this is mostly a mechanical cut-n-paste of the code
dealing with the callback.  However, locking down the drivers presents
a new problem with the callback.  Since the callback can be called
asynchronously from an SWI, it needs some way to synchronize with the
driver.  Adding code to each callback to conditionally grab the driver
mutex incurs a penalty (albiet small) and requires more effort.  The
better solution is to export the driver mutex to busdma and have the
SWI that runs the callback lock the mutex before calling the callback.

This requires adding a 'struct mtx *' argument to bus_dma_tag_create()
to hold the mutex to be exported.  For drivers that are under Giant
and/or decide not to use this functionality, passing NULL for this
argument is accepted.  Therefore, the change is fairly low-impact,
though it incurs an API change.  Since locking the peripheral drivers
is a major goal for 5.2 and 5-STABLE, it is time to bite the bullet
and do this now.  A few 3rd-party drivers stand to be affected, and
hopefully their maintainers will react accordingly.

Comments?

Scott



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3EF3C12F.9060303>