[Asterisk-code-review] lock: Reduce storage related to DEBUG THREADS. (asterisk[master])

Corey Farrell asteriskteam at digium.com
Thu Sep 27 05:04:21 CDT 2018


Corey Farrell has uploaded this change for review. ( https://gerrit.asterisk.org/10293


Change subject: lock: Reduce storage related to DEBUG_THREADS.
......................................................................

lock: Reduce storage related to DEBUG_THREADS.

This reduces storage related to DEBUG_THREADS by combining tracking
with track in mutex / rwlock structures.  The magic value (void*)-1
AST_LOCK_TRACK_DISABLED is used when tracking is disabled.  The field
has been renamed from 'track' to 'tracker' to ensure outside code must
be updated to avoid dereferencing AST_LOCK_TRACK_DISABLED.

Change-Id: Iabd650908901843e9fff47ef1c539f0e1b8cb13b
---
M include/asterisk/lock.h
M main/lock.c
M main/utils.c
3 files changed, 49 insertions(+), 82 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/93/10293/1

diff --git a/include/asterisk/lock.h b/include/asterisk/lock.h
index a46d047..2bea32b 100644
--- a/include/asterisk/lock.h
+++ b/include/asterisk/lock.h
@@ -66,6 +66,8 @@
 #define AST_PTHREADT_NULL (pthread_t) -1
 #define AST_PTHREADT_STOP (pthread_t) -2
 
+#define AST_LOCK_TRACK_DISABLED (void*) -1
+
 #if (defined(SOLARIS) || defined(BSD))
 #define AST_MUTEX_INIT_W_CONSTRUCTORS
 #endif /* SOLARIS || BSD */
@@ -92,11 +94,11 @@
 #define AST_LOCK_TRACK_INIT_VALUE { { NULL }, { 0 }, 0, { NULL }, { 0 }, PTHREAD_MUTEX_INIT_VALUE }
 #endif
 
-#define AST_MUTEX_INIT_VALUE { PTHREAD_MUTEX_INIT_VALUE, NULL, 1 }
-#define AST_MUTEX_INIT_VALUE_NOTRACKING { PTHREAD_MUTEX_INIT_VALUE, NULL, 0 }
+#define AST_MUTEX_INIT_VALUE { PTHREAD_MUTEX_INIT_VALUE, NULL }
+#define AST_MUTEX_INIT_VALUE_NOTRACKING { PTHREAD_MUTEX_INIT_VALUE, AST_LOCK_TRACK_DISABLED }
 
-#define AST_RWLOCK_INIT_VALUE { __AST_RWLOCK_INIT_VALUE, NULL, 1 }
-#define AST_RWLOCK_INIT_VALUE_NOTRACKING { __AST_RWLOCK_INIT_VALUE, NULL, 0 }
+#define AST_RWLOCK_INIT_VALUE { __AST_RWLOCK_INIT_VALUE, NULL }
+#define AST_RWLOCK_INIT_VALUE_NOTRACKING { __AST_RWLOCK_INIT_VALUE, AST_LOCK_TRACK_DISABLED }
 
 #define AST_MAX_REENTRANCY 10
 
@@ -128,8 +130,7 @@
 struct ast_mutex_info {
 	pthread_mutex_t mutex;
 	/*! Track which thread holds this mutex */
-	struct ast_lock_track *track;
-	unsigned int tracking:1;
+	struct ast_lock_track *tracker;
 };
 
 /*! \brief Structure for rwlock and tracking information.
@@ -140,8 +141,7 @@
 struct ast_rwlock_info {
 	pthread_rwlock_t lock;
 	/*! Track which thread holds this lock */
-	struct ast_lock_track *track;
-	unsigned int tracking:1;
+	struct ast_lock_track *tracker;
 };
 
 typedef struct ast_mutex_info ast_mutex_t;
diff --git a/main/lock.c b/main/lock.c
index dec814f..5d632b3 100644
--- a/main/lock.c
+++ b/main/lock.c
@@ -77,6 +77,10 @@
 	pthread_mutexattr_t reentr_attr;
 	struct ast_lock_track *lt;
 
