[Asterisk-code-review] lock: Improve performance of DEBUG THREADS. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/10293 )

Change subject: lock: Improve performance of DEBUG_THREADS.
......................................................................


Patch Set 2: Code-Review-1

(11 comments)

https://gerrit.asterisk.org/#/c/10293/2/include/asterisk/lock.h
File include/asterisk/lock.h:

https://gerrit.asterisk.org/#/c/10293/2/include/asterisk/lock.h@144
PS2, Line 144: #elif defined(DEBUG_THREADS) || !defined(DEBUG_THREADS_LOOSE_ABI)
You don't need to test DEBUG_THREADS_LOOSE_ABI here.

If DEBUG_THREADS is enabled you need to have these members everywhere.


https://gerrit.asterisk.org/#/c/10293/2/include/asterisk/lock.h@158
PS2, Line 158: #if defined(DEBUG_THREADS) || !defined(DEBUG_THREADS_LOOSE_ABI)
             : 	/*! Track which thread holds this lock */
             : 	struct ast_lock_track *track;
             : 	struct ast_lock_track_flags flags;
             : #endif
You should be able to do the same conditionals here as for ast_mutex_info.


https://gerrit.asterisk.org/#/c/10293/2/main/lock.c
File main/lock.c:

https://gerrit.asterisk.org/#/c/10293/2/main/lock.c@81
PS2, Line 81: 	if (no_setup || !flags->tracking || flags->setup) {
To be safer, we should test no_setup after we get the reentrancy lock.


https://gerrit.asterisk.org/#/c/10293/2/main/lock.c@87
PS2, Line 87: 	if (*plt) {
            : 		pthread_mutex_unlock(&reentrancy_lock.mutex);
            : 		return *plt;
            : 	}
This should be safer concerning the pointer we return when no_setup is set because we are destroying the lock.  We would only get this far if the lock was not already setup when we are destroying the lock.

if (*plt) {
	pthread_mutex_unlock(&reentrancy_lock.mutex);
	return *plt;
}
if (no_setup) {
	pthread_mutex_unlock(&reentrancy_lock.mutex);
	return NULL;
}


https://gerrit.asterisk.org/#/c/10293/2/main/lock.c@147
PS2, Line 147: 	t->flags.tracking = tracking;
Should also set t->flags.setup = 0;


https://gerrit.asterisk.org/#/c/10293/2/main/lock.c@356
PS2, Line 356: 	int canlog = lt && strcmp(filename, "logger.c");
canlog is not used


https://gerrit.asterisk.org/#/c/10293/2/main/lock.c@670
PS2, Line 670: 	t->flags.tracking = tracking;
Should also set t->flags.setup = 0;


https://gerrit.asterisk.org/#/c/10293/2/main/lock.c@689
PS2, Line 689: 	int canlog = lt && strcmp(filename, "logger.c");
canlog not used


https://gerrit.asterisk.org/#/c/10293/2/main/lock.c@736
PS2, Line 736: 	int canlog = t->flags.tracking && strcmp(filename, "logger.c");
canlog not used


https://gerrit.asterisk.org/#/c/10293/2/main/lock.c@1003
PS2, Line 1003: 	int canlog = lt && strcmp(filename, "logger.c");
canlog not used


https://gerrit.asterisk.org/#/c/10293/2/main/lock.c@1085
PS2, Line 1085: 	int canlog = lt && strcmp(filename, "logger.c");
canlog not used



-- 
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: comment
Gerrit-Change-Id: Iabd650908901843e9fff47ef1c539f0e1b8cb13b
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 2
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Fri, 28 Sep 2018 17:10:09 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180928/f0716277/attachment.html>