From e481ab2e8fa454f877043e7f595dd6680aa4d4e8 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 22 May 2013 14:16:42 -0700 Subject: [PATCH] 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. --- runtime/dyninst/runtime_context.h | 28 +++++++++++------------ runtime/dyninst/transport.c | 38 ++++++++++++++++++------------- runtime/linux/runtime_context.h | 2 +- runtime/runtime.h | 2 +- tapsets.cxx | 17 +++++++++----- translate.cxx | 2 +- 6 files changed, 50 insertions(+), 39 deletions(-) diff --git a/runtime/dyninst/runtime_context.h b/runtime/dyninst/runtime_context.h index 31f88adae..031ffc050 100644 --- 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 0053b0274..325b59201 100644 --- 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 c7c552c9c..18122749c 100644 --- 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 6dd96d3cb..bbf5c6da0 100644 --- 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 81fa4d8c3..d04d9d34a 100644 --- 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 9a5e6e026..32d2fa64a 100644 --- 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();"; } -- 2.43.5