RE: [PATCH v9 5/6] usb:cdns3 Add Cadence USB3 DRD Driver
From: Felipe Balbi <hidden>
Date: 2019-08-12 10:34:46
Also in:
linux-usb, lkml
Hi, Pawel Laszczak [off-list ref] writes:
quoted
quoted
quoted
Yet another thread? Can't you just run this right before giving back the USB request? So, don't do it from IRQ handler, but from giveback path?Do you mean in: if (request->complete) { spin_unlock(&priv_dev->lock); if (priv_dev->run_garbage_collector) { .... } usb_gadget_giveback_request(&priv_ep->endpoint, request); spin_lock(&priv_dev->lock); } ??right, you can do it right before giving back the request. Or right after.quoted
I ask because this is finally also called from IRQ handler: cdns3_device_thread_irq_handler -> cdns3_check_ep_interrupt_proceed -> cdns3_transfer_completed -> cdns3_gadget_giveback -> usb_gadget_giveback_requestDid you notice that it doesn't reenable interrupts, though?I noticed that there is a lack of reenabling interrupts :) The problem is that If I have disabled interrupt the kernel complains for using dma_free_coherent function in such place. Here you have a fragment of complaints: [ 7420.502863] WARNING: CPU: 0 PID: 10260 at kernel/dma/mapping.c:281 dma_free_attrs+0xa0/0xd0 [ 7420.502866] Modules linked in: usb_f_mass_storage cdns3(OE) cdns3_pci_wrap(OE) libcomposite ... [ 7420.502965] cdns3_gadget_giveback+0x159/0x2a0 [cdns3] [ 7420.502975] cdns3_transfer_completed+0xc5/0x3c0 [cdns3] [ 7420.502986] cdns3_device_thread_irq_handler+0x1b1/0xab0 [cdns3] [ 7420.502991] ? __schedule+0x333/0x7e0 [ 7420.503001] irq_thread_fn+0x26/0x60 [ 7420.503006] ? irq_thread+0xa8/0x1b0 [ 7420.503011] irq_thread+0x10e/0x1b0 [ 7420.503015] ? irq_forced_thread_fn+0x80/0x80 [ 7420.503021] ? wake_threads_waitq+0x30/0x30 [ 7420.503029] kthread+0x12c/0x150 [ 7420.503034] ? irq_thread_check_affinity+0xe0/0xe0 [ 7420.503038] ? kthread_park+0x90/0x90 [ 7420.503045] ret_from_fork+0x3a/0x50 [ 7420.503061] irq event stamp: 2962 [ 7420.503065] hardirqs last enabled at (2961): [<ffffffffb252672c>] _raw_spin_unlock_irq+0x2c/0x40 [ 7420.503070] hardirqs last disabled at (2962): [<ffffffffb25268f5>] _raw_spin_lock_irqsave+0x25/0x60 [ 7420.503074] softirqs last enabled at (2918): [<ffffffffb2800340>] __do_softirq+0x340/0x451 [ 7420.503079] softirqs last disabled at (2657): [<ffffffffb1aa02b6>] irq_exit+0xc6/0xd0 [ 7420.503082] ---[ end trace d02652af11011c3b ]--- Maybe it's a bug in implementation of this function. I allocate memory with flag GFP_ATOMIC with disabled interrupt, but I can't free such memory.
I don't understand the intricacies of the coherent API to judge if it's
a bug in the API itself. In any case, here's where the splat comes from:
void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
dma_addr_t dma_handle, unsigned long attrs)
{
const struct dma_map_ops *ops = get_dma_ops(dev);
if (dma_release_from_dev_coherent(dev, get_order(size), cpu_addr))
return;
/*
* On non-coherent platforms which implement DMA-coherent buffers via
* non-cacheable remaps, ops->free() may call vunmap(). Thus getting
* this far in IRQ context is a) at risk of a BUG_ON() or trying to
* sleep on some machines, and b) an indication that the driver is
* probably misusing the coherent API anyway.
*/
WARN_ON(irqs_disabled());
if (!cpu_addr)
return;
debug_dma_free_coherent(dev, size, cpu_addr, dma_handle);
if (dma_is_direct(ops))
dma_direct_free(dev, size, cpu_addr, dma_handle, attrs);
else if (ops->free)
ops->free(dev, size, cpu_addr, dma_handle, attrs);
}
EXPORT_SYMBOL(dma_free_attrs);
maybe you're gonna have to fire up a workqueue to free this memory for
you :-(
Unless someone else has better ideas. Alan, Greg, any ideas?
--
balbi Attachments
- signature.asc [application/pgp-signature] 832 bytes