Date:	Wed, 7 Nov 2012 12:40:57 -0800
From:	Dan Williams <djbw@...com>
To:	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	David Woodhouse <dwmw2@...radead.org>,
	Håvard Skinnemoen <hskinnemoen@...il.com>,
	Ira Snyder <iws@...o.caltech.edu>,
	"Koul, Vinod" <vinod.koul@...el.com>, vipin.kumar@...com,
	t.figa@...sung.com,
	"kyungmin.park@...sung.com" <kyungmin.park@...sung.com>,
	Dave Jiang <dave.jiang@...il.com>
Subject: Re: [PATCH 00/20] DMA: DMA unmap fixes

On Mon, Nov 5, 2012 at 2:00 AM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@...sung.com> wrote:
> Hi,
>
> Currently DMA subsystem does DMA mapping in the core code and DMA
> unmapping is done by device drivers.  This is counterintuitive,
> causes code duplication and subtle errors (some drivers like PL330
> one don't implement DMA unmapping code).  The following patchset
> modifies DMA subsystem to do DMA unmapping in the core code.
> It results in simpler code, less code duplication (more than 400
> LOC is gone) and fixes the issue with missing DMA unmapping code
> in some drivers.  Additionally many cases when DMA wasn't unmapped
> on a failure are also fixed.
>
>
> patches #1-3 add missing DMA unmap on failure to async_tx core
> code (async_memcpy()), ioat and fsmc_nand drivers
>
> patch #4 fixes DMA flags used by carma-fpga driver

Ack patches 1-4

> patches #5-7 fix core code and dmatest driver to DMA unmap for
> MEMCPY operations

Yes, this needs to move out of the drivers, but it needs to move past
the core and into the dmaengine clients directly (md/raid and
net_dma).  The slave_dma usage model already does this as each client
takes responsibility for dma mapping.  Doing this in the core has some
downsides, maybe they're negligible: 1/ all drivers suffer the size
increase to dma_async_tx_descriptor 2/ pure polling usage models like
net_dma will now trigger dma channel interrupts to run the callback.

That being said this cleanup does get us a step closer to where
dmaengine needs to go, and I have not found time to do the "move it to
the client" patches mentioned above.  So I'm inclined to proceed with
them.  Does anyone still see positive benefits from net_dma?

> patch #8 adds missing DMA unmap on failure to ioat3 driver
>
> patch #9 fixes build for async_memset.c
>
> patch #10 adds missing DMA unmap on failure to async tx core
> code (async_memset())

Ack patches 8-10

>
> patches #11-18 fix async_tx core code and dmatest driver to do
> DMA unmap for MEMSET, XOR, XOR_VAL, PQ and PQ_VAL operations
>

Some comments in the patch.

> patches #19-20 remove no longer needed DMA unmap code from
> device drivers and DMA unmap flags from code code
>
>
> This patchset was tested on PL330 DMA controller using MEMCPY
> operations.  It would be great if somebody could test it on
> more advanced controller capable of MEMSET, XOR, XOR_VAL,
> PQ and PQ_VAL operations (especially since the conversion of
> XOR and PQ operations was not obvious).

async_tx has a bug in that it permits overlapping dma mappings or
operations that cross dma boundaries, but these patches don't make
this worse.  The interim "fix" I am proposing for this to drop support
for channel configurations that require an operation chain to switch
channels.  This will allow the removal the async_tx_ack() machinery
and hopefully prompt channel switching users to contribute to the md
changes needed to support this properly.  In the meantime the
async_tx_ack bits are just adding needless complexity to the drivers
that don't care about that functionality.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists