From 44aae1ec25b3ccfd08e27374a48c4426e5564a11 Mon Sep 17 00:00:00 2001 From: Jesse Williamson Date: Thu, 6 Apr 2017 04:59:37 -0700 Subject: [PATCH] common: Implements simple_spin_t in terms of std::atomic_flag Removes the requirement of machine-specific code in favor of the standard library. See also PR #14337. Signed-off-by: Jesse Williamson --- src/CMakeLists.txt | 1 - src/common/buffer.cc | 11 ++++--- src/common/simple_spin.cc | 63 --------------------------------------- src/common/simple_spin.h | 34 +++++++++++---------- src/test/simple_spin.cc | 26 ++++++++++++++-- 5 files changed, 46 insertions(+), 89 deletions(-) delete mode 100644 src/common/simple_spin.cc diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 245b30c8ba1..06a812082bb 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -474,7 +474,6 @@ set(libcommon_files common/code_environment.cc common/dout.cc common/signal.cc - common/simple_spin.cc common/Thread.cc common/Formatter.cc common/HTMLFormatter.cc diff --git a/src/common/buffer.cc b/src/common/buffer.cc index 56b5770fa01..1a7626c96d8 100644 --- a/src/common/buffer.cc +++ b/src/common/buffer.cc @@ -38,13 +38,14 @@ #include #include +#include #include #define CEPH_BUFFER_ALLOC_UNIT (MIN(CEPH_PAGE_SIZE, 4096)) #define CEPH_BUFFER_APPEND_SIZE (CEPH_BUFFER_ALLOC_UNIT - sizeof(raw_combined)) #ifdef BUFFER_DEBUG -static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER; +static std::atomic_flag buffer_debug_lock = ATOMIC_FLAG_INIT; # define bdout { simple_spin_lock(&buffer_debug_lock); std::cout # define bendl std::endl; simple_spin_unlock(&buffer_debug_lock); } #else @@ -166,16 +167,14 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER; unsigned len; atomic_t nref; - mutable simple_spinlock_t crc_spinlock; + mutable std::atomic_flag crc_spinlock = ATOMIC_FLAG_INIT; map, pair > crc_map; explicit raw(unsigned l) - : data(NULL), len(l), nref(0), - crc_spinlock(SIMPLE_SPINLOCK_INITIALIZER) + : data(NULL), len(l), nref(0) { } raw(char *c, unsigned l) - : data(c), len(l), nref(0), - crc_spinlock(SIMPLE_SPINLOCK_INITIALIZER) + : data(c), len(l), nref(0) { } virtual ~raw() {} diff --git a/src/common/simple_spin.cc b/src/common/simple_spin.cc deleted file mode 100644 index 3e1b3dc048f..00000000000 --- a/src/common/simple_spin.cc +++ /dev/null @@ -1,63 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- -// vim: ts=8 sw=2 smarttab -/* - * Ceph - scalable distributed file system - * - * Copyright (C) 2011 New Dream Network - * - * This is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License version 2.1, as published by the Free Software - * Foundation. See file COPYING. - * - */ - -#include "common/simple_spin.h" - -#include -#include -#include - -namespace { - -#if !defined(__i386__) && !defined(__x86_64__) && !defined(__arm__) && !defined(__aarch64__) && !defined(__powerpc__) && !defined(__ppc__) && !defined(__s390__) && !defined(__s390x__) -static uint32_t bar = 13; -static uint32_t *foo = &bar; -#endif - -void cpu_relax() -{ -#if defined(__i386__) || defined(__x86_64__) - asm volatile("pause"); -#elif defined(__arm__) || defined(__aarch64__) - asm volatile("yield"); -#elif defined(__powerpc__) || defined(__ppc__) - asm volatile("or 27,27,27"); -#elif defined(__s390__) || defined(__s390x__) - asm volatile("": : :"memory"); -#else - for (int i = 0; i < 100000; i++) { - *foo = (*foo * 33) + 17; - } -#endif -} - -} // namespace - -void simple_spin_lock(simple_spinlock_t *lock) -{ - while (1) { - __sync_synchronize(); - uint32_t oldval = *lock; - if (oldval == 0) { - if (__sync_bool_compare_and_swap(lock, 0, 1)) - return; - } - cpu_relax(); - } -} - -void simple_spin_unlock(simple_spinlock_t *lock) -{ - __sync_bool_compare_and_swap(lock, 1, 0); -} diff --git a/src/common/simple_spin.h b/src/common/simple_spin.h index f2a8d78d914..dc86dd0c901 100644 --- a/src/common/simple_spin.h +++ b/src/common/simple_spin.h @@ -15,25 +15,27 @@ #ifndef CEPH_SIMPLE_SPIN_H #define CEPH_SIMPLE_SPIN_H -/* This is a simple spinlock implementation based on atomic compare-and-swap. - * Like all spinlocks, it is intended to protect short, simple critical - * sections. It is signal-safe. Unlike pthread_spin_lock and friends, it has a - * static initializer so you can write: - * - * simple_spinlock_t my_spinlock = SIMPLE_SPINLOCK_INITIALIZER - * - * This allows you to use the lock anywhere you want-- even in global - * constructors. Since simple_spinlock_t is a primitive type, it will start out - * correctly initialized. - */ +#include -#include +inline void simple_spin_lock(std::atomic_flag& lock) +{ + while(lock.test_and_set(std::memory_order_acquire)) + ; +} -typedef uint32_t simple_spinlock_t; +inline void simple_spin_unlock(std::atomic_flag& lock) +{ + lock.clear(std::memory_order_release); +} -#define SIMPLE_SPINLOCK_INITIALIZER 0 +inline void simple_spin_lock(std::atomic_flag *lock) +{ + simple_spin_lock(*lock); +} -void simple_spin_lock(simple_spinlock_t *lock); -void simple_spin_unlock(simple_spinlock_t *lock); +inline void simple_spin_unlock(std::atomic_flag *lock) +{ + simple_spin_unlock(*lock); +} #endif diff --git a/src/test/simple_spin.cc b/src/test/simple_spin.cc index 63e09a453ea..9fd850f7bb2 100644 --- a/src/test/simple_spin.cc +++ b/src/test/simple_spin.cc @@ -2,14 +2,16 @@ #include "common/simple_spin.h" +#include + TEST(SimpleSpin, Test0) { - simple_spinlock_t lock0 = SIMPLE_SPINLOCK_INITIALIZER; + std::atomic_flag lock0 = ATOMIC_FLAG_INIT; simple_spin_lock(&lock0); simple_spin_unlock(&lock0); } -static simple_spinlock_t lock = SIMPLE_SPINLOCK_INITIALIZER; +static std::atomic_flag lock = ATOMIC_FLAG_INIT; static uint32_t counter = 0; static void* mythread(void *v) @@ -24,6 +26,9 @@ static void* mythread(void *v) TEST(SimpleSpin, Test1) { + counter = 0; + const auto n = 2000000U; + int ret; pthread_t thread1; pthread_t thread2; @@ -35,5 +40,20 @@ TEST(SimpleSpin, Test1) ASSERT_EQ(ret, 0); ret = pthread_join(thread2, NULL); ASSERT_EQ(ret, 0); - ASSERT_EQ(counter, 2000000U); + ASSERT_EQ(counter, n); + + + // Should also work with pass-by-reference: + // (Note that we don't care about cross-threading here as-such.) + counter = 0; + ASSERT_EQ(counter, 0); + async(std::launch::async, []() { + for(int i = 0; n != i; ++i) { + simple_spin_lock(lock); + counter++; + simple_spin_unlock(lock); + } + }); + ASSERT_EQ(counter, n); } + -- 2.39.5