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

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

[Qiang Liu]

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 <herbert@gondor.apana.org.au>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
---
 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

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

[Timur Tabi]

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

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

[Liu Qiang-B32616]

> -----Original Message-----
> From: Tabi Timur-B04825
> Sent: Wednesday, July 11, 2012 5:26 AM
> To: Liu Qiang-B32616
> Cc: linux-crypto@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Herbert
> Xu; Li Yang-R58472; David S. Miller
> Subject: Re: [linuxppc-release] [PATCH 4/4] Talitos: fix the issue of dma
> memory leak
>=20
> Qiang Liu wrote:
> > An error will be happened when test with mass data:
>=20
> Please don't use the phrase "fix the issue" in patch summaries.  It's
> redundant.
>=20
> This patch should be titled,
>=20
> "drivers/crypto: fix memory leak in Talitos driver"
>=20
> > 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 =3D error;
> >
> > -		dma_unmap_single(dev, request->dma_desc,
> > +		 dma_unmap_single(priv->dev, request->dma_desc,
>=20
> You have an indentation problem here.
My fault, I will correct it and resend again. Thanks.

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

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

[Geanta Neag Horia Ioan-B05471]

On Tue, 10 Jul 2012 09:00:14 +0300, Qiang Liu <qiang.liu@freescale.com> wro=
te:
> 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;
>=20
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
> ---
>  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 =3D 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

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

[Liu Qiang-B32616]

> -----Original Message-----
> From: Geanta Neag Horia Ioan-B05471
> Sent: Wednesday, July 11, 2012 3:09 PM
> To: Liu Qiang-B32616
> Cc: linux-crypto@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; Li Yang-
> R58472; Phillips Kim-R1AAHA; Herbert Xu; David S. Miller
> Subject: RE: [PATCH 4/4] Talitos: fix the issue of dma memory leak
>=20
> On Tue, 10 Jul 2012 09:00:14 +0300, Qiang Liu <qiang.liu@freescale.com>
> 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 <herbert@gondor.apana.org.au>
> > Cc: David S. Miller <davem@davemloft.net>
> > Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
> > ---
> >  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 =3D error;
> > -		dma_unmap_single(dev, request->dma_desc,
> > +		 dma_unmap_single(priv->dev, request->dma_desc,
> >  				 sizeof(struct talitos_desc),
> >  				 DMA_BIDIRECTIONAL);
>=20
> Are you sure this fix applies to the upstream version of talitos?
> (i.e. have you encountered the error while running on cryptodev.git ?)
>=20
I found it on our own git, I'm going to test it. I will send the result lat=
er.
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).
>=20
> 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
>=20