Skip to content

Commit

Permalink
Backport from 10.0:
Browse files Browse the repository at this point in the history
MDEV-6483 - Deadlock around rw_lock_debug_mutex on PPC64

This problem affects only debug builds on PPC64.

There are at least two race conditions around
rw_lock_debug_mutex_enter and rw_lock_debug_mutex_exit:

- rw_lock_debug_waiters was loaded/stored without setting
  appropriate locks/memory barriers.
- there is a gap between calls to os_event_reset() and
  os_event_wait() and in such case we're supposed to pass
  return value of the former to the latter.

Fixed by replacing self-cooked spinlocks with system mutexes.
These days system mutexes offer much better performance. OTOH
performance is not that critical for debug builds.
  • Loading branch information
Sergey Vojtovich committed Aug 29, 2014
1 parent 4049757 commit c01c819
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 82 deletions.
8 changes: 1 addition & 7 deletions storage/innobase/include/sync0rw.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,8 @@ extern mutex_t rw_lock_list_mutex;
#ifdef UNIV_SYNC_DEBUG
/* The global mutex which protects debug info lists of all rw-locks.
To modify the debug info list of an rw-lock, this mutex has to be
acquired in addition to the mutex protecting the lock. */
extern mutex_t rw_lock_debug_mutex;
extern os_event_t rw_lock_debug_event; /*!< If deadlock detection does
not get immediately the mutex it
may wait for this event */
extern ibool rw_lock_debug_waiters; /*!< This is set to TRUE, if
there may be waiters for the event */
extern os_fast_mutex_t rw_lock_debug_mutex;
#endif /* UNIV_SYNC_DEBUG */

/** number of spin waits on rw-latches,
Expand Down
32 changes: 3 additions & 29 deletions storage/innobase/sync/sync0rw.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,18 +180,12 @@ UNIV_INTERN mysql_pfs_key_t rw_lock_mutex_key;
To modify the debug info list of an rw-lock, this mutex has to be
acquired in addition to the mutex protecting the lock. */

UNIV_INTERN mutex_t rw_lock_debug_mutex;
UNIV_INTERN os_fast_mutex_t rw_lock_debug_mutex;

# ifdef UNIV_PFS_MUTEX
UNIV_INTERN mysql_pfs_key_t rw_lock_debug_mutex_key;
# endif

/* If deadlock detection does not get immediately the mutex,
it may wait for this event */
UNIV_INTERN os_event_t rw_lock_debug_event;
/* This is set to TRUE, if there may be waiters for the event */
UNIV_INTERN ibool rw_lock_debug_waiters;

/******************************************************************//**
Creates a debug info struct. */
static
Expand Down Expand Up @@ -736,22 +730,7 @@ void
rw_lock_debug_mutex_enter(void)
/*===========================*/
{
loop:
if (0 == mutex_enter_nowait(&rw_lock_debug_mutex)) {
return;
}

os_event_reset(rw_lock_debug_event);

rw_lock_debug_waiters = TRUE;

if (0 == mutex_enter_nowait(&rw_lock_debug_mutex)) {
return;
}

os_event_wait(rw_lock_debug_event);

goto loop;
os_fast_mutex_lock(&rw_lock_debug_mutex);
}

/******************************************************************//**
Expand All @@ -761,12 +740,7 @@ void
rw_lock_debug_mutex_exit(void)
/*==========================*/
{
mutex_exit(&rw_lock_debug_mutex);

if (rw_lock_debug_waiters) {
rw_lock_debug_waiters = FALSE;
os_event_set(rw_lock_debug_event);
}
os_fast_mutex_unlock(&rw_lock_debug_mutex);
}

/******************************************************************//**
Expand Down
7 changes: 2 additions & 5 deletions storage/innobase/sync/sync0sync.c
Original file line number Diff line number Diff line change
Expand Up @@ -1535,11 +1535,7 @@ sync_init(void)
SYNC_NO_ORDER_CHECK);

#ifdef UNIV_SYNC_DEBUG
mutex_create(rw_lock_debug_mutex_key, &rw_lock_debug_mutex,
SYNC_NO_ORDER_CHECK);

rw_lock_debug_event = os_event_create(NULL);
rw_lock_debug_waiters = FALSE;
os_fast_mutex_init(rw_lock_debug_mutex_key, &rw_lock_debug_mutex);
#endif /* UNIV_SYNC_DEBUG */
}

