Krishna Yenduri wrote:
> Krishna Yenduri wrote:
>> Garrett D'Amore wrote:
>>> Krishna Yenduri wrote:
>>>
>>>>
>>>> The webrev is here:
>>>> http://cr.grommit.com/~krishna/6491965/
>>>>
>>>>
>>>
>>>
>>> Its been a while since I looked at PKCS#11 implementation. But I have
>>> to concerns with these changes:
>>>
>>> 1) this adds to the cost of creating a session. I seem to recall we
>>> found some PKCS#11 consumers that spent a lot of time
>>> creating/destroying sessions. (I.e. the sessions were short
>>> lived.) In
>>> practice, the extra syscall overhead would probably be small
>>> compared to
>>> the cost of the crypto operations, but it will still represent a slight
>>> impact over the existing code.
>>>
>>
>> Good point. This fix is aimed at workloads which have multiple
>> threads like
>> a web server. For these workloads, the cost of an extra syscall is
>> not a factor since
>> we gain from eliminating lock contention. I will check the case of a
>> single thread to
>> make sure there is no statistically significant regression.
>
> I checked this case and the single thread performance is slightly worse
> (-2%) on AMD64. On a sun4v, the single thread performance showed
> no change. Even for the multi thread case, the performance gains
> didn't turn out to be that we expected. I saw only a 3% improvement in
> throughput with 16 threads on a T2000 using 1024-byte data size.
>
> Given this data, I am not proceeding with this bug fix.
>
> Further investigation showed the problem is not the lock contention
> alone. But, also the time spent in a routine. lockstat with -g showed
> crypto_buffer_check() as a problem routine. I filed bug #6520071.
> Fixing that bug gave a lot better performance improvement on all
> platforms.
>
> The webrev for 6520071 is at
> http://cr.grommit.com/~krishna/webrev_6520071/
>
> Mark Powers, and Stephen Lawrence already reviewed it.
Looks good to me. Ship it!
-- Garrett
>
> Thanks,
> -Krishna
>
>
>
>>> 2) have you found all occurrences of kernel_fd? The changes made look
>>> good, but I'd be more concerned about making sure you found them
>>> all. :-)
>>>
>>
>> Yes. cscope is my friend :-).
>>
>> Thanks,
>> -Krishna
>>
>>> Modulo these comments, the changes look good to me.
>>>
>>>
>>
>> _______________________________________________
>> crypto-discuss mailing list
>> crypto-discuss at opensolaris.org
>> http://opensolaris.org/mailman/listinfo/crypto-discuss
