| Message ID | 20170104092252.GD3009@templeofstupid.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 01/04/2017 02:22 AM, Krister Johansen wrote: > 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_NOWAIT so that the > allocations do not block and instead use an allocator that uses > spinlocks. > > Signed-off-by: Krister Johansen <kjlx@templeofstupid.com> Acked-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/dma/ioat/init.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c > index 90eddd9..823cdb9 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_NOWAIT, &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_NOWAIT); > if (!ring) > return -ENOMEM; > > -- 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
On Wed, Jan 04, 2017 at 01:22:52AM -0800, Krister Johansen wrote: > 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_NOWAIT so that the > allocations do not block and instead use an allocator that uses > spinlocks. Applied now, after fixing the subsystem and driver tag.
On Tue, Jan 10, 2017 at 10:43:27AM +0530, Vinod Koul wrote: > On Wed, Jan 04, 2017 at 01:22:52AM -0800, Krister Johansen wrote: > > 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_NOWAIT so that the > > allocations do not block and instead use an allocator that uses > > spinlocks. > > Applied now, after fixing the subsystem and driver tag. Thanks for fixing that up and pulling this in. I also neglected to cc stable when I sent this out. Is this a fix that would qualify for inclusion in that tree too? Thanks, -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 --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c index 90eddd9..823cdb9 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_NOWAIT, &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_NOWAIT); if (!ring) return -ENOMEM;
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_NOWAIT 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(-)