From: "Nair, Vishnu" Subject: Re: [RFC PATCH v1 1/1] mm: zswap - Add crypto acomp/scomp framework support Date: Mon, 20 Feb 2017 09:52:57 +0000 Message-ID: References: <1487086821-5880-1-git-send-email-Mahipal.Challa@cavium.com> <1487086821-5880-2-git-send-email-Mahipal.Challa@cavium.com> <58A45E4A.8080508@caviumnetworks.com>,<20170215221208.GA820@silv-gc1.ir.intel.com> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="_000_MWHPR07MB2943F04F607146A954D404E3955E0MWHPR07MB2943namp_" Cc: Seth Jennings , Mahipal Challa , "herbert@gondor.apana.org.au" , "davem@davemloft.net" , "linux-crypto@vger.kernel.org" , LKML , Linux-MM , "Challa, Mahipal" To: Giovanni Cabiddu , "Narayana, Prasad Athreya" Return-path: In-Reply-To: <20170215221208.GA820@silv-gc1.ir.intel.com> Content-Language: en-US Sender: owner-linux-mm@kvack.org List-Id: linux-crypto.vger.kernel.org --_000_MWHPR07MB2943F04F607146A954D404E3955E0MWHPR07MB2943namp_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable >This assumption is not correct. An asynchronous implementation, when >it finishes processing a request, will call acomp_request_complete() which >in turn calls the callback. >If the callback is set to NULL, this function will dereference a NULL >pointer. This would leave us with the option of waiting in zswap until completion. H= ere we had a doubt. If we go ahead with an implementation similar to the one found in crypto/te= stmgr.c, the private data(result) which is registered via 'acomp_request_se= t_callback()' is coming from stack. Do you see this as a potential problem = for an acutal asynchronus algorithm due to the context from which callback = is called? Do we have to use per-cpu dynamic allocation? Thanks, Vishnu ________________________________ From: Giovanni Cabiddu Sent: Thursday, February 16, 2017 3:42 AM To: Narayana, Prasad Athreya Cc: Seth Jennings; Mahipal Challa; herbert@gondor.apana.org.au; davem@davem= loft.net; linux-crypto@vger.kernel.org; LKML; Linux-MM; Narayana, Prasad At= hreya; Nair, Vishnu; Challa, Mahipal; Nair, Vishnu Subject: Re: [RFC PATCH v1 1/1] mm: zswap - Add crypto acomp/scomp framewor= k support On Wed, Feb 15, 2017 at 07:27:30PM +0530, Narayana Prasad Athreya wrote: > > I assume all of these crypto_acomp_[compress|decompress] calls are > > actually synchronous, > > not asynchronous as the name suggests. Otherwise, this would blow up > > quite spectacularly > > since all the resources we use in the call get derefed/unmapped below. > > > > Could an async algorithm be implement/used that would break this assump= tion? > > The callback is set to NULL using acomp_request_set_callback(). This impl= ies > synchronous mode of operation. So the underlying implementation must > complete the operation synchronously. This assumption is not correct. An asynchronous implementation, when it finishes processing a request, will call acomp_request_complete() which in turn calls the callback. If the callback is set to NULL, this function will dereference a NULL pointer. Regards, -- Giovanni --_000_MWHPR07MB2943F04F607146A954D404E3955E0MWHPR07MB2943namp_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable

>This assumption is = not correct. An asynchronous implementation, when
>it finishes processing a request, will call acomp_request_complete() wh= ich
>in turn calls the callback.
>If the callback is set to NULL, this function will dereference a NULL >pointer.


This woul= d leave us with the option of waiting in zswap until completion. Here we ha= d a doubt.

If we go ahead with an implementation similar to the one found in crypto= /testmgr.c, the private data(result) which is registered via 'acomp_r= equest_set_callback()' is coming from stack. Do you see this as a po= tential problem for an acutal asynchronus algorithm due to the context from which callback is called? Do we have to = use per-cpu dynamic allocation?


Thanks,
Vishnu



From: Giovanni Cabiddu &l= t;giovanni.cabiddu@intel.com>
Sent: Thursday, February 16, 2017 3:42 AM
To: Narayana, Prasad Athreya
Cc: Seth Jennings; Mahipal Challa; herbert@gondor.apana.org.au; dave= m@davemloft.net; linux-crypto@vger.kernel.org; LKML; Linux-MM; Narayana, Pr= asad Athreya; Nair, Vishnu; Challa, Mahipal; Nair, Vishnu
Subject: Re: [RFC PATCH v1 1/1] mm: zswap - Add crypto acomp/scomp f= ramework support
 
On Wed, Feb 15, 2017 at 07:27:30PM +0530, Nara= yana Prasad Athreya wrote:
> > I assume all of these crypto_acomp_[compress|decompress] calls ar= e
> > actually synchronous,
> > not asynchronous as the name suggests.  Otherwise, this woul= d blow up
> > quite spectacularly
> > since all the resources we use in the call get derefed/unmapped b= elow.
> >
> > Could an async algorithm be implement/used that would break this = assumption?
>
> The callback is set to NULL using acomp_request_set_callback(). This i= mplies
> synchronous mode of operation. So the underlying implementation must > complete the operation synchronously.
This assumption is not correct. An asynchronous implementation, when
it finishes processing a request, will call acomp_request_complete() which<= br> in turn calls the callback.
If the callback is set to NULL, this function will dereference a NULL
pointer.

Regards,

--
Giovanni
--_000_MWHPR07MB2943F04F607146A954D404E3955E0MWHPR07MB2943namp_-- -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org