Date: Fri, 27 Jun 2003 11:27:02 -0400 (EDT) From: John Baldwin <jhb@FreeBSD.org> To: Scott Long <scott_long@btc.adaptec.com> Cc: freebsd-arch@freebsd.org Subject: RE: API change for bus_dma Message-ID: <XFMail.20030627112702.jhb@FreeBSD.org> In-Reply-To: <3EF3C12F.9060303@btc.adaptec.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 21-Jun-2003 Scott Long wrote:
> 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.
Erm, what's wrong with this:
void
foo_function()
{
mtx_assert(&mylock, MA_OWNED);
...
}
void
foo_callback()
{
mtx_lock(&mylock);
foo_function();
mtx_unlock(&mylock);
}
?
Using this approach is more flexible in case there is a driver
that uses a sx lock or a (not yet implemented) reader-writer lock
or a critical section, or whatever. This just means that the
callback uses a wrapper function but that really isn't that hard to
do and there are other cases (callouts in general) that need this.
To me this seems to be adding a special case to the API that won't
work for all situations anyways. I also don't see wrapper functions
as being all that hard.
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20030627112702.jhb>