From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Neuling Subject: Re: [PATCH V3 6/6] crypto/nx: Add P9 NX support for 842 compression engine Date: Sat, 02 Sep 2017 23:42:36 +1000 Message-ID: <1504359756.4728.7.camel@neuling.org> References: <1500699702.23205.8.camel@hbabu-laptop> <59AA6E71.8070704@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Cc: Herbert Xu , Ram Pai , npiggin@gmail.com, suka@us.ibm.com, Linux Crypto Mailing List , "linuxppc-dev@lists.ozlabs.org" To: Haren Myneni , Dan Streetman Return-path: Received: from ozlabs.org ([103.22.144.67]:54865 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752441AbdIBNmi (ORCPT ); Sat, 2 Sep 2017 09:42:38 -0400 In-Reply-To: <59AA6E71.8070704@linux.vnet.ibm.com> Sender: linux-crypto-owner@vger.kernel.org List-ID: > > > +static int nx842_powernv_crypto_init_vas(struct crypto_tfm *tfm) > > > +{ > > > +       struct nx842_crypto_ctx *ctx = crypto_tfm_ctx(tfm); > > > +       struct nx842_workmem *wmem; > > > +       struct nx842_coproc *coproc; > > > +       int ret; > > > + > > > +       ret = nx842_crypto_init(tfm, &nx842_powernv_driver); > > > + > > > +       if (ret) > > > +               return ret; > > > + > > > +       wmem = PTR_ALIGN((struct nx842_workmem *)ctx->wmem, > > > WORKMEM_ALIGN); > > > +       coproc = per_cpu(coproc_inst, smp_processor_id()); > > > > this is wrong.  the crypto transform init function is not guaranteed > > to be called by the same processor that later uses it.  Just because > > that happens to be how zswap operates doesn't guarantee other crypto > > users will do the same. > > Dan, Sorry missed this comment.  > > Right, The actual crypto request can be executed on other processor than the > CPU when the init is executed. The main goal is open send window on the NX > engine which is on the same chip for the corresponding CPU. So we are OK if > the request is scheduled on other CPU as long as it belongs to same chip. > Otherwise in the worst case we will end up using remote NX.   You want the NX to be close to the requester CPU, but probably more importantly you want the NX close to the memory it's going to be operating on.  Preferably they would all be on the same node. Mikey From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ozlabs.org (ozlabs.org [IPv6:2401:3900:2:1::2]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3xky3J5K6fzDqXs for ; Sat, 2 Sep 2017 23:42:36 +1000 (AEST) Message-ID: <1504359756.4728.7.camel@neuling.org> Subject: Re: [PATCH V3 6/6] crypto/nx: Add P9 NX support for 842 compression engine From: Michael Neuling To: Haren Myneni , Dan Streetman Cc: Herbert Xu , Ram Pai , npiggin@gmail.com, suka@us.ibm.com, Linux Crypto Mailing List , "linuxppc-dev@lists.ozlabs.org" Date: Sat, 02 Sep 2017 23:42:36 +1000 In-Reply-To: <59AA6E71.8070704@linux.vnet.ibm.com> References: <1500699702.23205.8.camel@hbabu-laptop> <59AA6E71.8070704@linux.vnet.ibm.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , > > > +static int nx842_powernv_crypto_init_vas(struct crypto_tfm *tfm) > > > +{ > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct nx842_crypto_ctx *c= tx =3D crypto_tfm_ctx(tfm); > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct nx842_workmem *wmem= ; > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct nx842_coproc *copro= c; > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0int ret; > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ret =3D nx842_crypto_init(= tfm, &nx842_powernv_driver); > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (ret) > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0return ret; > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0wmem =3D PTR_ALIGN((struct= nx842_workmem *)ctx->wmem, > > > WORKMEM_ALIGN); > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0coproc =3D per_cpu(coproc_= inst, smp_processor_id()); > >=20 > > this is wrong.=C2=A0=C2=A0the crypto transform init function is not gua= ranteed > > to be called by the same processor that later uses it.=C2=A0=C2=A0Just = because > > that happens to be how zswap operates doesn't guarantee other crypto > > users will do the same. >=20 > Dan, Sorry missed this comment.=C2=A0 >=20 > Right, The actual crypto request can be executed on other processor than = the > CPU when the init is executed. The main goal is open send window on the N= X > engine which is on the same chip for the corresponding CPU. So we are OK = if > the request is scheduled on other CPU as long as it belongs to same chip. > Otherwise in the worst case we will end up using remote NX.=C2=A0=C2=A0 You want the NX to be close to the requester CPU, but probably more importa= ntly you want the NX close to the memory it's going to be operating on.=C2=A0 Preferably they would all be on the same node. Mikey=20