+	if (*plt == AST_LOCK_TRACK_DISABLED) {
+		return NULL;
+	}
+
 	/* It's a bit painful to lock a global mutex for every access to the
 	 * reentrancy structure, but it's necessary to ensure that we don't
 	 * double-allocate the structure or double-initialize the reentr_mutex.
@@ -114,11 +118,16 @@
 	return lt;
 }
 
+#define ast_get_tracker_existing(t) ({ \
+		struct ast_lock_track *tracker = t->tracker; \
+		(tracker == AST_LOCK_TRACK_DISABLED ? NULL : tracker); \
+	})
+
 static inline void delete_reentrancy_cs(struct ast_lock_track **plt)
 {
 	struct ast_lock_track *lt;
 
-	if (*plt) {
+	if (*plt && *plt != AST_LOCK_TRACK_DISABLED) {
 		lt = *plt;
 		*plt = NULL;
 
@@ -147,8 +156,7 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	t->track = NULL;
-	t->tracking = tracking;
+	t->tracker = tracking ? NULL : AST_LOCK_TRACK_DISABLED;
 #endif /* DEBUG_THREADS */
 
 	pthread_mutexattr_init(&attr);
@@ -165,8 +173,8 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = t->track;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	struct ast_lock_track *lt = ast_get_tracker_existing(t);
+	int canlog = lt && strcmp(filename, "logger.c");
 
 #if defined(AST_MUTEX_INIT_W_CONSTRUCTORS) && defined(CAN_COMPARE_MUTEX_TO_INIT_VALUE)
 	if ((t->mutex) == ((pthread_mutex_t) PTHREAD_MUTEX_INITIALIZER)) {
@@ -230,7 +238,7 @@
 		memset(&lt->backtrace[0], 0, sizeof(lt->backtrace[0]));
 #endif
 		ast_reentrancy_unlock(lt);
-		delete_reentrancy_cs(&t->track);
+		delete_reentrancy_cs(&t->tracker);
 	}
 #endif /* DEBUG_THREADS */
 
