RE: API change for bus_dma

From: John Baldwin (jhb_at_FreeBSD.org)
Date: 06/27/03

  • Next message: Robert Watson: "Re: dynamically linked root and nscd" 
    Date: Fri, 27 Jun 2003 11:27:02 -0400 (EDT)
To: Scott Long <scott_long@btc.adaptec.com>

    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/
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"
  • Next message: Robert Watson: "Re: dynamically linked root and nscd"

    Relevant Pages