[openssl-dev] [openssl-team] Discussion: design issue: async and -lpthread
Paul Dale
paul.dale at oracle.com
Tue Dec 8 23:27:16 UTC 2015
It will be possible to support atomics in such a way that there is no performance penalty for machines without them or for single threaded operation.
My sketcy design is along the lines of adding a new API CRYPTO_add_atomic that takes the same arguments as CRYPTO_add (i.e. reference to counter, value to add and lock to use):
CRYPTO_add_atomic(int *addr, int amount, int lock)
if have-atomics then
atomic_add(addr, amount)
else if (lock == have-lock-already)
*addr += amount
else
CRYPTO_add(addr, amount, lock)
The have-lock-already will need to be a new code that indicates that the caller has the relevant lock held and there is no need to lock before the add. Some conditional compilation like CRYPTO_add & CRYPTO_add_lock have can be done to get the overhead down to zero in the single threaded case and the case where it is known beforehand that there are no atomic operations. It is also possible for the atomic_add function to be passed in as a user call back as per the other locking callbacks which means OSSL doesn't actually need to know how any of this works underneath.
Once this is done, most instances of CRYPTO_add can be changed to CRYPTO_add_atomic. Unfortunately, not all can be changed, so this would involve manual inspection of each lock for which CRYPTO_add is used to see if atomics are suitable. I've done a partial list of which could be changed over (attached) but it is pretty rough and needs rechecking.
It would be prudent to have a CRYPTO_add_atomic_lock call underneath CRYPTO_add_atomic, like CRYPTO_add has CRYPTO_add_lock, to get the extra debug output.
Finally, can someone explain what the callback passed to CRYPTO_set_add_lock_callback is supposed to do? Superficially, it seems like a way to use atomic operations instead of full locking -- but that breaks things due to the way the locking is done elsewhere. So this call back needs to lock, add and unlock like the alternate code path in the CRYPTO_add_lock function. There is no obvious benefit to providing it.
Pauli
On Tue, 8 Dec 2015 11:22:01 AM Nico Williams wrote:
> On Tue, Dec 08, 2015 at 11:19:32AM +0100, Florian Weimer wrote:
> > > Maybe http://trac.mpich.org/projects/openpa/ would fit the bill?
> >
> > It seems to have trouble to keep up with new architectures.
>
> New architectures are not really a problem because between a) decent
> compilers with C11 and/or non-C11 atomic intrinsics, b) asm-coded
> atomics, and c) mutex-based dumb atomics, we can get full coverage.
> Anyone who's still not satisfied can then contribute missing asm-coded
> atomics to OpenPA. I suspect that OpenSSL using OpenPA is likely to
> lead to contributions to OpenPA that will make it better anyways.
>
> What's the alternative anyways?
>
> We're talking about API and performance enhancements to OpenSSL to go
> faster on platforms for which there are atomics, and maybe slower
> otherwise -- or maybe not; maybe we can implement context up-/down-ref
> functions that use fine-grained (or even global) locking as a fallback
> that yields performance comparable to today's.
>
> If OpenPA's (or some other such library's) license works for OpenSSL,
> someone might start using it. That someone might be me. So that seems
> like a good question to ask: is OpenPA's license compatible with
> OpenSSL's? For inclusion into OpenSSL's tree, or for use by OpenSSL?
>
> Nico
>
--
Oracle
Dr Paul Dale | Cryptographer | Network Security & Encryption
Phone +61 7 3031 7217
Oracle Australia
-------------- next part --------------
Lock # Note
CRYPTO_LOCK_ERR 28 one problem with decrement using CRYPTO_add -- leave as is
CRYPTO_LOCK_EX_DATA 7 safe, no CRYPTO_add
CRYPTO_LOCK_X509 10 most likely safe, needs deeper recheck down call chains
CRYPTO_LOCK_X509_INFO 2 safe, only uses CRYPTO_add
CRYPTO_LOCK_X509_PKEY 2 safe, only uses CRYPTO_add
CRYPTO_LOCK_X509_CRL 5 unsure
CRYPTO_LOCK_X509_REQ 2 only ASN1_SEQUENCE_re
CRYPTO_LOCK_DSA 5 safe with atomic add (double check)?
CRYPTO_LOCK_RSA 19 most likely safe, needs deeper recheck down call chains
CRYPTO_LOCK_EVP_PKEY 27 safe with atomic add
CRYPTO_LOCK_X509_STORE 35 assume unsafe, one CRYPTO_add only
CRYPTO_LOCK_SSL_CTX 26 one increment unsafe, rest seems okay -> make increment atomic
CRYPTO_LOCK_SSL_CERT 3 safe, only uses CRYPTO_add
CRYPTO_LOCK_SSL_SESSION 9 safe if atomic add is used inside locked block in ssl/ssl_sess.c
CRYPTO_LOCK_SSL_SESS_CERT 1 unused
CRYPTO_LOCK_SSL 11 safe without compression, probably safe with but would need a double check
CRYPTO_LOCK_SSL_METHOD 1 unused
CRYPTO_LOCK_RAND 16 safe, no CRYPTO_add
CRYPTO_LOCK_RAND2 11 safe, no CRYPTO_add
CRYPTO_LOCK_MALLOC 13 safe, no CRYPTO_add
CRYPTO_LOCK_BIO 8 safe with atomic add
CRYPTO_LOCK_GETHOSTBYNAME 3 safe, no CRYPTO_add
CRYPTO_LOCK_GETSERVBYNAME 3 safe, no CRYPTO_add
CRYPTO_LOCK_READDIR 3 safe, no CRYPTO_add
CRYPTO_LOCK_RSA_BLINDING 4 safe, no CRYPTO_add
CRYPTO_LOCK_DH 5 safe with atomic add
CRYPTO_LOCK_MALLOC2 9 safe, no CRYPTO_add
CRYPTO_LOCK_DSO 3 safe, only uses CRYPTO_add
CRYPTO_LOCK_DYNLOCK 12 safe, no CRYPTO_add
CRYPTO_LOCK_ENGINE 65 unsafe, two CRYPTO_add -- eave as is.
CRYPTO_LOCK_UI 3 safe, no CRYPTO_add
CRYPTO_LOCK_ECDSA 1 unused
CRYPTO_LOCK_EC 7 safe to change to atomic add outside locked blocks
CRYPTO_LOCK_ECDH 1 unused
CRYPTO_LOCK_BN 2 unused
CRYPTO_LOCK_EC_PRE_COMP 16 safe, only uses CRYPTO_add
CRYPTO_LOCK_STORE 1 unused
CRYPTO_LOCK_COMP 1 unused
CRYPTO_LOCK_FIPS 1 unused
CRYPTO_LOCK_FIPS2 1 unused
More information about the openssl-dev
mailing list