[PATCH] dma driver: fix potential oom issue of fsldma
From: Xuelin Shi missing unmap sources and destinations while doing dequeue. Signed-off-by: Xuelin Shi --- drivers/dma/fsldma.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index 2209f75..aac85c3 100644 --- a/drivers/dma/fsldma.c +++ b/drivers/dma/fsldma.c @@ -522,6 +522,8 @@ static dma_cookie_t fsldma_run_tx_complete_actions(struct fsldma_chan *chan, chan_dbg(chan, "LD %p callback\n", desc); txd->callback(txd->callback_param); } + + dma_descriptor_unmap(txd); } /* Run any dependencies */ -- 1.8.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v5] dmaengine: Driver support for FSL RaidEngine device.
Hi,
Please see my comments inline.
Thanks,
Shi xuelin
> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Wednesday, February 11, 2015 09:06
> To: Shi Xuelin-B29237
> Cc: dan.j.willi...@intel.com; dmaeng...@vger.kernel.org; linuxppc-
> d...@lists.ozlabs.org; Rai Harninder-B01044; Burmi Naveen-B16502
> Subject: Re: [PATCH v5] dmaengine: Driver support for FSL RaidEngine
> device.
>
> On Mon, Dec 15, 2014 at 10:32:28AM +0800, xuelin@freescale.com wrote:
>
> > +/* Copy descriptor from per chan software queue into hardware job
> > +ring */ static void fsl_re_issue_pending(struct dma_chan *chan) {
> > + struct fsl_re_chan *re_chan;
> > + int avail;
> > + struct fsl_re_desc *desc, *_desc;
> > + unsigned long flags;
> > +
> > + re_chan = container_of(chan, struct fsl_re_chan, chan);
> > +
> > + spin_lock_irqsave(&re_chan->desc_lock, flags);
> > + avail = FSL_RE_SLOT_AVAIL(
> > + in_be32(&re_chan->jrregs->inbring_slot_avail));
> okay this seems slots avail in hw
> > +
> > + list_for_each_entry_safe(desc, _desc, &re_chan->submit_q, node) {
> > + if (!avail)
> > + break;
> and why break inside the loop. You shouldn't even enter this only to keep
> breaking!
This "if" is necessary because "avail--" is also in the loop.
>
>
> > +static void fsl_re_dequeue(unsigned long data) {
> > + struct fsl_re_chan *re_chan;
> > + struct fsl_re_desc *desc, *_desc;
> > + struct fsl_re_hw_desc *hwdesc;
> > + unsigned long flags;
> > + unsigned int count, oub_count;
> > + int found;
> > +
> > + re_chan = dev_get_drvdata((struct device *)data);
> > +
> > + fsl_re_cleanup_descs(re_chan);
> > +
> > + spin_lock_irqsave(&re_chan->desc_lock, flags);
> > + count = FSL_RE_SLOT_FULL(in_be32(&re_chan->jrregs-
> >oubring_slot_full));
> > + while (count--) {
> > + found = 0;
> > + hwdesc = &re_chan->oub_ring_virt_addr[re_chan->oub_count];
> > + list_for_each_entry_safe(desc, _desc, &re_chan->active_q,
> > +node) {
> > + /* compare the hw dma addr to find the completed */
> > + if (desc->hwdesc.lbea32 == hwdesc->lbea32 &&
> > + desc->hwdesc.addr_low == hwdesc->addr_low) {
> > + found = 1;
> > + break;
> > + }
> > + }
> > +
> > + BUG_ON(!found);
> you are killing the machine here. I don't think it too severe and can't
> be handled gracefully?
>
OK, BUG_ON should be avoided.
> > +static struct fsl_re_desc *fsl_re_chan_alloc_desc(struct fsl_re_chan
> *re_chan,
> > + unsigned long flags)
> > +{
> > + struct fsl_re_desc *desc = NULL;
> > + void *cf;
> > + dma_addr_t paddr;
> > + unsigned long lock_flag;
> > +
> > + fsl_re_cleanup_descs(re_chan);
> > +
> > + spin_lock_irqsave(&re_chan->desc_lock, lock_flag);
> > + if (!list_empty(&re_chan->free_q)) {
> > + /* take one desc from free_q */
> > + desc = list_first_entry(&re_chan->free_q,
> > + struct fsl_re_desc, node);
> > + list_del(&desc->node);
> > +
> > + desc->async_tx.flags = flags;
> > + }
> > + spin_unlock_irqrestore(&re_chan->desc_lock, lock_flag);
> > +
> > + if (!desc) {
> > + desc = kzalloc(sizeof(*desc), GFP_NOWAIT);
> > + cf = dma_pool_alloc(re_chan->re_dev->cf_desc_pool, GFP_NOWAIT,
> > + &paddr);
> > + if (!desc || !cf) {
> > + kfree(desc);
> and this is buggy, you can fail in desc while cf is okay..!
>
OK, will be revised.
> > + return NULL;
> > + }
> > +
> > + desc = fsl_re_init_desc(re_chan, desc, cf, paddr);
> > + desc->async_tx.flags = flags;
> > +
> > + spin_lock_irqsave(&re_chan->desc_lock, lock_flag);
> > + re_chan->alloc_count++;
> > + spin_unlock_irqrestore(&re_chan->desc_lock, lock_flag);
> > + }
> > +
> > + return desc;
> > +}
> > +
> > +static struct dma_async_tx_descriptor *fsl_re_prep_dma_genq(
> > + struct dma_chan *chan, dma_addr_t dest, dma_addr_t *src,
> > + unsigned int src_cnt, const unsigned char *scf, size_t len,
> > + unsigned long flags)
> > +{
> > + struct fsl_re_chan *re_chan;
> > + struct fsl_re_desc *desc;
> > + struct fsl_re_xor_cdb *xor;
> > + struct fsl_re_cmpnd_frame *cf;
> > + u32 cdb;
> > + unsigned int i, j;
> > + unsigned int save_src_cnt = src_cnt;
> > + int cont_q = 0;
> > +
> > + if (len > FSL_RE_MAX_DATA_LEN) {
> > + pr_err("Length greater than %d not supported\n",
> > + FSL_RE_MAX_DATA_LEN);
> printing length will be helpful
>
> btw whats wrong with dev_err. You do realize that someone who seems this
> will have no clue which device is reportin
RE: [PATCH v3] dmaengine: driver support for FSL RaidEngine device.
Hi Dan & Vinod, I have sent out the v4 of this patch and not received any further feedback yet. This patch looks ruled out from the patchwork. https://patchwork.kernel.org/project/linux-dmaengine/list/?page=2 So do you know what happened to this patch? Thanks, Xuelin Shi -Original Message- From: Shi Xuelin-B29237 Sent: 2014年4月15日 11:08 To: 'Dan Williams' Cc: Koul, Vinod; andriy.shevche...@intel.com; dmaeng...@vger.kernel.org; linuxppc-dev; Rai Harninder-B01044; Burmi Naveen-B16502 Subject: RE: [PATCH v3] dmaengine: driver support for FSL RaidEngine device. Yes, "depend on !ASYNC_TX_CHANNEL_SWITCH" is better since fsldma selects this condition. Thanks, Xuelin Shi -Original Message- From: Dan Williams [mailto:dan.j.willi...@intel.com] Sent: 2014年4月15日 8:30 To: Shi Xuelin-B29237 Cc: Koul, Vinod; andriy.shevche...@intel.com; dmaeng...@vger.kernel.org; linuxppc-dev; Rai Harninder-B01044; Burmi Naveen-B16502 Subject: Re: [PATCH v3] dmaengine: driver support for FSL RaidEngine device. On Sun, Apr 13, 2014 at 7:48 PM, Xuelin Shi wrote: > Hi Dan, > > fsl dma device and fsl raid device are two differenct devices that > both provide async_memcpy capability, so I use !FSL_DMA to disable the fsl > dma device. > > That's to say, either select fsldma device, either fsl raid device. > Right, but that's not what your proposed Kconfig dependency line does. You want something like "depends on FSL_SOC && !(FSL_DMA || FSL_DMA=m)" However, the more problematic option is ASYNC_TX_CHANNEL_SWITCH. That option is problematic for RAID, so I propose "depend on !ASYNC_TX_CHANNEL_SWITCH" since that addresses both problems. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v3] dmaengine: driver support for FSL RaidEngine device.
Yes, "depend on !ASYNC_TX_CHANNEL_SWITCH" is better since fsldma selects this condition. Thanks, Xuelin Shi -Original Message- From: Dan Williams [mailto:dan.j.willi...@intel.com] Sent: 2014年4月15日 8:30 To: Shi Xuelin-B29237 Cc: Koul, Vinod; andriy.shevche...@intel.com; dmaeng...@vger.kernel.org; linuxppc-dev; Rai Harninder-B01044; Burmi Naveen-B16502 Subject: Re: [PATCH v3] dmaengine: driver support for FSL RaidEngine device. On Sun, Apr 13, 2014 at 7:48 PM, Xuelin Shi wrote: > Hi Dan, > > fsl dma device and fsl raid device are two differenct devices that > both provide async_memcpy capability, so I use !FSL_DMA to disable the fsl > dma device. > > That's to say, either select fsldma device, either fsl raid device. > Right, but that's not what your proposed Kconfig dependency line does. You want something like "depends on FSL_SOC && !(FSL_DMA || FSL_DMA=m)" However, the more problematic option is ASYNC_TX_CHANNEL_SWITCH. That option is problematic for RAID, so I propose "depend on !ASYNC_TX_CHANNEL_SWITCH" since that addresses both problems. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v3] dmaengine: driver support for FSL RaidEngine device.
Hi Dan,
fsl dma device and fsl raid device are two differenct devices that both provide
async_memcpy
capability, so I use !FSL_DMA to disable the fsl dma device.
That's to say, either select fsldma device, either fsl raid device.
Thanks,
Xuelin Shi
-Original Message-
From: dan.j.willi...@gmail.com [mailto:dan.j.willi...@gmail.com] On Behalf Of
Dan Williams
Sent: 2014年4月12日 1:43
To: Shi Xuelin-B29237
Cc: Koul, Vinod; andriy.shevche...@intel.com; dmaeng...@vger.kernel.org;
linuxppc-dev; Rai Harninder-B01044; Burmi Naveen-B16502
Subject: Re: [PATCH v3] dmaengine: driver support for FSL RaidEngine device.
A few more comments:
On Fri, Apr 11, 2014 at 12:41 AM, wrote:
> From: Xuelin Shi
>
> The RaidEngine is a new FSL hardware used for Raid5/6 acceration.
>
> This patch enables the RaidEngine functionality and provides hardware
> offloading capability for memcpy, xor and pq computation.
> It works with async_tx.
>
> Signed-off-by: Harninder Rai
> Signed-off-by: Naveen Burmi
> Signed-off-by: Xuelin Shi
> ---
> changes for v3:
> - fix memory allocation flag GFP_xxx usage.
> - add re_jr_issue_pending call in cleanup.
> - remove unnecessary dma_run_dependencies(...).
> - use dma_cookie_complete(...) instead of direct updating cookie.
>
> drivers/dma/Kconfig| 11 +
> drivers/dma/Makefile | 1 +
> drivers/dma/fsl_raid.c | 878
> +
> drivers/dma/fsl_raid.h | 308 +
> 4 files changed, 1198 insertions(+)
> create mode 100644 drivers/dma/fsl_raid.c create mode 100644
> drivers/dma/fsl_raid.h
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index
> 605b016..829f41c 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -100,6 +100,17 @@ config FSL_DMA
> EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3 is on
> some Txxx and Bxxx parts.
>
> +config FSL_RAID
> +tristate "Freescale RAID engine Support"
> +depends on FSL_SOC && !FSL_DMA
This won't work in the case where FSL_DMA=m.
Instead, I think you want to use "depends on !ASYNC_TX_ENABLE_CHANNEL_SWITCH"
which is the actual dependency.
> +select DMA_ENGINE
> +select DMA_ENGINE_RAID
> +---help---
> + Enable support for Freescale RAID Engine. RAID Engine is
> + available on some QorIQ SoCs (like P5020). It has
> + the capability to offload memcpy, xor and pq computation
> + for raid5/6.
> +
[..]
> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c new file
> mode 100644 index 000..4b389b1
> --- /dev/null
> +++ b/drivers/dma/fsl_raid.c
> @@ -0,0 +1,878 @@
[..]
> +void fill_cfd_frame(struct cmpnd_frame *cf, u8 index,
> + size_t length, dma_addr_t addr, bool final) {
> + u32 efrl = length & CF_LENGTH_MASK;
> + efrl |= final << CF_FINAL_SHIFT;
> + cf[index].efrl32 = efrl;
> + cf[index].addr_high = (addr >> 32) & HWDESC_ADDR_HIGH_MASK;
Use "upper_32_bits()" here otherwise:
drivers/dma/fsl_raid.c: In function 'fill_cfd_frame':
drivers/dma/fsl_raid.c:258:2: warning: right shift count >= width of type
> + cf[index].addr_low = (u32)addr;
Use lower_32_bits() here to be symmetrical.
> +}
> +
> +static struct fsl_re_dma_async_tx_desc *re_jr_init_desc(struct re_jr *jr,
> + struct fsl_re_dma_async_tx_desc *desc, void *cf, dma_addr_t
> +paddr) {
> + desc->jr = jr;
> + desc->async_tx.tx_submit = re_jr_tx_submit;
> + dma_async_tx_descriptor_init(&desc->async_tx, &jr->chan);
> + INIT_LIST_HEAD(&desc->node);
> +
> + desc->hwdesc.fmt32 = FRAME_FORMAT << HWDESC_FMT_SHIFT;
> + desc->hwdesc.lbea32 = (paddr >> 32) & HWDESC_ADDR_HIGH_MASK;
Same comment/warning as above...
> + desc->hwdesc.addr_low = (u32)paddr;
ditto.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v2] fix wrong usage of dmaengine_unmap_put in async_xxx
Hi Dan, With https://patchwork.kernel.org/patch/3863711/ applied, the issue disappeared. Thanks, Xuelin Shi -Original Message- From: dan.j.willi...@gmail.com [mailto:dan.j.willi...@gmail.com] On Behalf Of Dan Williams Sent: 2014年4月10日 12:02 To: Shi Xuelin-B29237 Cc: Koul, Vinod; dmaeng...@vger.kernel.org; linuxppc-dev Subject: Re: [PATCH v2] fix wrong usage of dmaengine_unmap_put in async_xxx On Thu, Mar 20, 2014 at 1:16 AM, wrote: > From: Xuelin Shi > > dmaengine_unmap_put does below two things: > a) unmap pages for srcs and dests > b) free unmap struct > > The unmap struct data is generated but only initialized while other > some dma contions are met, like dma alignment etc. > If the unmap data is not initialized, call dmaengine_unmap_put will > unmap some random data in unmap->addr[...] Actually, this should be fixed by your other patch: https://patchwork.kernel.org/patch/3863711/ Because the to_cnt, from_cnt, are properly initialized to zero. The only issue was that the unmap pool was not specified. Can you verify that the problem still exists with that patch applied? I'll mark it for -stable. > Also call dmaengine_get_unmap_data immediatally after generating tx is > not correct. Maybe the tx has not been finished by DMA hardware yet > but the srcs and dests are dma unmapped. I disagree. It is correct because the unmap data is reference counted. If the DMA hardware is still using the buffers then it must hold a reference on the unmap data. The dmaengine_put_unmap_data() instances your are removing are the ones that drop the initial reference count set in dmaengine_get_unmap_data(). ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH v2] fix wrong usage of dmaengine_unmap_put in async_xxx
Hi Dan & vinod,
Have you got a chance to see this patch?
Also what's your comment about the RaidEngine driver patch?
Thanks,
Xuelin Shi
-Original Message-
From: xuelin@freescale.com [mailto:xuelin@freescale.com]
Sent: 2014年3月20日 16:16
To: vinod.k...@intel.com; dan.j.willi...@intel.com
Cc: dmaeng...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Shi Xuelin-B29237
Subject: [PATCH v2] fix wrong usage of dmaengine_unmap_put in async_xxx
From: Xuelin Shi
dmaengine_unmap_put does below two things:
a) unmap pages for srcs and dests
b) free unmap struct
The unmap struct data is generated but only initialized while other some dma
contions are met, like dma alignment etc.
If the unmap data is not initialized, call dmaengine_unmap_put will unmap some
random data in unmap->addr[...]
Also call dmaengine_get_unmap_data immediatally after generating tx is not
correct. Maybe the tx has not been finished by DMA hardware yet but the srcs
and dests are dma unmapped.
This patch fixed above two issues by:
a) only generates unmap struct data when other dma conditions are met.
b) eliminates dmaengine_unmap_put when tx is generated because tx knowes the
best time to unmap it (in interrupt processing).
Signed-off-by: Xuelin Shi
---
change log:
v1: include change in async_memcpy, async_xor, async_pq
v2: add change in async_raid6_recov.c and fix some style issue
crypto/async_tx/async_memcpy.c | 80 ---
crypto/async_tx/async_pq.c | 189 +++-
crypto/async_tx/async_raid6_recov.c | 108 +++--
crypto/async_tx/async_xor.c | 164 ---
4 files changed, 286 insertions(+), 255 deletions(-)
diff --git a/crypto/async_tx/async_memcpy.c b/crypto/async_tx/async_memcpy.c
index f8c0b8d..6546e87 100644
--- a/crypto/async_tx/async_memcpy.c
+++ b/crypto/async_tx/async_memcpy.c
@@ -51,11 +51,10 @@ async_memcpy(struct page *dest, struct page *src, unsigned
int dest_offset,
struct dma_device *device = chan ? chan->device : NULL;
struct dma_async_tx_descriptor *tx = NULL;
struct dmaengine_unmap_data *unmap = NULL;
+ void *dest_buf, *src_buf;
- if (device)
- unmap = dmaengine_get_unmap_data(device->dev, 2, GFP_NOIO);
-
- if (unmap && is_dma_copy_aligned(device, src_offset, dest_offset, len))
{
+ if (device &&
+ is_dma_copy_aligned(device, src_offset, dest_offset, len)) {
unsigned long dma_prep_flags = 0;
if (submit->cb_fn)
@@ -63,45 +62,56 @@ async_memcpy(struct page *dest, struct page *src, unsigned
int dest_offset,
if (submit->flags & ASYNC_TX_FENCE)
dma_prep_flags |= DMA_PREP_FENCE;
- unmap->to_cnt = 1;
- unmap->addr[0] = dma_map_page(device->dev, src, src_offset, len,
- DMA_TO_DEVICE);
- unmap->from_cnt = 1;
- unmap->addr[1] = dma_map_page(device->dev, dest, dest_offset,
len,
- DMA_FROM_DEVICE);
- unmap->len = len;
-
- tx = device->device_prep_dma_memcpy(chan, unmap->addr[1],
- unmap->addr[0], len,
- dma_prep_flags);
+ unmap = dmaengine_get_unmap_data(device->dev, 2, GFP_NOIO);
+ if (unmap) {
+ unmap->to_cnt = 1;
+ unmap->addr[0] = dma_map_page(device->dev, src,
+ src_offset, len,
+ DMA_TO_DEVICE);
+ unmap->from_cnt = 1;
+ unmap->addr[1] = dma_map_page(device->dev, dest,
+ dest_offset, len,
+ DMA_FROM_DEVICE);
+ unmap->len = len;
+
+ tx = device->device_prep_dma_memcpy(chan,
+ unmap->addr[1],
+ unmap->addr[0],
+ len,
+ dma_prep_flags);
+ if (tx) {
+ pr_debug("%s: (async) len: %zu\n", __func__,
+len);
+
+ dma_set_unmap(tx, unmap);
+ async_tx_submit(chan, tx, submit);
+ return tx;
+ }
+
+ /* could not get a descriptor, unmap and fall through to
+
RE: [PATCH v2] fix wrong usage of dmaengine_unmap_put in async_xxx
Yes, this patch is run by checkpatch and found 0 errors, 0 warnings.
-Original Message-
From: Shevchenko, Andriy [mailto:andriy.shevche...@intel.com]
Sent: 2014年3月20日 17:45
To: Shi Xuelin-B29237
Cc: Koul, Vinod; Williams, Dan J; dmaeng...@vger.kernel.org;
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2] fix wrong usage of dmaengine_unmap_put in async_xxx
On Thu, 2014-03-20 at 16:16 +0800, xuelin@freescale.com wrote:
> From: Xuelin Shi
>
> dmaengine_unmap_put does below two things:
> a) unmap pages for srcs and dests
> b) free unmap struct
>
> The unmap struct data is generated but only initialized while other
> some dma contions are met, like dma alignment etc.
> If the unmap data is not initialized, call dmaengine_unmap_put will
> unmap some random data in unmap->addr[...]
>
> Also call dmaengine_get_unmap_data immediatally after generating tx is
> not correct. Maybe the tx has not been finished by DMA hardware yet
> but the srcs and dests are dma unmapped.
>
> This patch fixed above two issues by:
> a) only generates unmap struct data when other dma conditions are met.
> b) eliminates dmaengine_unmap_put when tx is generated because tx
> knowes the best time to unmap it (in interrupt processing).
Have you run checkpatch.pl against this one?
>
> Signed-off-by: Xuelin Shi
> ---
> change log:
> v1: include change in async_memcpy, async_xor, async_pq
> v2: add change in async_raid6_recov.c and fix some style issue
>
> crypto/async_tx/async_memcpy.c | 80 ---
> crypto/async_tx/async_pq.c | 189
> +++-
> crypto/async_tx/async_raid6_recov.c | 108 +++--
> crypto/async_tx/async_xor.c | 164 ---
> 4 files changed, 286 insertions(+), 255 deletions(-)
>
> diff --git a/crypto/async_tx/async_memcpy.c
> b/crypto/async_tx/async_memcpy.c index f8c0b8d..6546e87 100644
> --- a/crypto/async_tx/async_memcpy.c
> +++ b/crypto/async_tx/async_memcpy.c
> @@ -51,11 +51,10 @@ async_memcpy(struct page *dest, struct page *src,
> unsigned int dest_offset,
> struct dma_device *device = chan ? chan->device : NULL;
> struct dma_async_tx_descriptor *tx = NULL;
> struct dmaengine_unmap_data *unmap = NULL;
> + void *dest_buf, *src_buf;
>
> - if (device)
> - unmap = dmaengine_get_unmap_data(device->dev, 2, GFP_NOIO);
> -
> - if (unmap && is_dma_copy_aligned(device, src_offset, dest_offset, len))
> {
> + if (device &&
> + is_dma_copy_aligned(device, src_offset, dest_offset, len)) {
> unsigned long dma_prep_flags = 0;
>
> if (submit->cb_fn)
> @@ -63,45 +62,56 @@ async_memcpy(struct page *dest, struct page *src,
> unsigned int dest_offset,
> if (submit->flags & ASYNC_TX_FENCE)
> dma_prep_flags |= DMA_PREP_FENCE;
>
> - unmap->to_cnt = 1;
> - unmap->addr[0] = dma_map_page(device->dev, src, src_offset, len,
> - DMA_TO_DEVICE);
> - unmap->from_cnt = 1;
> - unmap->addr[1] = dma_map_page(device->dev, dest, dest_offset,
> len,
> - DMA_FROM_DEVICE);
> - unmap->len = len;
> -
> - tx = device->device_prep_dma_memcpy(chan, unmap->addr[1],
> - unmap->addr[0], len,
> - dma_prep_flags);
> + unmap = dmaengine_get_unmap_data(device->dev, 2, GFP_NOIO);
> + if (unmap) {
> + unmap->to_cnt = 1;
> + unmap->addr[0] = dma_map_page(device->dev, src,
> + src_offset, len,
> + DMA_TO_DEVICE);
> + unmap->from_cnt = 1;
> + unmap->addr[1] = dma_map_page(device->dev, dest,
> + dest_offset, len,
> + DMA_FROM_DEVICE);
> + unmap->len = len;
> +
> + tx = device->device_prep_dma_memcpy(chan,
> + unmap->addr[1],
> + unmap->addr[0],
> + len,
> + dma_prep_flags);
> + if (tx) {
> +
RE: [PATCH] fix dmaengine_unmap failure.
Hi Dan, I'm OK to save memory here. I'd like to send v2. Thanks, Xuelin Shi -Original Message- From: Dan Williams [mailto:dan.j.willi...@intel.com] Sent: 2014年3月19日 23:44 To: Shi Xuelin-B29237 Cc: Vinod Koul; linuxppc-dev; dmaeng...@vger.kernel.org Subject: Re: [PATCH] fix dmaengine_unmap failure. On Tue, Mar 18, 2014 at 11:39 PM, Xuelin Shi wrote: > Hi Dan, > > In async_mult(...) of async_raid6_recov.c, the count 3 is used to request an > unmap. > However the to_cnt and bidi_cnt are both set to 1 and from_cnt to 0. > Then while trying to do unmap, we are getting the wrong "unmap" from a > different mempool. > > In this patch, the mempool is associated with the unmap structure instead of > computing it again. > By this way, it is guaranteed that the unmap is the same when we get and put > the unmap data. > > BTW: the mempool is just used to manage the struct unmap, not the pages. > I see, what about just storing the map_cnt at allocation time? It could be another byte in struct dmaengine_unmap_data rather than an 8 byte pointer. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] fix dmaengine_unmap failure.
Hi Dan, In async_mult(...) of async_raid6_recov.c, the count 3 is used to request an unmap. However the to_cnt and bidi_cnt are both set to 1 and from_cnt to 0. Then while trying to do unmap, we are getting the wrong "unmap" from a different mempool. In this patch, the mempool is associated with the unmap structure instead of computing it again. By this way, it is guaranteed that the unmap is the same when we get and put the unmap data. BTW: the mempool is just used to manage the struct unmap, not the pages. Thanks, Xuelin Shi -Original Message- From: Dan Williams [mailto:dan.j.willi...@intel.com] Sent: 2014年3月19日 1:14 To: Shi Xuelin-B29237 Cc: Vinod Koul; linuxppc-dev; dmaeng...@vger.kernel.org Subject: Re: [PATCH] fix dmaengine_unmap failure. On Tue, Mar 18, 2014 at 1:32 AM, wrote: > From: Xuelin Shi > > The count which is used to get_unmap_data maybe not the same as the > count computed in dmaengine_unmap which causes to free data in a wrong > pool. > > This patch fixes this issue by keeping the pool in unmap_data > structure. Won't this free the entire count anyways? In what scenario is the count different at unmap? ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
