You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2015/02/23 21:35:06 UTC
trafficserver git commit: TS-3402: rationalize lock debugging
infrastructure
Repository: trafficserver
Updated Branches:
refs/heads/master 5f332c4b9 -> ac4a7dbe2
TS-3402: rationalize lock debugging infrastructure
Use SrcLoc to track lock holding locations. Enable lock diagnostics
in DEBUG builds when the "lock" debug tag is set. Previously you
had to do this *and* toggle some there magic #defines.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/ac4a7dbe
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/ac4a7dbe
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/ac4a7dbe
Branch: refs/heads/master
Commit: ac4a7dbe26ec83b66d311d3b9e1f9280a99172f6
Parents: 5f332c4
Author: James Peach <jp...@apache.org>
Authored: Tue Jan 27 13:12:43 2015 -0800
Committer: James Peach <jp...@apache.org>
Committed: Mon Feb 23 12:20:20 2015 -0800
----------------------------------------------------------------------
CHANGES | 2 ++
iocore/eventsystem/I_Lock.h | 72 +++++++++++++++++++---------------------
iocore/eventsystem/Lock.cc | 56 +++++++++++--------------------
lib/ts/Diags.h | 12 ++++++-
4 files changed, 67 insertions(+), 75 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac4a7dbe/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index da96344..eb1619f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
-*- coding: utf-8 -*-
Changes with Apache Traffic Server 5.3.0
+ *) [TS-3402] Rationalize lock debugging infrastructure.
+
*) [TS-3358] Add access checking to the management API.
*) [TS-3400] Use common FNV hash code everywhere.
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac4a7dbe/iocore/eventsystem/I_Lock.h
----------------------------------------------------------------------
diff --git a/iocore/eventsystem/I_Lock.h b/iocore/eventsystem/I_Lock.h
index d733e07..bc79f0b 100644
--- a/iocore/eventsystem/I_Lock.h
+++ b/iocore/eventsystem/I_Lock.h
@@ -49,7 +49,7 @@
*/
# ifdef DEBUG
-# define MUTEX_LOCK(_l,_m,_t) MutexLock _l(__FILE__,__LINE__,NULL,_m,_t)
+# define MUTEX_LOCK(_l,_m,_t) MutexLock _l(DiagsMakeLocation(),NULL,_m,_t)
# else
# define MUTEX_LOCK(_l,_m,_t) MutexLock _l(_m,_t)
# endif //DEBUG
@@ -69,7 +69,7 @@
*/
# define MUTEX_TRY_LOCK(_l,_m,_t) \
-MutexTryLock _l(__FILE__,__LINE__,(char*)NULL,_m,_t)
+MutexTryLock _l(DiagsMakeLocation(),(char*)NULL,_m,_t)
/**
Attempts to acquire the lock to the ProxyMutex.
@@ -86,7 +86,7 @@ MutexTryLock _l(__FILE__,__LINE__,(char*)NULL,_m,_t)
*/
# define MUTEX_TRY_LOCK_SPIN(_l,_m,_t,_sc) \
-MutexTryLock _l(__FILE__,__LINE__,(char*)NULL,_m,_t,_sc)
+MutexTryLock _l(DiagsMakeLocation(),(char*)NULL,_m,_t,_sc)
/**
Attempts to acquire the lock to the ProxyMutex.
@@ -104,7 +104,7 @@ MutexTryLock _l(__FILE__,__LINE__,(char*)NULL,_m,_t,_sc)
*/
# define MUTEX_TRY_LOCK_FOR(_l,_m,_t,_c) \
-MutexTryLock _l(__FILE__,__LINE__,NULL,_m,_t)
+MutexTryLock _l(DiagsMakeLocation(),NULL,_m,_t)
# else //DEBUG
# define MUTEX_TRY_LOCK(_l,_m,_t) MutexTryLock _l(_m,_t)
# define MUTEX_TRY_LOCK_SPIN(_l,_m,_t,_sc) MutexTryLock _l(_m,_t,_sc)
@@ -129,11 +129,11 @@ MutexTryLock _l(__FILE__,__LINE__,NULL,_m,_t)
// DEPRECATED DEPRECATED DEPRECATED
#ifdef DEBUG
# define MUTEX_TAKE_TRY_LOCK(_m,_t) \
-Mutex_trylock(__FILE__,__LINE__,(char*)NULL,_m,_t)
+Mutex_trylock(DiagsMakeLocation(),(char*)NULL,_m,_t)
# define MUTEX_TAKE_TRY_LOCK_FOR(_m,_t,_c) \
-Mutex_trylock(__FILE__,__LINE__,(char*)NULL,_m,_t)
+Mutex_trylock(DiagsMakeLocation(),(char*)NULL,_m,_t)
# define MUTEX_TAKE_TRY_LOCK_FOR_SPIN(_m,_t,_c,_sc) \
-Mutex_trylock_spin(__FILE__,__LINE__,NULL,_m,_t,_sc)
+Mutex_trylock_spin(DiagsMakeLocation(),NULL,_m,_t,_sc)
#else
# define MUTEX_TAKE_TRY_LOCK(_m,_t) Mutex_trylock(_m,_t)
# define MUTEX_TAKE_TRY_LOCK_FOR(_m,_t,_c) Mutex_trylock(_m,_t)
@@ -143,9 +143,9 @@ Mutex_trylock_spin(_m,_t,_sc)
#ifdef DEBUG
# define MUTEX_TAKE_LOCK(_m,_t)\
-Mutex_lock(__FILE__,__LINE__,(char*)NULL,_m,_t)
+Mutex_lock(DiagsMakeLocation(),(char*)NULL,_m,_t)
# define MUTEX_TAKE_LOCK_FOR(_m,_t,_c) \
-Mutex_lock(__FILE__,__LINE__,NULL,_m,_t)
+Mutex_lock(DiagsMakeLocation(),NULL,_m,_t)
#else
# define MUTEX_TAKE_LOCK(_m,_t) Mutex_lock(_m,_t)
# define MUTEX_TAKE_LOCK_FOR(_m,_t,_c) Mutex_lock(_m,_t)
@@ -159,9 +159,11 @@ class EThread;
typedef EThread *EThreadPtr;
typedef volatile EThreadPtr VolatileEThreadPtr;
-inkcoreapi extern void lock_waiting(const char *file, int line, const char *handler);
-inkcoreapi extern void lock_holding(const char *file, int line, const char *handler);
-extern void lock_taken(const char *file, int line, const char *handler);
+#if DEBUG
+inkcoreapi extern void lock_waiting(const SrcLoc&, const char *handler);
+inkcoreapi extern void lock_holding(const SrcLoc&, const char *handler);
+inkcoreapi extern void lock_taken(const SrcLoc&, const char *handler);
+#endif
/**
Lock object used in continuations and threads.
@@ -215,8 +217,7 @@ public:
#ifdef DEBUG
ink_hrtime hold_time;
- const char *file;
- int line;
+ SrcLoc srcloc;
const char *handler;
# ifdef MAX_LOCK_TAKEN
@@ -242,13 +243,14 @@ public:
*/
ProxyMutex()
+#ifdef DEBUG
+ : srcloc(NULL, NULL, 0)
+#endif
{
thread_holding = NULL;
nthread_holding = 0;
#ifdef DEBUG
hold_time = 0;
- file = NULL;
- line = 0;
handler = NULL;
# ifdef MAX_LOCK_TAKEN
taken = 0;
@@ -285,7 +287,7 @@ extern inkcoreapi ClassAllocator<ProxyMutex> mutexAllocator;
inline bool
Mutex_trylock(
#ifdef DEBUG
- const char *afile, int aline, const char *ahandler,
+ const SrcLoc& location, const char *ahandler,
#endif
ProxyMutex * m, EThread * t)
{
@@ -295,7 +297,7 @@ Mutex_trylock(
if (m->thread_holding != t) {
if (!ink_mutex_try_acquire(&m->the_mutex)) {
#ifdef DEBUG
- lock_waiting(m->file, m->line, m->handler);
+ lock_waiting(m->srcloc, m->handler);
#ifdef LOCK_CONTENTION_PROFILING
m->unsuccessful_nonblocking_acquires++;
m->nonblocking_acquires++;
@@ -307,8 +309,7 @@ Mutex_trylock(
}
m->thread_holding = t;
#ifdef DEBUG
- m->file = afile;
- m->line = aline;
+ m->srcloc = location;
m->handler = ahandler;
m->hold_time = ink_get_hrtime();
#ifdef MAX_LOCK_TAKEN
@@ -331,7 +332,7 @@ Mutex_trylock(
inline bool
Mutex_trylock_spin(
#ifdef DEBUG
- const char *afile, int aline, const char *ahandler,
+ const SrcLoc& location, const char *ahandler,
#endif
ProxyMutex * m, EThread * t, int spincnt = 1)
{
@@ -345,7 +346,7 @@ Mutex_trylock_spin(
} while (--spincnt);
if (!locked) {
#ifdef DEBUG
- lock_waiting(m->file, m->line, m->handler);
+ lock_waiting(m->srcloc, m->handler);
#ifdef LOCK_CONTENTION_PROFILING
m->unsuccessful_nonblocking_acquires++;
m->nonblocking_acquires++;
@@ -358,8 +359,7 @@ Mutex_trylock_spin(
m->thread_holding = t;
ink_assert(m->thread_holding);
#ifdef DEBUG
- m->file = afile;
- m->line = aline;
+ m->srcloc = location;
m->handler = ahandler;
m->hold_time = ink_get_hrtime();
#ifdef MAX_LOCK_TAKEN
@@ -382,7 +382,7 @@ Mutex_trylock_spin(
inline int
Mutex_lock(
#ifdef DEBUG
- const char *afile, int aline, const char *ahandler,
+ const SrcLoc& location, const char *ahandler,
#endif
ProxyMutex * m, EThread * t)
{
@@ -393,8 +393,7 @@ Mutex_lock(
m->thread_holding = t;
ink_assert(m->thread_holding);
#ifdef DEBUG
- m->file = afile;
- m->line = aline;
+ m->srcloc = location;
m->handler = ahandler;
m->hold_time = ink_get_hrtime();
#ifdef MAX_LOCK_TAKEN
@@ -422,13 +421,12 @@ Mutex_unlock(ProxyMutex * m, EThread * t)
if (!m->nthread_holding) {
#ifdef DEBUG
if (ink_get_hrtime() - m->hold_time > MAX_LOCK_TIME)
- lock_holding(m->file, m->line, m->handler);
+ lock_holding(m->srcloc, m->handler);
#ifdef MAX_LOCK_TAKEN
if (m->taken > MAX_LOCK_TAKEN)
- lock_taken(m->file, m->line, m->handler);
+ lock_taken(m->srcloc, m->handler);
#endif //MAX_LOCK_TAKEN
- m->file = NULL;
- m->line = 0;
+ m->srcloc = SrcLoc(NULL, NULL, 0);
m->handler = NULL;
#endif //DEBUG
ink_assert(m->thread_holding);
@@ -448,13 +446,13 @@ private:
public:
MutexLock(
#ifdef DEBUG
- const char *afile, int aline, const char *ahandler,
+ const SrcLoc& location, const char *ahandler,
#endif //DEBUG
ProxyMutex * am, EThread * t):m(am)
{
Mutex_lock(
#ifdef DEBUG
- afile, aline, ahandler,
+ location, ahandler,
#endif //DEBUG
m, t);
}
@@ -476,26 +474,26 @@ private:
public:
MutexTryLock(
#ifdef DEBUG
- const char *afile, int aline, const char *ahandler,
+ const SrcLoc& location, const char *ahandler,
#endif //DEBUG
ProxyMutex * am, EThread * t) : m(am)
{
lock_acquired = Mutex_trylock(
#ifdef DEBUG
- afile, aline, ahandler,
+ location, ahandler,
#endif //DEBUG
m, t);
}
MutexTryLock(
#ifdef DEBUG
- const char *afile, int aline, const char *ahandler,
+ const SrcLoc& location, const char *ahandler,
#endif //DEBUG
ProxyMutex * am, EThread * t, int sp) : m(am)
{
lock_acquired = Mutex_trylock_spin(
#ifdef DEBUG
- afile, aline, ahandler,
+ location, ahandler,
#endif //DEBUG
m, t, sp);
}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac4a7dbe/iocore/eventsystem/Lock.cc
----------------------------------------------------------------------
diff --git a/iocore/eventsystem/Lock.cc b/iocore/eventsystem/Lock.cc
index 8f9ee34..c58160a 100644
--- a/iocore/eventsystem/Lock.cc
+++ b/iocore/eventsystem/Lock.cc
@@ -29,56 +29,38 @@
**************************************************************************/
#include "P_EventSystem.h"
-
-ClassAllocator<ProxyMutex> mutexAllocator("mutexAllocator");
-
-// #define ERROR_CONFIG_TAG_LOCKS
-
-#ifdef ERROR_CONFIG_TAG_LOCKS
#include "Diags.h"
-#endif
-#ifdef DEBUG // debug build needs lock_* functions
-#undef INK_NO_LOCKS
-#endif
+ClassAllocator<ProxyMutex> mutexAllocator("mutexAllocator");
void
-lock_waiting(const char *file, int line, const char *handler)
+lock_waiting(const SrcLoc& srcloc, const char *handler)
{
- (void) file;
- (void) line;
- (void) handler;
-#ifdef ERROR_CONFIG_TAG_LOCKS
- if (is_diags_on("locks"))
- fprintf(stderr, "WARNING: waiting on lock %s:%d for %s\n",
- file ? file : "UNKNOWN", line, handler ? handler : "UNKNOWN");
-#endif
+ if (is_diags_on("locks")) {
+ char buf[128];
+ fprintf(stderr, "WARNING: waiting on lock %s for %s\n",
+ srcloc.str(buf, sizeof(buf)), handler ? handler : "UNKNOWN");
+ }
}
void
-lock_holding(const char *file, int line, const char *handler)
+lock_holding(const SrcLoc& srcloc, const char *handler)
{
- (void) file;
- (void) line;
- (void) handler;
-#ifdef ERROR_CONFIG_TAG_LOCKS
- if (is_diags_on("locks"))
- fprintf(stderr, "WARNING: holding lock %s:%d too long for %s\n",
- file ? file : "UNKNOWN", line, handler ? handler : "UNKNOWN");
-#endif
+ if (is_diags_on("locks")) {
+ char buf[128];
+ fprintf(stderr, "WARNING: holding lock %s too long for %s\n",
+ srcloc.str(buf, sizeof(buf)), handler ? handler : "UNKNOWN");
+ }
}
void
-lock_taken(const char *file, int line, const char *handler)
+lock_taken(const SrcLoc& srcloc, const char *handler)
{
- (void) file;
- (void) line;
- (void) handler;
-#ifdef ERROR_CONFIG_TAG_LOCKS
- if (is_diags_on("locks"))
- fprintf(stderr, "WARNING: lock %s:%d taken too many times for %s\n",
- file ? file : "UNKNOWN", line, handler ? handler : "UNKNOWN");
-#endif
+ if (is_diags_on("locks")) {
+ char buf[128];
+ fprintf(stderr, "WARNING: lock %s taken too many times for %s\n",
+ srcloc.str(buf, sizeof(buf)), handler ? handler : "UNKNOWN");
+ }
}
#ifdef LOCK_CONTENTION_PROFILING
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ac4a7dbe/lib/ts/Diags.h
----------------------------------------------------------------------
diff --git a/lib/ts/Diags.h b/lib/ts/Diags.h
index bad36c5..7a228c5 100644
--- a/lib/ts/Diags.h
+++ b/lib/ts/Diags.h
@@ -107,16 +107,26 @@ class SrcLoc
public:
const char *file;
const char *func;
- const int line;
+ int line;
bool valid() const {
return file && line;
}
+ SrcLoc(const SrcLoc& rhs) : file(rhs.file), func(rhs.func), line(rhs.line) {
+ }
+
SrcLoc(const char *_file, const char *_func, int _line)
: file(_file), func(_func), line(_line) {
}
+ SrcLoc& operator=(const SrcLoc& rhs) {
+ this->file = rhs.file;
+ this->func = rhs.func;
+ this->line = rhs.line;
+ return *this;
+ }
+
char * str(char *buf, int buflen) const;
};