Merge lp:~laurynas-biveinis/percona-server/bug1295523 into lp:percona-server/5.6

[Laurynas Biveinis]

2nd MP:

Introduce a separate priority mutex list, free both lists in sync_close.
http://jenkins.percona.com/job/percona-server-5.6-param/560/

1st MP:

Fix bug 1295523 (sync_close destroys priority mutexes as regular
ones).

First, assert that sync_close never sees any priority mutexes. For
that introduce debug-only priority mutex counter
sync_priority_mutex_count, maintain it in mutex_create_func and
mutex_free_func.

Then, introduce new function buf_pool_free_prio_mutexes, called on
server shutdown just before sync_close, that frees the priority
mutexes. The option of releasing the mutexes in buf_free does not
work, because its call cannot go before sync_close.

http://jenkins.percona.com/job/percona-server-5.6-param/559/

[Alexey Kopytov (akopytov)]

One has to maintain a list of exceptions from sync_close() in buf_pool_free_prio_mutexes() whenever a new prio mutex is added to buf_pool_t. The new assertion helps, but is still ugly. Introducing a prio mutex outside of buf_pool_t will require even less trivial changes.

How about making it transparent to callers? E.g. introduce prio_mutex_list where prio mutexes will be added in addition to mutex_list. In sync_close() walk prio_mutex_list first and use the correct mutex_free_func() overload. In the overload, remove it from prio_mutex_list before calling mutex_free_func() for non-prio mutexes.

review: Needs Fixing

[Laurynas Biveinis (laurynas-biveinis)]

Good idea. Priority mutex struct will have to get the list node fields, but that should be fine, there are few of those mutexes.

[Alexey Kopytov (akopytov)]

review: Approve

=== modified file 'storage/innobase/include/sync0sync.h'
--- storage/innobase/include/sync0sync.h	2014-01-27 10:35:37 +0000
+++ storage/innobase/include/sync0sync.h	2014-03-21 13:23:05 +0000
@@ -984,6 +984,7 @@
 					priority in the global wait array
 					waiting for this mutex to be
 					released. */
+	UT_LIST_NODE_T(ib_prio_mutex_t)	list;
 };
 
 /** Constant determining how long spin wait is continued before suspending

=== modified file 'storage/innobase/sync/sync0sync.cc'
--- storage/innobase/sync/sync0sync.cc	2014-02-17 11:12:40 +0000
+++ storage/innobase/sync/sync0sync.cc	2014-03-21 13:23:05 +0000
@@ -209,7 +209,10 @@
 /** Global list of database mutexes (not OS mutexes) created. */
 UNIV_INTERN ut_list_base_node_t  mutex_list;
 
-/** Mutex protecting the mutex_list variable */
+/** Global list of priority mutexes. A subset of mutex_list */
+UNIV_INTERN UT_LIST_BASE_NODE_T(ib_prio_mutex_t)  prio_mutex_list;
+
+/** Mutex protecting the mutex_list and prio_mutex_list variables */
 UNIV_INTERN ib_mutex_t mutex_list_mutex;
 
 #ifdef UNIV_PFS_MUTEX
@@ -353,6 +356,10 @@
 			  cmutex_name);
 	mutex->high_priority_waiters = 0;
 	mutex->high_priority_event = os_event_create();
+
+	mutex_enter(&mutex_list_mutex);
+	UT_LIST_ADD_FIRST(list, prio_mutex_list, mutex);
+	mutex_exit(&mutex_list_mutex);
 }
 
 /******************************************************************//**
@@ -426,6 +433,10 @@
 /*============*/
 	ib_prio_mutex_t*	mutex)	/*!< in: mutex */
 {
+	mutex_enter(&mutex_list_mutex);
+	UT_LIST_REMOVE(list, prio_mutex_list, mutex);
+	mutex_exit(&mutex_list_mutex);
+
 	ut_a(mutex->high_priority_waiters == 0);
 	os_event_free(mutex->high_priority_event);
 	mutex_free_func(&mutex->base_mutex);
@@ -1572,6 +1583,7 @@
 	/* Init the mutex list and create the mutex to protect it. */
 
 	UT_LIST_INIT(mutex_list);
+	UT_LIST_INIT(prio_mutex_list);
 	mutex_create(mutex_list_mutex_key, &mutex_list_mutex,
 		     SYNC_NO_ORDER_CHECK);
 #ifdef UNIV_SYNC_DEBUG
@@ -1630,10 +1642,16 @@
 sync_close(void)
 /*===========*/
 {
-	ib_mutex_t*	mutex;
+	ib_mutex_t*		mutex;
+	ib_prio_mutex_t*	prio_mutex;
 
 	sync_array_close();
 
+	for (prio_mutex = UT_LIST_GET_FIRST(prio_mutex_list); prio_mutex;) {
+		mutex_free(prio_mutex);
+		prio_mutex = UT_LIST_GET_FIRST(prio_mutex_list);
+	}
+
 	for (mutex = UT_LIST_GET_FIRST(mutex_list);
 	     mutex != NULL;
 	     /* No op */) {