On 08/31/2017 06:31 AM, Dan Streetman wrote:

On Tue, Aug 29, 2017 at 5:54 PM, Haren Myneni [off-list ref] wrote:

quoted

On 08/29/2017 02:23 PM, Benjamin Herrenschmidt wrote:

quoted

On Tue, 2017-08-29 at 09:58 -0400, Dan Streetman wrote:

quoted

quoted

+
+       ret = -EINVAL;
+       if (coproc && coproc->vas.rxwin) {
+               wmem->txwin = nx842_alloc_txwin(coproc);

this is wrong.  the workmem is scratch memory that's valid only for
the duration of a single operation.

Correct, workmem is used until crypto_free is called.

that's not a 'single operation'.  a single operation is compress() or
decompress().

workmem is allocated in nx842_crypto_init (called from crypto_alloc) and freed in crypto_free(). We can have single compression / decompression() operation or multiple within this crypto session. In case of single operation, we will end up workmem, ctx->sbounce/dbounce alloc/free for each request.  

quoted

quoted

quoted

do you actually need a txwin per crypto transform?  or do you need a
txwin per coprocessor?  or txwin per processor?  either per-coproc or
per-cpu should be created at driver init and held separately
(globally) instead of a per-transform txwin.  I really don't see why
you would need a txwin per transform, because the coproc should not
care how many different transforms there are.

We should only need a single window for the whole kernel really, plus
one per user process who wants direct access but that's not relevant
here.

Opening send window for each crypto transform (crypto_alloc,
compression/decompression, ..., crypto_free) so that does not
have to wait for the previous copy/paste complete.
VAS will map send and receive windows, and can cache in send
windows (up to 128). So I thought using the same send window
(per chip) for more requests (say 1000) may be adding overhead.

I will make changes if you prefer using 1 send window per chip.

i don't have the spec, so i shouldn't be making the decision on it,
but i do know putting a persistent field into the workmem is the wrong
location.  If it's valid for the life of the transform, put it into
the transform context.  The workmem buffer is intended to be used only
during a single operation - it's "working memory" to perform each
individual crypto transformation.

quoted

quoted

Cheers,
Ben.