From: Liu Qiang-B32616 Subject: RE: [PATCH v7 1/8] Talitos: Support for async_tx XOR offload Date: Fri, 31 Aug 2012 10:41:08 +0000 Message-ID: References: <1344500448-10927-1-git-send-email-qiang.liu@freescale.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT Cc: Li Yang-R58472 , Phillips Kim-R1AAHA , "vinod.koul@intel.com" , Dan Williams , "arnd@arndb.de" , "gregkh@linuxfoundation.org" To: Geanta Neag Horia Ioan-B05471 , "linux-crypto@vger.kernel.org" , "dan.j.williams@gmail.com" , "herbert@gondor.hengli.com.au" , "davem@davemloft.net" , "linux-kernel@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" Return-path: Received: from va3ehsobe003.messaging.microsoft.com ([216.32.180.13]:20618 "EHLO va3outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752300Ab2HaKlP convert rfc822-to-8bit (ORCPT ); Fri, 31 Aug 2012 06:41:15 -0400 In-Reply-To: Content-Language: en-US Sender: linux-crypto-owner@vger.kernel.org List-ID: > -----Original Message----- > From: Geanta Neag Horia Ioan-B05471 > Sent: Friday, August 31, 2012 6:39 PM > To: Liu Qiang-B32616; linux-crypto@vger.kernel.org; > dan.j.williams@gmail.com; herbert@gondor.hengli.com.au; > davem@davemloft.net; linux-kernel@vger.kernel.org; linuxppc- > dev@lists.ozlabs.org > Cc: Li Yang-R58472; Phillips Kim-R1AAHA; vinod.koul@intel.com; Dan > Williams; arnd@arndb.de; gregkh@linuxfoundation.org > Subject: RE: [PATCH v7 1/8] Talitos: Support for async_tx XOR offload > > On Fri, 31 Aug 2012 06:08:05 +0300, Liu Qiang-B32616 > > wrote: > > > -----Original Message----- > > > From: Geanta Neag Horia Ioan-B05471 > > > Sent: Thursday, August 30, 2012 10:23 PM > > > To: Liu Qiang-B32616; linux-crypto@vger.kernel.org; > > > dan.j.williams@gmail.com; herbert@gondor.hengli.com.au; > > > davem@davemloft.net; linux-kernel@vger.kernel.org; linuxppc- > > > dev@lists.ozlabs.org > > > Cc: Li Yang-R58472; Phillips Kim-R1AAHA; vinod.koul@intel.com; > > > dan.j.williams@intel.com; arnd@arndb.de; gregkh@linuxfoundation.org; > Liu > > > Qiang-B32616 > > > Subject: RE: [PATCH v7 1/8] Talitos: Support for async_tx XOR offload > > > > > > On Thu, 9 Aug 2012 11:20:48 +0300, qiang.liu@freescale.com wrote: > > > > From: Qiang Liu > > > > > > > > Expose Talitos's XOR functionality to be used for RAID parity > > > > calculation via the Async_tx layer. > > > > > > > > Cc: Herbert Xu > > > > Cc: David S. Miller > > > > Signed-off-by: Dipen Dudhat > > > > Signed-off-by: Maneesh Gupta > > > > Signed-off-by: Kim Phillips > > > > Signed-off-by: Vishnu Suresh > > > > Signed-off-by: Qiang Liu > > > > --- > > > > drivers/crypto/Kconfig | 9 + > > > > drivers/crypto/talitos.c | 413 > > > ++++++++++++++++++++++++++++++++++++++++++++++ > > > > drivers/crypto/talitos.h | 53 ++++++ > > > > 3 files changed, 475 insertions(+), 0 deletions(-) > > > > > > > > +static int talitos_alloc_chan_resources(struct dma_chan *chan) > > > > +{ > > > > + struct talitos_xor_chan *xor_chan; > > > > + struct talitos_xor_desc *desc; > > > > + LIST_HEAD(tmp_list); > > > > + int i; > > > > + > > > > + xor_chan = container_of(chan, struct talitos_xor_chan, > common); > > > > + > > > > + if (!list_empty(&xor_chan->free_desc)) > > > > + return xor_chan->total_desc; > > > > + > > > > + for (i = 0; i < TALITOS_MAX_DESCRIPTOR_NR; i++) { > > > > + desc = talitos_xor_alloc_descriptor(xor_chan, > > > > + GFP_KERNEL | GFP_DMA); > > > > > > talitos_xor_alloc_descriptor() is called here without holding > > > the xor_chan->desc_lock and it increments xor_chan->total_desc. > > > Isn't this an issue ? > > > > No, please refer to the code as below, > > + list_add_tail(&desc->node, &tmp_list); > > > > The list is temporary list, it will be merged to xor_chan->free_desc in > next step, here is protected > > by lock, > > + spin_lock_bh(&xor_chan->desc_lock); > > + list_splice_init(&tmp_list, &xor_chan->free_desc); > > + spin_unlock_bh(&xor_chan->desc_lock); > > I was not referring to the list, but to xor_chan->total_desc variable. > The following access: > talitos_alloc_chan_resources()->talitos_xor_alloc_descriptor()- > >total_desc++ > is not protected by the xor_chan->desc_lock. Ok, I will correct it in next series. Thanks.