Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760842AbZLOSXw (ORCPT ); Tue, 15 Dec 2009 13:23:52 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1760828AbZLOSXp (ORCPT ); Tue, 15 Dec 2009 13:23:45 -0500 Received: from mga09.intel.com ([134.134.136.24]:25022 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760781AbZLOSXn (ORCPT ); Tue, 15 Dec 2009 13:23:43 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.47,401,1257148800"; d="scan'208";a="476345478" Message-ID: <4B27D42C.8070309@intel.com> Date: Tue, 15 Dec 2009 11:23:40 -0700 From: Dan Williams User-Agent: Thunderbird 2.0.0.14 (Windows/20080421) MIME-Version: 1.0 To: Suresh Vishnu-B05022 CC: "linuxppc-dev@ozlabs.org" , "linux-kernel@vger.kernel.org" , "linux-raid@vger.kernel.org" , "linux-crypto@vger.kernel.org" , "herbert@gondor.apana.org.au" , Tabi Timur-B04825 , Li Yang-R58472 , Ira Snyder Subject: Re: [PATCH v0] Crypto: Talitos: re-initialize async_tx descriptors References: <1260797602-7476-1-git-send-email-Vishnu@freescale.com> <76952DF81420A349876E087D089DF034562C75@zin33exm21.fsl.freescale.net> In-Reply-To: <76952DF81420A349876E087D089DF034562C75@zin33exm21.fsl.freescale.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2689 Lines: 66 Suresh Vishnu-B05022 wrote: > > 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 = NULL; > > > + new->async_tx.next = NULL; > > > + > > > > > > desc = &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, > so that if the changes are needed and can be done in > dma_async_tx_descriptor_init(), > these patches can be ignored. > > 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. > 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). > > When used without fsldma, this driver works fine. > Does iop_adma and ppx4xx work in similar enviroment? Yes, iop_adma when running on iop3xx hardware has one xor channel and two memcpy channels. The bug may very well be in the fsldma driver, but the workarounds are just band aids. I would be more comfortable with dropping the three workaround patches and simply adding "depends on !FSL_DMA" to the CRYPTO_DEV_TALITOS option until the true fix can be developed. -- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/