[ https://issues.apache.org/jira/browse/TS-4234?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15171243#comment-15171243 ]
James Peach commented on TS-4234: --------------------------------- [~shaygalon] could you please make this a separate pull request from the general Aarch64 changes? Are you able to publish your benchmarking? [~briang], [~shinrich] is this consistent with your benchmarking? Finally, if this is an unambiguous win under all workloads, we should just switch. If there's cases when muteness are better, I think it ought to be a run-time option. > ATS on AARCH64 use spinlock for SSL locking > ------------------------------------------- > > Key: TS-4234 > URL: https://issues.apache.org/jira/browse/TS-4234 > Project: Traffic Server > Issue Type: Improvement > Components: SSL > Reporter: Shay Gal-On > Fix For: sometime > > > Currently on aarch64 mutex locking can be slower then necessary under load. > The patch below adds an option to substitute the mutex in ssl locking > callback with a spinlock using gcc intrinsics. > diff -Nau -x Makefile -x .Tpo -x .deps > orig-trafficserver-5.3.2/iocore/net/SSLUtils.cc > trafficserver-5.3.2/iocore/net/SSLUtils.cc > --- orig-trafficserver-5.3.2/iocore/net/SSLUtils.cc 2015-09-08 > 13:05:06.000000000 -0700 > +++ trafficserver-5.3.2/iocore/net/SSLUtils.cc 2016-02-26 11:44:55.709451000 > -0800 > @@ -119,9 +119,17 @@ > #endif > -static pthread_mutex_t *mutex_buf = NULL; > static bool open_ssl_initialized = false; > +#ifdef SSL_USE_SPINLOCK > +#include "Spinlock.h" > +#define SSL_locking_callback SSL_locking_callback_spinlock > +static Spinlock *spinlock_buf = NULL; > +#else > +#define SSL_locking_callback SSL_locking_callback_mutex > +static pthread_mutex_t *mutex_buf = NULL; > +#endif > + > RecRawStatBlock *ssl_rsb = NULL; > static InkHashTable *ssl_cipher_name_table = NULL; > @@ -135,9 +143,24 @@ > { > return (unsigned long)pthread_self(); > } > +#ifdef SSL_USE_SPINLOCK > +static void > +SSL_locking_callback_spinlock(int mode, int type, const char * /* file > ATS_UNUSED */, int /* line ATS_UNUSED */) > +{ > + ink_assert(type < CRYPTO_num_locks()); > + if (mode & CRYPTO_LOCK) { > + spinlock_buf[type].lock(); > + } else if (mode & CRYPTO_UNLOCK) { > + spinlock_buf[type].unlock(); > + } else { > + Debug("ssl", "invalid SSL locking mode 0x%x", mode); > + ink_assert(0); > + } > +} > +#else > static void > -SSL_locking_callback(int mode, int type, const char * /* file ATS_UNUSED */, > int /* line ATS_UNUSED */) > +SSL_locking_callback_mutex(int mode, int type, const char * /* file > ATS_UNUSED */, int /* line ATS_UNUSED */) > { > ink_assert(type < CRYPTO_num_locks()); > @@ -150,6 +173,7 @@ > ink_assert(0); > } > } > +#endif > static bool > SSL_CTX_add_extra_chain_cert_file(SSL_CTX *ctx, const char *chainfile) > @@ -758,11 +782,14 @@ > SSL_load_error_strings(); > SSL_library_init(); > +#ifdef SSL_USE_SPINLOCK > + spinlock_buf = new Spinlock[CRYPTO_num_locks()]; > +#else > mutex_buf = (pthread_mutex_t *)OPENSSL_malloc(CRYPTO_num_locks() * > sizeof(pthread_mutex_t)); > - > for (int i = 0; i < CRYPTO_num_locks(); i++) { > pthread_mutex_init(&mutex_buf[i], NULL); > } > +#endif > CRYPTO_set_locking_callback(SSL_locking_callback); > CRYPTO_set_id_callback(SSL_pthreads_thread_id); > diff -Nau -x Makefile -x .Tpo -x .deps > orig-trafficserver-5.3.2/iocore/net/Spinlock.h > trafficserver-5.3.2/iocore/net/Spinlock.h > --- orig-trafficserver-5.3.2/iocore/net/Spinlock.h 1969-12-31 > 16:00:00.000000000 -0800 > +++ trafficserver-5.3.2/iocore/net/Spinlock.h 2016-02-26 11:34:06.359451000 > -0800 > @@ -0,0 +1,38 @@ > +/** @file > + > +Author: Shay Gal-On > + > +A class implementing a simple spinlock using GCC atomics, > +using sched_yield while lock fails. > + > +*/ > + > +#include <sched.h> > + > +class Spinlock > +{ > +public: > + int _data; > + > +public: > + bool try_lock() > + { > + int r = __sync_lock_test_and_set( &_data, 1 ); > + return r == 0; > + } > + > + void lock() > + { > + for( unsigned k = 0; !try_lock(); ++k ) > + { > + sched_yield(); > + } > + } > + > + void unlock() > + { > + __sync_lock_release( &_data ); > + } > +}; > + > + -- This message was sent by Atlassian JIRA (v6.3.4#6332)