stapdyn: isolate the tls context pointer more

* runtime/dyninst/runtime_context.h: Rename contexts to tls_context to
  better indicate the purpose of this pointer; update everywhere.
  (_stp_runtime_entryfn_put_context): Take a pointer argument for the
  context you're releasing, to slightly help prevent coding accidents.
* runtime/dyninst/transport.c: Stop the insider-trading of contexts, not
  renaming to tls_context either.  Use _stp_runtime_get_context().
* tapsets.cxx, translate.cxx: Pass the argument to put_context.

diff --git a/runtime/dyninst/runtime_context.h b/runtime/dyninst/runtime_context.h
index 31f88adae4fd1a04a77dcd129f43f160acc7d47d..031ffc0509a433447c8614dd1a8ddb10941dc135 100644 (file)
--- a/runtime/dyninst/runtime_context.h
+++ b/runtime/dyninst/runtime_context.h
@@ -27,7 +27,7 @@ static int _stp_runtime_num_contexts;
 
 /* Locally-cached context pointer -- see _stp_runtime_entryfn_get_context()
  * and _stp_runtime_entryfn_put_context().  */
-static __thread struct context *contexts;
+static __thread struct context *tls_context;
 
 static int _stp_runtime_contexts_init(void)
 {
@@ -80,8 +80,8 @@ static int _stp_runtime_get_data_index(void)
 
     /* If this thread has already gotten a context structure,
      * return the data index from it. */
-    if (contexts != NULL)
-       return contexts->data_index;
+    if (tls_context != NULL)
+       return tls_context->data_index;
 
     /* This shouldn't happen. */
     /* FIXME: assert? */
@@ -112,9 +112,9 @@ static struct context * _stp_runtime_entryfn_get_context(void)
     struct context *c;
     int i, index, rc, data_index;
 
-    /* If 'contexts' (which is thread-local storage) is already set
+    /* If 'tls_context' (which is thread-local storage) is already set
      * for this thread, we are re-entrant, so just quit. */
-    if (contexts != NULL)
+    if (tls_context != NULL)
        return NULL;
 
     data_index = _stp_context_index();
@@ -130,8 +130,8 @@ static struct context * _stp_runtime_entryfn_get_context(void)
        if (pthread_mutex_trylock(&c->lock) == 0) {
            /* We found a free context structure. Now that it is
             * locked, set the TLS pointer and return the context. */
-           contexts = c;
-           return contexts;
+           tls_context = c;
+           return tls_context;
        }
     }
 
@@ -140,19 +140,19 @@ static struct context * _stp_runtime_entryfn_get_context(void)
     c = stp_session_context(data_index);
     rc = pthread_mutex_lock(&c->lock);
     if (rc == 0) {
-       contexts = c;
-       return contexts;
+       tls_context = c;
+       return tls_context;
     }
     return NULL;
 }
 
-static void _stp_runtime_entryfn_put_context(void)
+static void _stp_runtime_entryfn_put_context(struct context *c)
 {
-    if (contexts) {
-       struct context *c = contexts;
-       contexts = NULL;
+    if (c && c == tls_context) {
+       tls_context = NULL;
        pthread_mutex_unlock(&c->lock);
     }
+    // else, warn about bad state?
     return;
 }
 
@@ -162,7 +162,7 @@ static struct context *_stp_runtime_get_context(void)
      * here. This function is called after
      * _stp_runtime_entryfn_get_context() and has no corresponding
      * "put" function. */
-    return contexts;
+    return tls_context;
 }
 
 static void _stp_runtime_context_wait(void)

diff --git a/runtime/dyninst/transport.c b/runtime/dyninst/transport.c
index 0053b0274d1c95aa90d9dc57d0334fb712b861de..325b592014c4fb4238ce6ad0960afe7b50686dbc 100644 (file)
--- a/runtime/dyninst/transport.c
+++ b/runtime/dyninst/transport.c
@@ -467,7 +467,8 @@ static int _stp_ctl_send(int type, void *data, unsigned len)
                        type, data, len);
 
        // This thread should already have a context structure.
-       if (contexts == NULL)
+        struct context* c = _stp_runtime_get_context();
+       if (c == NULL)
                return EINVAL;
 
        // Currently, we're only handling 'STP_SYSTEM' control
