Hi Horia,
On Tue, Feb 24, 2015 at 1:21 PM, Horia Geantă
<horia.geanta@xxxxxxxxxxxxx> wrote:
> On 2/20/2015 6:21 PM, Martin Hicks wrote:
>> + int ret = -EINPROGRESS;
>>
>> spin_lock_irqsave(&priv->chan[ch].head_lock, flags);
>>
>> + if (edesc) {
>> + orig_request = &edesc->req;
>> + crypto_enqueue_request(&priv->chan[ch].queue, &orig_request->base);
>> + }
>
> The request goes through the SW queue even if there are empty slots in
> the HW queue, doing unnecessary crypto_queue_encrypt() and
> crypto_queue_decrypt(). Trying to use the HW queue first would be better.
>
Definitely a valid point. In trying to reorganize this it really
complicates things.
Splitting the submit from issuing requests from the backlog seems to
make it much more straightforward.
>> @@ -1170,6 +1211,8 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
>> edesc->dma_len,
>> DMA_BIDIRECTIONAL);
>> edesc->req.desc = &edesc->desc;
>> + /* A copy of the crypto_async_request to use the crypto_queue backlog */
>> + memcpy(&edesc->req.base, areq, sizeof(struct crypto_async_request));
>
> Why not have a
> struct crypto_async_request *base;
> instead of
> struct crypto_async_request base;
> in talitos_request structure?
>
> This way:
> 1. memcopy above is avoided
> 2. talitos_request structure gets smaller - remember that
> talitos_request is now embedded in talitos_edesc structure, which gets
> kmalloc-ed for every request
The trouble I ran into was that after a request is backlogged and put
on the crypto queue, I couldn't see how else I could go from the
crypto_async_request that is pulled from the queue back to the
talitos_request that is needed in order put the pointer into the
hardware FIFO.
This is the one issue from you review that I didn't address in v2 of the patch.
Thanks for the review. Not releasing / re-acquiring the spinlock
while trying to flush the backlog really improved performance.
mh
--
Martin Hicks P.Eng. | mort@xxxxxxxx
Bork Consulting Inc. | +1 (613) 266-2296
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
