Re: [PATCH v2 1/4] dma-mapping: Add devm_ interface for dma_map_single()

From: Eli Billauer
Date: Fri Jun 06 2014 - 07:46:33 EST


Hello Joerg.


On 05/06/14 00:25, Joerg Roedel wrote:

What you are trying to do should work with dma_alloc_noncoherent(). The
API allows partial syncs on this memory, so you should be fine.
Please try to put yourself in my position: I have a driver that I care about, which works fine for a few years. It's based upon dma_map_single(), which seems to be the common way to get non-coherent memory, even for the driver's entire lifespan. I realize that dma_alloc_* was the intended way to do it, but fact is that dma_map_* has become the common choice.

Now I need to switch to dma_alloc_noncoherent(), which isn't used by many drivers, it seems. It should work the same, but there's always the worry if I'll cover all the corners. So will I take this risk of a nasty DMA bug on some esoteric platform, just to cut some lines in the code?

And if I choose to keep the unmanaged dma_map_single(), maybe I'll mess up if I convert other allocations to the managed API? Hmmm, maybe it's best to forget all about it.
The problem with a devm variant of dma_map_* is that it is too easy to
misuse or to use it wrong so that a driver could eat up all available
DMA handles on some platforms.
I suppose you mean that those who use dma_map_* for a one-off DMA session will drop the unmapping, thinking that it's "managed anyhow"...? Well, you can say that about any of the managed functions. For example, when devm_kzalloc() was introduced, someone maybe argued that people would drop kfree()s where they shouldn't, causing memory leaks.

So I think it boils down to whether devres is a good idea or not. Someone who thinks it's bad, will reject any new API, referring to memory efficiency, additional causes of failure and the risk of misleading the herd.

But if devres is to become commonly used in the future, I think drop-in replacements are necessary. In my opinion, telling people to adopt another methodology (e.g. dma_alloc_noncoherent vs. mapping), even if functionally equivalent, is a good way to make sure devres is never adopted.

Regards,
Eli
Joerg




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/