@@ -480,9 +481,9 @@ static int _stp_ctl_send(int type, void *data, unsigned len)
                return 0;
 
        memcpy(buffer, data, len);
-       size_t offset = buffer - contexts->transport_data.log_buf;
+       size_t offset = buffer - c->transport_data.log_buf;
        __stp_dyninst_transport_queue_add(STP_DYN_SYSTEM,
-                                         contexts->data_index, offset, len);
+                                         c->data_index, offset, len);
        return len;
 }
 
@@ -574,21 +575,23 @@ static int
 _stp_dyninst_transport_write_oob_data(char *buffer, size_t bytes)
 {
        // This thread should already have a context structure.
-       if (contexts == NULL)
+        struct context* c = _stp_runtime_get_context();
+       if (c == NULL)
                return EINVAL;
 
-       size_t offset = buffer - contexts->transport_data.log_buf;
+       size_t offset = buffer - c->transport_data.log_buf;
        __stp_dyninst_transport_queue_add(STP_DYN_OOB_DATA,
-                                         contexts->data_index, offset, bytes);
+                                         c->data_index, offset, bytes);
        return 0;
 }
 
 static int _stp_dyninst_transport_write(void)
 {
        // This thread should already have a context structure.
-       if (contexts == NULL)
+        struct context* c = _stp_runtime_get_context();
+       if (c == NULL)
                return 0;
-       struct _stp_transport_context_data *data = &contexts->transport_data;
+       struct _stp_transport_context_data *data = &c->transport_data;
        size_t bytes = data->write_bytes;
 
        if (bytes == 0)
@@ -614,7 +617,7 @@ static int _stp_dyninst_transport_write(void)
        data->write_offset = _STP_D_T_PRINT_ADD(data->write_offset, bytes);
 
        __stp_dyninst_transport_queue_add(STP_DYN_NORMAL_DATA,
-                                         contexts->data_index,
+                                         c->data_index,
                                          saved_write_offset, bytes);
        return 0;
 }
