maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Vicentiu,

I'm back from vacation, but quite busy with paperwork etc. I'll review your
patch tomorrow.

Regards,
Sergey

On Mon, Aug 01, 2016 at 02:46:03PM +0300, vicentiu wrote:
> revision-id: 5486f3458859fa4bd161150b9ac0e9c0f633a0e3 (mariadb-10.2.1-9-g5486f34)
> parent(s): 08683a726773f8cdf16a4a3dfb3920e5f7842481
> author: Vicențiu Ciorbaru
> committer: Vicențiu Ciorbaru
> timestamp: 2016-08-01 14:43:41 +0300
> message:
> 
> MDEV-10341: InnoDB: Failing assertion: mutex_own(mutex) - mutex_exit_func
> 
> Fix memory barrier issues on releasing mutexes. We must have
> 
> ---
>  storage/innobase/include/os0sync.h    | 17 ++++++++---------
>  storage/innobase/include/sync0sync.ic |  2 ++
>  storage/xtradb/include/os0sync.h      | 16 ++++++++--------
>  storage/xtradb/include/sync0sync.ic   |  2 ++
>  4 files changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/storage/innobase/include/os0sync.h b/storage/innobase/include/os0sync.h
> index 1cf4e9c..08911e7 100644
> --- a/storage/innobase/include/os0sync.h
> +++ b/storage/innobase/include/os0sync.h
> @@ -479,20 +479,13 @@ os_atomic_test_and_set(volatile lock_word_t* ptr)
>  }
>  
>  /** Do an atomic release.
> -
> -In theory __sync_lock_release should be used to release the lock.
> -Unfortunately, it does not work properly alone. The workaround is
> -that more conservative __sync_lock_test_and_set is used instead.
> -
> -Performance regression was observed at some conditions for Intel
> -architecture. Disable release barrier on Intel architecture for now.
>  @param[in,out]	ptr		Memory location to write to
>  @return the previous value */
>  inline
> -lock_word_t
> +void
>  os_atomic_clear(volatile lock_word_t* ptr)
>  {
> -	return(__sync_lock_test_and_set(ptr, 0));
> +	__sync_lock_release(ptr);
>  }
>  
>  # elif defined(HAVE_IB_GCC_ATOMIC_TEST_AND_SET)
> @@ -861,6 +854,12 @@ for synchronization */
>  architecture. Disable memory barrier for Intel architecture for now. */
>  # define os_rmb do { } while(0)
>  # define os_wmb do { } while(0)
> +# ifndef __WIN__
> +#  define os_mb  __sync_synchronize()
> +# else
> +   /* Windows atomic primitives imply a full memory barrier themselves. */
> +#  define os_mb  do { } while(0)
> +# endif
>  # define os_isync do { } while(0)
>  # define IB_MEMORY_BARRIER_STARTUP_MSG \
>  	"Memory barrier is not used"
> diff --git a/storage/innobase/include/sync0sync.ic b/storage/innobase/include/sync0sync.ic
> index 85c7f75..19474e7 100644
> --- a/storage/innobase/include/sync0sync.ic
> +++ b/storage/innobase/include/sync0sync.ic
> @@ -178,6 +178,8 @@ mutex_exit_func(
>  	to wake up possible hanging threads if
>  	they are missed in mutex_signal_object. */
>  
> +	os_mb;
> +
>  	if (mutex_get_waiters(mutex) != 0) {
>  
>  		mutex_signal_object(mutex);
> diff --git a/storage/xtradb/include/os0sync.h b/storage/xtradb/include/os0sync.h
> index 0f93f3f..f06aa9d 100644
> --- a/storage/xtradb/include/os0sync.h
> +++ b/storage/xtradb/include/os0sync.h
> @@ -531,19 +531,13 @@ os_atomic_test_and_set(volatile lock_word_t* ptr)
>  
>  /** Do an atomic release.
>  
> -In theory __sync_lock_release should be used to release the lock.
> -Unfortunately, it does not work properly alone. The workaround is
> -that more conservative __sync_lock_test_and_set is used instead.
> -
> -Performance regression was observed at some conditions for Intel
> -architecture. Disable release barrier on Intel architecture for now.
>  @param[in,out]	ptr		Memory location to write to
>  @return the previous value */
>  inline
> -lock_word_t
> +void
>  os_atomic_clear(volatile lock_word_t* ptr)
>  {
> -	return(__sync_lock_test_and_set(ptr, 0));
> +	__sync_lock_release(ptr);
>  }
>  
>  # elif defined(HAVE_IB_GCC_ATOMIC_TEST_AND_SET)
> @@ -912,6 +906,12 @@ for synchronization */
>  architecture. Disable memory barrier for Intel architecture for now. */
>  # define os_rmb do { } while(0)
>  # define os_wmb do { } while(0)
> +# ifndef __WIN__
> +#  define os_mb  __sync_synchronize()
> +# else
> +   /* Windows atomic primitives imply a full memory barrier themselves. */
> +#  define os_mb  do { } while(0)
> +# endif
>  # define os_isync do { } while(0)
>  # define IB_MEMORY_BARRIER_STARTUP_MSG \
>  	"Memory barrier is not used"
> diff --git a/storage/xtradb/include/sync0sync.ic b/storage/xtradb/include/sync0sync.ic
> index 83f28bf..dbecc79 100644
> --- a/storage/xtradb/include/sync0sync.ic
> +++ b/storage/xtradb/include/sync0sync.ic
> @@ -181,6 +181,8 @@ mutex_exit_func(
>  	to wake up possible hanging threads if
>  	they are missed in mutex_signal_object. */
>  
> +	os_mb;
> +
>  	if (mutex_get_waiters(mutex) != 0) {
>  
>  		mutex_signal_object(mutex);
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits