2012-07-10 06:21:20

by Qiang Liu

[permalink] [raw]
Subject: [PATCH 4/4] Talitos: fix the issue of dma memory leak

An error will be happened when test with mass data:
"DMA-API: device driver tries to sync DMA memory it has not allocated";
"DMA-API: debugging out of memory - disabling"
dma mapping memory of request->desc is not released by right device,
it should be private->dev but not dev;

Cc: Herbert Xu <[email protected]>
Cc: David S. Miller <[email protected]>
Signed-off-by: Qiang Liu <[email protected]>
---
drivers/crypto/talitos.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 81f8497..a7da48c 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -264,7 +264,7 @@ static void flush_channel(struct device *dev, int ch, int error, int reset_ch)
else
status = error;

- dma_unmap_single(dev, request->dma_desc,
+ dma_unmap_single(priv->dev, request->dma_desc,
sizeof(struct talitos_desc),
DMA_BIDIRECTIONAL);

--
1.7.5.1


2012-07-10 21:25:58

by Timur Tabi

[permalink] [raw]
Subject: Re: [linuxppc-release] [PATCH 4/4] Talitos: fix the issue of dma memory leak

Qiang Liu wrote:
> An error will be happened when test with mass data:

Please don't use the phrase "fix the issue" in patch summaries. It's
redundant.

This patch should be titled,

"drivers/crypto: fix memory leak in Talitos driver"

> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> index 81f8497..a7da48c 100644
> --- a/drivers/crypto/talitos.c
> +++ b/drivers/crypto/talitos.c
> @@ -264,7 +264,7 @@ static void flush_channel(struct device *dev, int ch, int error, int reset_ch)
> else
> status = error;
>
> - dma_unmap_single(dev, request->dma_desc,
> + dma_unmap_single(priv->dev, request->dma_desc,

You have an indentation problem here.

--
Timur Tabi
Linux kernel developer at Freescale

2012-07-11 02:30:48

by Liu Qiang-B32616

[permalink] [raw]
Subject: RE: [linuxppc-release] [PATCH 4/4] Talitos: fix the issue of dma memory leak

> -----Original Message-----
> From: Tabi Timur-B04825
> Sent: Wednesday, July 11, 2012 5:26 AM
> To: Liu Qiang-B32616
> Cc: [email protected]; [email protected]; Herbert
> Xu; Li Yang-R58472; David S. Miller
> Subject: Re: [linuxppc-release] [PATCH 4/4] Talitos: fix the issue of dma
> memory leak
>
> Qiang Liu wrote:
> > An error will be happened when test with mass data:
>
> Please don't use the phrase "fix the issue" in patch summaries. It's
> redundant.
>
> This patch should be titled,
>
> "drivers/crypto: fix memory leak in Talitos driver"
>
> > diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c index
> > 81f8497..a7da48c 100644
> > --- a/drivers/crypto/talitos.c
> > +++ b/drivers/crypto/talitos.c
> > @@ -264,7 +264,7 @@ static void flush_channel(struct device *dev, int
> ch, int error, int reset_ch)
> > else
> > status = error;
> >
> > - dma_unmap_single(dev, request->dma_desc,
> > + dma_unmap_single(priv->dev, request->dma_desc,
>
> You have an indentation problem here.
My fault, I will correct it and resend again. Thanks.

>
> --
> Timur Tabi
> Linux kernel developer at Freescale

Subject: RE: [PATCH 4/4] Talitos: fix the issue of dma memory leak

On Tue, 10 Jul 2012 09:00:14 +0300, Qiang Liu <[email protected]> wrote:
> An error will be happened when test with mass data:
> "DMA-API: device driver tries to sync DMA memory it has not allocated";
> "DMA-API: debugging out of memory - disabling"
> dma mapping memory of request->desc is not released by right device,
> it should be private->dev but not dev;
>
> Cc: Herbert Xu <[email protected]>
> Cc: David S. Miller <[email protected]>
> Signed-off-by: Qiang Liu <[email protected]>
> ---
> drivers/crypto/talitos.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> index 81f8497..a7da48c 100644
> --- a/drivers/crypto/talitos.c
> +++ b/drivers/crypto/talitos.c
> @@ -264,7 +264,7 @@ static void flush_channel(struct device *dev, int
> ch, int error, int reset_ch)
> else
> status = error;
> - dma_unmap_single(dev, request->dma_desc,
> + dma_unmap_single(priv->dev, request->dma_desc,
> sizeof(struct talitos_desc),
> DMA_BIDIRECTIONAL);

Are you sure this fix applies to the upstream version of talitos?
(i.e. have you encountered the error while running on cryptodev.git ?)

Looks to me this is a fix for the not-upstreamed-yet NAPI patch
(which needs to be reworked according to Dave's feedback).

When you respin the patch series, consider removing this one.

Cheers,
Horia

2012-07-11 07:18:12

by Liu Qiang-B32616

[permalink] [raw]
Subject: RE: [PATCH 4/4] Talitos: fix the issue of dma memory leak

> -----Original Message-----
> From: Geanta Neag Horia Ioan-B05471
> Sent: Wednesday, July 11, 2012 3:09 PM
> To: Liu Qiang-B32616
> Cc: [email protected]; [email protected]; Li Yang-
> R58472; Phillips Kim-R1AAHA; Herbert Xu; David S. Miller
> Subject: RE: [PATCH 4/4] Talitos: fix the issue of dma memory leak
>
> On Tue, 10 Jul 2012 09:00:14 +0300, Qiang Liu <[email protected]>
> wrote:
> > An error will be happened when test with mass data:
> > "DMA-API: device driver tries to sync DMA memory it has not
> > allocated";
> > "DMA-API: debugging out of memory - disabling"
> > dma mapping memory of request->desc is not released by right device,
> > it should be private->dev but not dev;
> >
> > Cc: Herbert Xu <[email protected]>
> > Cc: David S. Miller <[email protected]>
> > Signed-off-by: Qiang Liu <[email protected]>
> > ---
> > drivers/crypto/talitos.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-) diff --git
> > a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c index
> > 81f8497..a7da48c 100644
> > --- a/drivers/crypto/talitos.c
> > +++ b/drivers/crypto/talitos.c
> > @@ -264,7 +264,7 @@ static void flush_channel(struct device *dev, int
> > ch, int error, int reset_ch)
> > else
> > status = error;
> > - dma_unmap_single(dev, request->dma_desc,
> > + dma_unmap_single(priv->dev, request->dma_desc,
> > sizeof(struct talitos_desc),
> > DMA_BIDIRECTIONAL);
>
> Are you sure this fix applies to the upstream version of talitos?
> (i.e. have you encountered the error while running on cryptodev.git ?)
>
I found it on our own git, I'm going to test it. I will send the result later.
Thanks.

> Looks to me this is a fix for the not-upstreamed-yet NAPI patch (which
> needs to be reworked according to Dave's feedback).
>
> When you respin the patch series, consider removing this one.
Thanks for your mention, I will remove it if I cannot reproduce it.

> Cheers,
> Horia
>