Hi Fredrik,
On 02/03/18 18:07, Fredrik Noring wrote:
Hello DMA mapping helpers maintainers,
I'm porting the PS2 OHCI driver to v4.15 and it triggers
WARN_ON(irqs_disabled());
in include/linux/dma-mapping.h:dma_free_attrs (trace below). USB maintainer
Alan Stern notes in
https://www.spinics.net/lists/linux-usb/msg162817.html
that
This is caused by a deficiency in the DMA core: dma_free_coherent()
wants interrupts to be enabled when it is called. I'm not sure how
the other host controller drivers cope with this.
What is the best way to proceed? Can dma_free_attrs be reworked to handle
disabled IRQs?
Historically, that particular line of code appears to date back to
commit aa24886e379d (and tracking it's ancestry was quite fun).
Now, I'm sure not all of the considerations of 11-and-a-half years ago
still apply, but one certainly does: ARM* still uses non-cacheable
mappings for coherent allocations on systems which aren't
hardware-coherent (i.e. most of them), thus the alloc and free paths may
respectively set up and tear down those mappings, and the latter
involves this guy:
void vunmap(const void *addr)
{
BUG_ON(in_interrupt());
might_sleep();
if (addr)
__vunmap(addr, 0);
}
Even in the non-coherent ARM case it *would* technically be viable to
free a coherent buffer from interrupt context iff it was allocated with
GFP_ATOMIC, as those are satisfied from a statically-mapped pool rather
than dynamically vmapped, but that doesn't really expand to the general
case, and I certainly can't speak for all the other architectures.
As Alan implies, I guess the way forward is to figure out how similar
drivers manage - your backtrace suggests that you might be using
HCD_LOCAL_MEM, which probably isn't the common case but does seem to
appear in at least a couple of other OHCI drivers.
On the other hand, HCD_LOCAL_MEM implies a per-device coherent pool.
Since those bypass the arch-specific code, then depending on how
unlikely we think it is for devices covered by a single driver to exist
both with and without their own memory then it *might* also be
reasonable to make the change below. Christoph, what do you reckon?
Robin.
*plus now arm64 and probably others too
----->8-----
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 34fe8463d10e..eb8e42ce89a0 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -542,11 +542,11 @@ static inline void dma_free_attrs(struct device
*dev, size_t size,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!ops);
- WARN_ON(irqs_disabled());
if (dma_release_from_dev_coherent(dev, get_order(size), cpu_addr))
return;
+ WARN_ON(irqs_disabled());
if (!ops->free || !cpu_addr)
return;
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