@@ -669,12 +672,13 @@ _stp_dyninst_transport_log_buffer_full(struct _stp_transport_context_data *data)
 static char *_stp_dyninst_transport_log_buffer(void)
 {
        // This thread should already have a context structure.
-       if (contexts == NULL)
+        struct context* c = _stp_runtime_get_context();
+       if (c == NULL)
                return NULL;
 
        // Note that the context structure is locked, so only one
        // probe at a time can be operating on it.
-       struct _stp_transport_context_data *data = &contexts->transport_data;
+       struct _stp_transport_context_data *data = &c->transport_data;
 
        // If there isn't an available log buffer, wait.
        if (_stp_dyninst_transport_log_buffer_full(data)) {
@@ -743,12 +747,13 @@ static void *_stp_dyninst_transport_reserve_bytes(int numbytes)
        void *ret;
 
        // This thread should already have a context structure.
-       if (contexts == NULL) {
-               _stp_transport_debug("NULL contexts!\n");
+        struct context* c = _stp_runtime_get_context();
+       if (c == NULL) {
+               _stp_transport_debug("NULL context!\n");
                return NULL;
        }
 
-       struct _stp_transport_context_data *data = &contexts->transport_data;
+       struct _stp_transport_context_data *data = &c->transport_data;
        size_t space_before, space_after, read_offset;
 
 recheck:
@@ -908,10 +913,11 @@ recheck:
 static void _stp_dyninst_transport_unreserve_bytes(int numbytes)
 {
        // This thread should already have a context structure.
-       if (contexts == NULL)
+        struct context* c = _stp_runtime_get_context();
+       if (c == NULL)
                return;
 
-       struct _stp_transport_context_data *data = &contexts->transport_data;
+       struct _stp_transport_context_data *data = &c->transport_data;
        if (unlikely(numbytes <= 0 || numbytes > data->write_bytes))
                return;
 

diff --git a/runtime/linux/runtime_context.h b/runtime/linux/runtime_context.h
index c7c552c9c578e9486062280bcb3a00c3fc21a49f..18122749c5a05523f88605e7a79203ca29e7b0a4 100644 (file)
--- a/runtime/linux/runtime_context.h
+++ b/runtime/linux/runtime_context.h
@@ -48,7 +48,7 @@ static struct context * _stp_runtime_entryfn_get_context(void)
        return contexts[smp_processor_id()];
 }
 
-static inline void _stp_runtime_entryfn_put_context(void)
+static inline void _stp_runtime_entryfn_put_context(struct context *c __attribute__((unused)))
 {
        /* Do nothing. */
        return;

diff --git a/runtime/runtime.h b/runtime/runtime.h
index 6dd96d3cbbce7a5afa8df8963a4892d804b96fb9..bbf5c6da0c4cf59bfedd0100ff6c12370eb71d4c 100644 (file)
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -16,7 +16,7 @@ static int _stp_runtime_contexts_alloc(void);
 static void _stp_runtime_contexts_free(void);
 static int _stp_runtime_get_data_index(void);
 static struct context *_stp_runtime_entryfn_get_context(void);
-static void _stp_runtime_entryfn_put_context(void);
+static void _stp_runtime_entryfn_put_context(struct context *);
 static struct context *_stp_runtime_get_context(void);
 
 #if defined(__KERNEL__)

diff --git a/tapsets.cxx b/tapsets.cxx
index 81fa4d8c3341959991fd83684a52a09933e3741c..d04d9d34ab2dcdb563339a8d4534347d3b020fc0 100644 (file)
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -90,13 +90,16 @@ common_probe_entryfn_prologue (systemtap_session& s,
       // and there's *nothing* for the probe to do.  (even alibi is in shm)
       // So failure skips this whole block through the end of the epilogue.
       s.op->newline() << "if (likely(session_state())) {";
-      s.op->newline(1) << "int _stp_saved_errno = errno;";
+      s.op->indent(1);
     }
 
   s.op->newline() << "#ifdef STP_ALIBI";
   s.op->newline() << "atomic_inc(probe_alibi(" << probe << "->index));";
   s.op->newline() << "#else";
 
+  if (s.runtime_usermode_p())
+    s.op->newline() << "int _stp_saved_errno = errno;";
+
   s.op->newline() << "struct context* __restrict__ c = NULL;";
   s.op->newline() << "#if !INTERRUPTIBLE";
   s.op->newline() << "unsigned long flags;";
@@ -339,7 +342,7 @@ common_probe_entryfn_epilogue (systemtap_session& s,
   // buffers are stored there).
   if (!s.runtime_usermode_p())
     {
-      s.op->newline() << "_stp_runtime_entryfn_put_context();";
+      s.op->newline() << "_stp_runtime_entryfn_put_context(c);";
     }
   if (! s.suppress_handler_errors) // PR 13306
     {
@@ -356,14 +359,16 @@ common_probe_entryfn_epilogue (systemtap_session& s,
   s.op->newline() << "local_irq_restore (flags);";
   s.op->newline() << "#endif";
 
-  s.op->newline() << "#endif // STP_ALIBI";
-
   if (s.runtime_usermode_p())
     {
-      s.op->newline() << "if (c) _stp_runtime_entryfn_put_context();";
+      s.op->newline() << "_stp_runtime_entryfn_put_context(c);";
       s.op->newline() << "errno = _stp_saved_errno;";
-      s.op->newline(-1) << "}";
     }
+
+  s.op->newline() << "#endif // STP_ALIBI";
+
+  if (s.runtime_usermode_p())
+    s.op->newline(-1) << "}";
 }
 
 

diff --git a/translate.cxx b/translate.cxx
index 9a5e6e02684ecee3e30facbdbdd74f735921b0b7..32d2fa64a037b938acb5b16af79d88a74c34d1c7 100644 (file)
--- a/translate.cxx
+++ b/translate.cxx
@@ -2099,7 +2099,7 @@ c_unparser::emit_module_exit ()
   // In dyninst mode, now we're done with the contexts, transport, everything!
   if (session->runtime_usermode_p())
     {
-      o->newline() << "_stp_runtime_entryfn_put_context();";
+      o->newline() << "_stp_runtime_entryfn_put_context(c);";
       o->newline() << "_stp_dyninst_transport_shutdown();";
       o->newline() << "_stp_runtime_contexts_free();";
     }