Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-06-22 Thread Fredrik Noring
Hi Robin, Christoph, On Thu, Mar 15, 2018 at 01:11:03PM +, Robin Murphy wrote: > On 15/03/18 07:58, Christoph Hellwig wrote: > > On Wed, Mar 14, 2018 at 05:43:46PM +, Robin Murphy wrote: > > > > So maybe for now the quick fix is to move the sleep check as suggested > > > > earlier in this

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-15 Thread Robin Murphy
On 15/03/18 07:58, Christoph Hellwig wrote: On Wed, Mar 14, 2018 at 05:43:46PM +, Robin Murphy wrote: Looking back I don't really understand why we even indirect the "classic" per-device dma_declare_coherent_memory use case through the DMA API. It certainly makes sense for devices which

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-15 Thread Christoph Hellwig
On Wed, Mar 14, 2018 at 05:43:46PM +, Robin Murphy wrote: >> Looking back I don't really understand why we even indirect the "classic" >> per-device dma_declare_coherent_memory use case through the DMA API. > > It certainly makes sense for devices which can exist in both shared-memory > and

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-14 Thread Robin Murphy
On 13/03/18 13:17, Christoph Hellwig wrote: On Tue, Mar 13, 2018 at 12:11:49PM +, Robin Murphy wrote: Taking a step back, though, provided the original rationale about dma_declare_coherent_memory() is still valid, I wonder if we should simply permit the USB code to call

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-13 Thread Christoph Hellwig
On Tue, Mar 13, 2018 at 12:11:49PM +, Robin Murphy wrote: > Taking a step back, though, provided the original rationale about > dma_declare_coherent_memory() is still valid, I wonder if we should simply > permit the USB code to call dma_{alloc,free}_from_dev_coherent() directly > here

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-13 Thread Robin Murphy
On 11/03/18 18:01, Fredrik Noring wrote: Hi Christoph, The point is that you should always use a pool, period. dma_alloc*/dma_free* are fundamentally expensive operations on my architectures, so if you call them from a fast path you are doing something wrong. The author's comment in commit

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-11 Thread Fredrik Noring
Hi Christoph, > The point is that you should always use a pool, period. > dma_alloc*/dma_free* are fundamentally expensive operations on my > architectures, so if you call them from a fast path you are doing > something wrong. The author's comment in commit b3476675320e "usb: dma bounce buffer

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-05 Thread Christoph Hellwig
On Sat, Mar 03, 2018 at 07:19:06PM +0100, Fredrik Noring wrote: > Christoph, Alan, > > > If it is allocating / freeing this memory all the time in the hot path > > it should really use a dma pool (see include/ilinux/dmapool.h). > > The dma coherent APIs aren't really built for being called in the

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-04 Thread Alan Stern
On Sun, 4 Mar 2018, Fredrik Noring wrote: > Hi Alan Stern, > > > > Alan, can dma_free_coherent be delayed to a point when IRQs are enabled? > > > > Yes, subject to the usual concerns about not being delayed for too > > long. Also, some HCDs are highly memory-constrained. I don't know if > >

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-04 Thread Fredrik Noring
Hi Alan Stern, > > Alan, can dma_free_coherent be delayed to a point when IRQs are enabled? > > Yes, subject to the usual concerns about not being delayed for too > long. Also, some HCDs are highly memory-constrained. I don't know if > they use this API, but if they do then delaying a free

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-04 Thread Alan Stern
On Sat, 3 Mar 2018, Fredrik Noring wrote: > Christoph, Alan, > > > If it is allocating / freeing this memory all the time in the hot path > > it should really use a dma pool (see include/ilinux/dmapool.h). > > The dma coherent APIs aren't really built for being called in the > > hot path. > >

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-03 Thread Fredrik Noring
Christoph, Alan, > If it is allocating / freeing this memory all the time in the hot path > it should really use a dma pool (see include/ilinux/dmapool.h). > The dma coherent APIs aren't really built for being called in the > hot path. hcd_buffer_free uses a combination of dma pools and dma

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-03 Thread Christoph Hellwig
On Sat, Mar 03, 2018 at 09:22:35AM +0100, Fredrik Noring wrote: > Critically, it performs the following calls: If it is allocating / freeing this memory all the time in the hot path it should really use a dma pool (see include/ilinux/dmapool.h). The dma coherent APIs aren't really built for being

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-03 Thread Fredrik Noring
Hi Christoph, > Why do you want to free coherent dma allocations from irq context? > They generally are a long-term resource that as a rule of thumb should > be allocated in ->probe and freed in ->remove. The device specific HCD only does dma_declare_coherent_memory (with HCD_LOCAL_MEM) in

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-03 Thread Fredrik Noring
Hi Robin, > 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

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-02 Thread Christoph Hellwig
On Fri, Mar 02, 2018 at 07:55:48PM +, Robin Murphy wrote: > 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 >

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-02 Thread Christoph Hellwig
On Fri, Mar 02, 2018 at 07:07:06PM +0100, Fredrik Noring wrote: > What is the best way to proceed? Can dma_free_attrs be reworked to handle > disabled IRQs? Why do you want to free coherent dma allocations from irq context? They generally are a long-term resource that as a rule of thumb should be

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-02 Thread Robin Murphy
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

WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-02 Thread Fredrik Noring
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