diff mbox

ioat_alloc_chan_resources should not perform sleeping allocations.

Message ID 20161231041024.GB2448@templeofstupid.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Krister Johansen Dec. 31, 2016, 4:10 a.m. UTC
On a kernel with DEBUG_LOCKS, ioat_free_chan_resources triggers an
in_interrupt() warning.  With PROVE_LOCKING, it reports detecting a
SOFTIRQ-safe to SOFTIRQ-unsafe lock ordering in the same code path.

This is because dma_generic_alloc_coherent() checks if the GFP flags
permit blocking.  It allocates from different subsystems if blocking is
permitted.  The free path knows how to return the memory to the correct
allocator.  If GFP_KERNEL is specified then the alloc and free end up
going through cma_alloc(), which uses mutexes.

Given that ioat_free_chan_resources() can be called in interrupt
context, ioat_alloc_chan_resources() must specify GFP_ATOMIC so that the
allocations do not block and instead use an allocator that uses
spinlocks.

Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
---
 drivers/dma/ioat/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Vinod Koul Jan. 2, 2017, 10:24 a.m. UTC | #1
On Fri, Dec 30, 2016 at 08:10:24PM -0800, Krister Johansen wrote:
 
> diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
> index 90eddd9..9d82f8f 100644
> --- a/drivers/dma/ioat/init.c
> +++ b/drivers/dma/ioat/init.c
> @@ -693,7 +693,7 @@ static int ioat_alloc_chan_resources(struct dma_chan *c)
>  	/* doing 2 32bit writes to mmio since 1 64b write doesn't work */
>  	ioat_chan->completion =
>  		dma_pool_zalloc(ioat_chan->ioat_dma->completion_pool,
> -				GFP_KERNEL, &ioat_chan->completion_dma);
> +				GFP_ATOMIC, &ioat_chan->completion_dma);

can you actually make it GFP_NOWAIT, we prefer that.
Krister Johansen Jan. 4, 2017, 9:22 a.m. UTC | #2
On Mon, Jan 02, 2017 at 03:54:13PM +0530, Vinod Koul wrote:
> On Fri, Dec 30, 2016 at 08:10:24PM -0800, Krister Johansen wrote:
>  
> > diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
> > index 90eddd9..9d82f8f 100644
> > --- a/drivers/dma/ioat/init.c
> > +++ b/drivers/dma/ioat/init.c
> > @@ -693,7 +693,7 @@ static int ioat_alloc_chan_resources(struct dma_chan *c)
> >  	/* doing 2 32bit writes to mmio since 1 64b write doesn't work */
> >  	ioat_chan->completion =
> >  		dma_pool_zalloc(ioat_chan->ioat_dma->completion_pool,
> > -				GFP_KERNEL, &ioat_chan->completion_dma);
> > +				GFP_ATOMIC, &ioat_chan->completion_dma);
> 
> can you actually make it GFP_NOWAIT, we prefer that.

Absolutely.  I'll send one that's updated to use that instead.

-K
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 90eddd9..9d82f8f 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -693,7 +693,7 @@  static int ioat_alloc_chan_resources(struct dma_chan *c)
 	/* doing 2 32bit writes to mmio since 1 64b write doesn't work */
 	ioat_chan->completion =
 		dma_pool_zalloc(ioat_chan->ioat_dma->completion_pool,
-				GFP_KERNEL, &ioat_chan->completion_dma);
+				GFP_ATOMIC, &ioat_chan->completion_dma);
 	if (!ioat_chan->completion)
 		return -ENOMEM;
 
@@ -703,7 +703,7 @@  static int ioat_alloc_chan_resources(struct dma_chan *c)
 	       ioat_chan->reg_base + IOAT_CHANCMP_OFFSET_HIGH);
 
 	order = IOAT_MAX_ORDER;
-	ring = ioat_alloc_ring(c, order, GFP_KERNEL);
+	ring = ioat_alloc_ring(c, order, GFP_ATOMIC);
 	if (!ring)
 		return -ENOMEM;