From: "Suresh Vishnu-B05022" Subject: RE: [PATCH v0] Crypto: Talitos: re-initialize async_tx descriptors Date: Tue, 15 Dec 2009 16:33:42 +0530 Message-ID: <76952DF81420A349876E087D089DF034562C75@zin33exm21.fsl.freescale.net> References: <1260797602-7476-1-git-send-email-Vishnu@freescale.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2599548354204149037==" Cc: herbert@gondor.apana.org.au, Tabi Timur-B04825 , linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org, linuxppc-dev@ozlabs.org, linux-crypto@vger.kernel.org, Li Yang-R58472 To: "Dan Williams" Return-path: Content-class: urn:content-classes:message List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linuxppc-dev-bounces+glppd-linuxppc64-dev=m.gmane.org@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+glppd-linuxppc64-dev=m.gmane.org@lists.ozlabs.org List-Id: linux-crypto.vger.kernel.org This is a multi-part message in MIME format. --===============2599548354204149037== Content-class: urn:content-classes:message Content-Type: multipart/alternative; boundary="----_=_NextPart_001_01CA7D76.437CCC34" This is a multi-part message in MIME format. ------_=_NextPart_001_01CA7D76.437CCC34 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable > On Mon, Dec 14, 2009 at 6:33 AM, Vishnu Suresh = wrote: > > The async_tx descriptors contains dangling pointers. > > Hence, re-initialize them to NULL before use. > > > > Signed-off-by: Vishnu Suresh > > --- > > o. Rebased to linux-next as of 20091214 > > > > drivers/crypto/talitos.c | 3 +++ > > 1 files changed, 3 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c > > index 87f06be..9e261c6 100644 > > --- a/drivers/crypto/talitos.c > > +++ b/drivers/crypto/talitos.c > > @@ -952,6 +952,9 @@ static struct dma_async_tx_descriptor * = talitos_prep_dma_xor( > > return NULL; > > } > > dma_async_tx_descriptor_init(&new->async_tx, = &xor_chan->common); > > + new->async_tx.parent =3D NULL; > > + new->async_tx.next =3D NULL; > > + > > > > desc =3D &new->hwdesc; > > /* Set destination: Last pointer pair */ > These two values are owned by the async_tx api, drivers are not > supposed to touch them. I have sent this patch and the similar one for fsldma seperately,=20 so that if the changes are needed and can be done in = dma_async_tx_descriptor_init(),=20 these patches can be ignored. =20 > Both iop_adma and the new ppx4xx driver > (which use the async_tx channel switching capability) get away without > touching these fields which makes me suspect there is a > misunderstanding/bug somewhere else in the talitos implementation. This bug does not occur on all the platforms. The occurrance is random.=20 This occurs when a channel switch between two different devices are = present.=20 This same initialization is required in case of fsldma as well.=20 In case of fsldma/talitosXOR, there are two DMA channels (on same = device) and single XOR channel (on another device).=20 When used without fsldma, this driver works fine. Does iop_adma and ppx4xx work in similar enviroment? > > Also that dma_async_tx_descriptor_init() is unexpected in the hot > > path, it's only needed at initial descriptor allocation. End result = I > > think this driver needs some more time to brew. > -- > Dan ------_=_NextPart_001_01CA7D76.437CCC34 Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Re: [PATCH v0] Crypto: Talitos: = re-initialize async_tx descriptors=0A= =0A= =0A= =0A=
=0A=
> On Mon, Dec 14, 2009 at 6:33 AM, = Vishnu Suresh <Vishnu@freescale.com> wrote:
> > The = async_tx descriptors contains dangling pointers.
> > Hence, = re-initialize them to NULL before use.
> >
> > = Signed-off-by: Vishnu Suresh <Vishnu@freescale.com>
> > = ---
> > o. Rebased to linux-next as of 20091214
> = >
> >  drivers/crypto/talitos.c |    3 = +++
> >  1 files changed, 3 insertions(+), 0 = deletions(-)
> >
> > diff --git = a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> > index = 87f06be..9e261c6 100644
> > --- = a/drivers/crypto/talitos.c
> > +++ = b/drivers/crypto/talitos.c
> > @@ -952,6 +952,9 @@ static = struct dma_async_tx_descriptor * talitos_prep_dma_xor(
> > =                return = NULL;
> >        }
> >   =      dma_async_tx_descriptor_init(&new->async_tx, = &xor_chan->common);
> > +       = new->async_tx.parent =3D NULL;
> > +       = new->async_tx.next =3D NULL;
> > +
> >
> > =        desc =3D &new->hwdesc;
> > =        /* Set destination: Last pointer pair = */

> These two values are owned by the async_tx api, drivers = are not
> supposed to touch them.
=0A=
I have sent this patch and = the similar one for fsldma seperately,
so that if the = changes are needed and can be done in = dma_async_tx_descriptor_init(),
these patches can be ignored.  =
=0A=
> Both iop_adma and the new ppx4xx = driver
> (which use the async_tx channel switching capability) get = away without
> touching these fields which makes me suspect there = is a
> misunderstanding/bug somewhere else in the talitos = implementation.
=0A=
=0A=

This bug does not occur on all the = platforms. The occurrance is random.
This occurs when a channel = switch between two different devices are present.
This same initialization is required in case of fsldma as = well. 
In case of fsldma/talitosXOR, there are two DMA = channels (on same device) and single XOR channel (on another device). =

=0A=

When used without fsldma, this driver works = fine.
Does iop_adma and ppx4xx work in similar enviroment?

=0A=

> > Also that dma_async_tx_descriptor_init() is = unexpected in the hot
> > path, it's only needed at initial = descriptor allocation.  End result I
> > think this driver = needs some more time to brew.

=0A=



> --
> = Dan

------_=_NextPart_001_01CA7D76.437CCC34-- --===============2599548354204149037== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev --===============2599548354204149037==--