tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: proposal for bus_dma(9) change

On Wed, Mar 04, 2009 at 02:27:02PM +0100, Christoph Egger wrote:
> The driver's attaching function knows something failed and calls the
> function which free's the dma memory (actually, some drivers

And who says this is a correct thing to do?  It seems to me that here
you're only trying to give an excuse to sloppy programming.  An
allocation function that leaves half of what it is supposed to allocate
hanging around while returning an error is not exactly good practice.

> don't do even that!). This free function just gets a softc argument,
> so the error code is unknown. The free routine doesn't even
> know if something failed. All what it can do is that it checks

This is where your argument is fallacious.  The error code returned by
the xxx_dma_alloc function doesn't express the point at which it stopped
doing allocations anyway.

> the dmamap pointer against NULL. If it isn't NULL, it calls
> bus_dmamap_destroy() => BOOM
> > If the bug is caused by reusing dmamaps, we should explicitly
> > make dmamap NULL in driver dependent free functions, rather than
> > MI bus_dmamap_create(9) API.
> How should the *free* function do that ?

But that's the point.  Calling the free function for partial clean-up is
what is sloppy.

I know of no other API that uses two different means simultaneously to
indicate an error.  Changing bus_dma(9 to do that would be setting a
very bad precedent.

> There are two ways to fix above described problem:
> 1) In the driver:  In the dma allocation routine, explicitely set
> the dmamap pointer to NULL in the error path of bus_dmamap_create()
> before it returns. This makes the dmamap pointer check against NULL
> as described above correct and bus_dmamap_destroy() isn't called.
> Required effort: We have to go through all drivers and we always
> have to fix modified/new drivers again and again.

That's something to be done on all drivers *anyway*.

> Disadvantage: This is a forever task since we will never stop changing or 
> writing new drivers.

That's another fallacious argument.  The reason why failures during
attachment are so badly handled is because historically the driver was
toasted at that point anyway.  It's only very recently that the user is
able to detach it, although most of them don't even have a detach()

I'm sure you have noticed how anything that uses bus_dma(9) or
bus_space(9) after attachment takes good care of using it properly.

All drivers will have to be revisted at some point.  Attach functions
will be allowed to return an error, and no, it's no my intent to call
the detach function in that case to get the driver to clean up.  It'd
just not make any sense to me, no matter how much sense it seems to make
to you with DMA memory allocation routines.

Quentin Garnier - -
"See the look on my face from staying too long in one place
[...] every time the morning breaks I know I'm closer to falling"
KT Tunstall, Saving My Face, Drastic Fantastic, 2007.

Attachment: pgp7agwMnhmQ4.pgp
Description: PGP signature

Home | Main Index | Thread Index | Old Index