fixup! [PAL/Linux-SGX] Print SGX stats on SIGUSR1 and reset them

Signed-off-by: Dmitrii Kuvaiskii <[email protected]>

pal/src/host/linux-sgx/host_exception.c

@@ -282,11 +282,8 @@ void maybe_dump_and_reset_stats(void) {
         return;
 
     PAL_HOST_TCB* tcb = pal_get_host_tcb();
-    if (__atomic_load_n(&tcb->reset_stats, __ATOMIC_RELAXED) == false)
-        return;
-
-    collect_and_print_sgx_stats();
-
-    __atomic_store_n(&tcb->reset_stats, false, __ATOMIC_RELAXED);
+    if (__atomic_exchange_n(&tcb->reset_stats, false, __ATOMIC_RELAXED) == true) {
+        collect_and_print_sgx_stats();
+    }
 }
 #endif

pal/src/host/linux-sgx/host_ocalls.c

@@ -32,7 +32,7 @@ rpc_queue_t* g_rpc_queue = NULL; /* pointer to untrusted queue */
 
 static noreturn void process_exit(int exitcode) {
 #ifdef DEBUG
-    update_sgx_stats(/*print_process_wide_stats=*/true);
+    update_sgx_stats_on_exit(/*print_process_wide_stats=*/true);
     sgx_profile_finish();
 #endif
 
@@ -75,7 +75,7 @@ static long sgx_ocall_exit(void* args) {
     }
 
 #ifdef DEBUG
-    update_sgx_stats(/*print_process_wide_stats=*/false);
+    update_sgx_stats_on_exit(/*print_process_wide_stats=*/false);
 #endif
     thread_exit((int)ocall_exit_args->exitcode);
     return 0;

pal/src/host/linux-sgx/host_thread.c

@@ -64,25 +64,15 @@ static void reset_global_sgx_stats(void) {
 static void update_global_sgx_stats_from_thread_stats(PAL_HOST_TCB* tcb) {
     assert(spinlock_is_locked(&g_enclave_thread_map_lock));
 
-    g_eenter_cnt       += tcb->eenter_cnt;
-    g_eexit_cnt        += tcb->eexit_cnt;
-    g_aex_cnt          += tcb->aex_cnt;
-    g_sync_signal_cnt  += tcb->sync_signal_cnt;
-    g_async_signal_cnt += tcb->async_signal_cnt;
-
-    /* there is a small window when the thread's counters may be updated in-between reading and
-     * resetting these counters -- some SGX events will be lost; we ignore this as the number of
-     * lost events is negligible for perf analysis purposes */
-
-    tcb->eenter_cnt       = 0;
-    tcb->eexit_cnt        = 0;
-    tcb->aex_cnt          = 0;
-    tcb->sync_signal_cnt  = 0;
-    tcb->async_signal_cnt = 0;
+    g_eenter_cnt       += __atomic_exchange_n(&tcb->eenter_cnt, 0, __ATOMIC_RELAXED);
+    g_eexit_cnt        += __atomic_exchange_n(&tcb->eexit_cnt, 0, __ATOMIC_RELAXED);
+    g_aex_cnt          += __atomic_exchange_n(&tcb->aex_cnt, 0, __ATOMIC_RELAXED);
+    g_sync_signal_cnt  += __atomic_exchange_n(&tcb->sync_signal_cnt, 0, __ATOMIC_RELAXED);
+    g_async_signal_cnt += __atomic_exchange_n(&tcb->async_signal_cnt, 0, __ATOMIC_RELAXED);
 }
 
 /* this function is called only on thread/process exit (never in the middle of thread exec) */
-void update_sgx_stats(bool print_process_wide_stats) {
+void update_sgx_stats_on_exit(bool print_process_wide_stats) {
     if (!g_sgx_enable_stats)
         return;
 
@@ -101,6 +91,15 @@ void collect_and_print_sgx_stats(void) {
     if (!g_sgx_enable_stats)
         return;
 
+    /*
+     * This function is executed by the "SGX-stats-collecting" thread (that received SIGUSR1). Thus,
+     * this thread is able to peek into the local storage of other threads. This is typically
+     * considered a bad smell (one thread reads local data of another thread), but here it's a
+     * reasonable trade-off: most of the accesses to the thread-local SGX counters are done on EEXIT
+     * and AEX events by the thread itself, so the memory access should be as simple as possible and
+     * as fast as possible. An alternative would be to move all SGX stats in a shared array, then we
+     * would have false cache sharing and complex memory management of the shared array.
+     */
     spinlock_lock(&g_enclave_thread_map_lock);
     for (size_t i = 0; i < g_enclave_thread_num; i++) {
         if (!g_enclave_thread_map[i].tcs || !g_enclave_thread_map[i].tid)

pal/src/host/linux-sgx/pal_tcb.h

@@ -116,7 +116,7 @@ static inline PAL_HOST_TCB* pal_get_host_tcb(void) {
 }
 
 extern bool g_sgx_enable_stats;
-void update_sgx_stats(bool print_process_wide_stats);
+void update_sgx_stats_on_exit(bool print_process_wide_stats);
 void collect_and_print_sgx_stats(void);
 #ifdef DEBUG
 void maybe_dump_and_reset_stats(void);