@@ -243,14 +251,10 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = NULL;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	struct ast_lock_track *lt = ast_get_reentrancy(&t->tracker);
+	int canlog = lt && strcmp(filename, "logger.c");
 	struct ast_bt *bt = NULL;
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
-
 	if (lt) {
 #ifdef HAVE_BKTR
 		struct ast_bt tmp;
@@ -360,14 +364,10 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = NULL;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	struct ast_lock_track *lt = ast_get_reentrancy(&t->tracker);
+	int canlog = lt && strcmp(filename, "logger.c");
 	struct ast_bt *bt = NULL;
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
-
 	if (lt) {
 #ifdef HAVE_BKTR
 		struct ast_bt tmp;
@@ -420,7 +420,7 @@
 
 #ifdef DEBUG_THREADS
 	struct ast_lock_track *lt = NULL;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	int canlog = t->tracker != AST_LOCK_TRACK_DISABLED && strcmp(filename, "logger.c");
 	struct ast_bt *bt = NULL;
 
 #if defined(AST_MUTEX_INIT_W_CONSTRUCTORS) && defined(CAN_COMPARE_MUTEX_TO_INIT_VALUE)
@@ -432,9 +432,7 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
+	lt = ast_get_reentrancy(&t->tracker);
 
 	if (lt) {
 		ast_reentrancy_lock(lt);
@@ -543,7 +541,7 @@
 #ifdef DEBUG_THREADS
 	struct ast_lock_track *lt = NULL;
 	struct ast_lock_track lt_orig;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	int canlog = t->tracker != AST_LOCK_TRACK_DISABLED && strcmp(filename, "logger.c");
 
 #if defined(AST_MUTEX_INIT_W_CONSTRUCTORS) && defined(CAN_COMPARE_MUTEX_TO_INIT_VALUE)
 	if ((t->mutex) == ((pthread_mutex_t) PTHREAD_MUTEX_INITIALIZER)) {
@@ -554,9 +552,7 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
+	lt = ast_get_reentrancy(&t->tracker);
 
 	if (lt) {
 		ast_reentrancy_lock(lt);
@@ -611,7 +607,7 @@
 #ifdef DEBUG_THREADS
 	struct ast_lock_track *lt = NULL;
 	struct ast_lock_track lt_orig;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	int canlog = t->tracker != AST_LOCK_TRACK_DISABLED && strcmp(filename, "logger.c");
 
 #if defined(AST_MUTEX_INIT_W_CONSTRUCTORS) && defined(CAN_COMPARE_MUTEX_TO_INIT_VALUE)
 	if ((t->mutex) == ((pthread_mutex_t) PTHREAD_MUTEX_INITIALIZER)) {
@@ -622,9 +618,7 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
+	lt = ast_get_reentrancy(&t->tracker);
 
 	if (lt) {
 		ast_reentrancy_lock(lt);
@@ -687,8 +681,7 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	t->track = NULL;
-	t->tracking = tracking;
+	t->tracker = tracking ? NULL : AST_LOCK_TRACK_DISABLED;
 #endif /* DEBUG_THREADS */
 
 	pthread_rwlockattr_init(&attr);
@@ -706,8 +699,8 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = t->track;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	struct ast_lock_track *lt = ast_get_tracker_existing(t);
+	int canlog = lt && strcmp(filename, "logger.c");
 
 #if defined(AST_MUTEX_INIT_W_CONSTRUCTORS) && defined(CAN_COMPARE_MUTEX_TO_INIT_VALUE)
 	if (t->lock == ((pthread_rwlock_t) __AST_RWLOCK_INIT_VALUE)) {
@@ -741,7 +734,7 @@
 		memset(&lt->backtrace[0], 0, sizeof(lt->backtrace[0]));
 #endif
 		ast_reentrancy_unlock(lt);
-		delete_reentrancy_cs(&t->track);
+		delete_reentrancy_cs(&t->tracker);
 	}
 #endif /* DEBUG_THREADS */
 
@@ -754,7 +747,7 @@
 
 #ifdef DEBUG_THREADS
 	struct ast_lock_track *lt = NULL;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	int canlog = t->tracker != AST_LOCK_TRACK_DISABLED && strcmp(filename, "logger.c");
 	struct ast_bt *bt = NULL;
 	int lock_found = 0;
 
@@ -767,9 +760,7 @@
 	}
 #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
+	lt = ast_get_reentrancy(&t->tracker);
 
 	if (lt) {
 		ast_reentrancy_lock(lt);
@@ -827,14 +818,10 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = NULL;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	struct ast_lock_track *lt = ast_get_reentrancy(&t->tracker);
+	int canlog = lt && strcmp(filename, "logger.c");
 	struct ast_bt *bt = NULL;
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
-
 	if (lt) {
 #ifdef HAVE_BKTR
 		struct ast_bt tmp;
@@ -929,14 +916,10 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = NULL;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	struct ast_lock_track *lt = ast_get_reentrancy(&t->tracker);
+	int canlog = lt && strcmp(filename, "logger.c");
 	struct ast_bt *bt = NULL;
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
-
 	if (lt) {
 #ifdef HAVE_BKTR
 		struct ast_bt tmp;
@@ -1031,14 +1014,10 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = NULL;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	struct ast_lock_track *lt = ast_get_reentrancy(&t->tracker);
+	int canlog = lt && strcmp(filename, "logger.c");
 	struct ast_bt *bt = NULL;
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
-
 	if (lt) {
 #ifdef HAVE_BKTR
 		struct ast_bt tmp;
@@ -1117,14 +1096,10 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = NULL;
-	int canlog = t->tracking && strcmp(filename, "logger.c");
+	struct ast_lock_track *lt = ast_get_reentrancy(&t->tracker);
+	int canlog = lt && strcmp(filename, "logger.c");
 	struct ast_bt *bt = NULL;
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
-
 	if (lt) {
 #ifdef HAVE_BKTR
 		struct ast_bt tmp;
@@ -1202,13 +1177,9 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = NULL;
+	struct ast_lock_track *lt = ast_get_reentrancy(&t->tracker);
 	struct ast_bt *bt = NULL;
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
-
 	if (lt) {
 #ifdef HAVE_BKTR
 		struct ast_bt tmp;
@@ -1256,13 +1227,9 @@
 	int res;
 
 #ifdef DEBUG_THREADS
-	struct ast_lock_track *lt = NULL;
+	struct ast_lock_track *lt = ast_get_reentrancy(&t->tracker);
 	struct ast_bt *bt = NULL;
 
-	if (t->tracking) {
-		lt = ast_get_reentrancy(&t->track);
-	}
-
 	if (lt) {
 #ifdef HAVE_BKTR
 		struct ast_bt tmp;
diff --git a/main/utils.c b/main/utils.c
index 75f4360..108b442 100644
--- a/main/utils.c
+++ b/main/utils.c
@@ -1020,7 +1020,7 @@
 		return;
 
 	lock = lock_info->locks[i].lock_addr;
-	lt = lock->track;
+	lt = lock->tracker == AST_LOCK_TRACK_DISABLED ? NULL : lock->tracker;
 	ast_reentrancy_lock(lt);
 	for (j = 0; *str && j < lt->reentrancy; j++) {
 		ast_str_append(str, 0, "=== --- ---> Locked Here: %s line %d (%s)\n",

-- 
To view, visit https://gerrit.asterisk.org/10293
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iabd650908901843e9fff47ef1c539f0e1b8cb13b
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180927/18c63f6a/attachment-0001.html>


More information about the asterisk-code-review mailing list