Expand Down Expand Up @@ -1607,6 +1603,7 @@ sync_close(void)
sync_order_checks_on = FALSE;

sync_thread_level_arrays_free();
os_fast_mutex_free(&rw_lock_debug_mutex);
#endif /* UNIV_SYNC_DEBUG */

sync_initialized = FALSE;
Expand Down
8 changes: 1 addition & 7 deletions storage/xtradb/include/sync0rw.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,8 @@ extern mutex_t rw_lock_list_mutex;
#ifdef UNIV_SYNC_DEBUG
/* The global mutex which protects debug info lists of all rw-locks.
To modify the debug info list of an rw-lock, this mutex has to be
acquired in addition to the mutex protecting the lock. */
extern mutex_t rw_lock_debug_mutex;
extern os_event_t rw_lock_debug_event; /*!< If deadlock detection does
not get immediately the mutex it
may wait for this event */
extern ibool rw_lock_debug_waiters; /*!< This is set to TRUE, if
there may be waiters for the event */
extern os_fast_mutex_t rw_lock_debug_mutex;
#endif /* UNIV_SYNC_DEBUG */

/** number of spin waits on rw-latches,
Expand Down
32 changes: 3 additions & 29 deletions storage/xtradb/sync/sync0rw.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,18 +180,12 @@ UNIV_INTERN mysql_pfs_key_t rw_lock_mutex_key;
To modify the debug info list of an rw-lock, this mutex has to be
acquired in addition to the mutex protecting the lock. */

UNIV_INTERN mutex_t rw_lock_debug_mutex;
UNIV_INTERN os_fast_mutex_t rw_lock_debug_mutex;

# ifdef UNIV_PFS_MUTEX
UNIV_INTERN mysql_pfs_key_t rw_lock_debug_mutex_key;
# endif

/* If deadlock detection does not get immediately the mutex,
it may wait for this event */
UNIV_INTERN os_event_t rw_lock_debug_event;
/* This is set to TRUE, if there may be waiters for the event */
UNIV_INTERN ibool rw_lock_debug_waiters;

/******************************************************************//**
Creates a debug info struct. */
static
Expand Down Expand Up @@ -731,22 +725,7 @@ void
rw_lock_debug_mutex_enter(void)
/*===========================*/
{
loop:
if (0 == mutex_enter_nowait(&rw_lock_debug_mutex)) {
return;
}

os_event_reset(rw_lock_debug_event);

rw_lock_debug_waiters = TRUE;

if (0 == mutex_enter_nowait(&rw_lock_debug_mutex)) {
return;
}

os_event_wait(rw_lock_debug_event);

goto loop;
os_fast_mutex_lock(&rw_lock_debug_mutex);
}

/******************************************************************//**
Expand All @@ -756,12 +735,7 @@ void
rw_lock_debug_mutex_exit(void)
/*==========================*/
{
mutex_exit(&rw_lock_debug_mutex);

if (rw_lock_debug_waiters) {
rw_lock_debug_waiters = FALSE;
os_event_set(rw_lock_debug_event);
}
os_fast_mutex_unlock(&rw_lock_debug_mutex);
}

/******************************************************************//**
Expand Down
7 changes: 2 additions & 5 deletions storage/xtradb/sync/sync0sync.c
Original file line number Diff line number Diff line change
Expand Up @@ -1514,11 +1514,7 @@ sync_init(void)
SYNC_NO_ORDER_CHECK);

#ifdef UNIV_SYNC_DEBUG
mutex_create(rw_lock_debug_mutex_key, &rw_lock_debug_mutex,
SYNC_NO_ORDER_CHECK);

rw_lock_debug_event = os_event_create(NULL);
rw_lock_debug_waiters = FALSE;
os_fast_mutex_init(rw_lock_debug_mutex_key, &rw_lock_debug_mutex);
#endif /* UNIV_SYNC_DEBUG */
}

Expand Down Expand Up @@ -1586,6 +1582,7 @@ sync_close(void)
sync_order_checks_on = FALSE;

sync_thread_level_arrays_free();
os_fast_mutex_free(&rw_lock_debug_mutex);
#endif /* UNIV_SYNC_DEBUG */

sync_initialized = FALSE;
Expand Down

0 comments on commit c01c819

Please sign in to comment.