From 03c83799b3915dd8e42606a23bcc9b01a5a48bac Mon Sep 17 00:00:00 2001 From: Colin Patrick McCabe Date: Fri, 10 Jun 2011 14:43:28 -0700 Subject: [PATCH] include/atomic cleanup * Don't allow copying of class atomic_t. * Remove common/Spinlock.h because it's unecessary * SimpleMessenger: use atomic var for qlen Signed-off-by: Colin McCabe --- src/Makefile.am | 1 - src/include/Spinlock.h | 127 ------------------------------------- src/include/atomic.h | 57 +++++++++++------ src/msg/SimpleMessenger.cc | 12 +--- src/msg/SimpleMessenger.h | 11 +--- 5 files changed, 42 insertions(+), 166 deletions(-) delete mode 100644 src/include/Spinlock.h diff --git a/src/Makefile.am b/src/Makefile.am index 0455dbfb635ab..32f616492bd04 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -844,7 +844,6 @@ noinst_HEADERS = \ include/Context.h\ include/CompatSet.h\ include/Distribution.h\ - include/Spinlock.h\ include/addr_parsing.h\ include/assert.h\ include/atomic.h\ diff --git a/src/include/Spinlock.h b/src/include/Spinlock.h deleted file mode 100644 index 690c87c157f66..0000000000000 --- a/src/include/Spinlock.h +++ /dev/null @@ -1,127 +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) 2004-2006 Sage Weil - * - * 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. - * - */ - -#ifndef CEPH_SPINLOCK_H -#define CEPH_SPINLOCK_H - -#include -#include "assert.h" - -namespace ceph { - -//#define SPINLOCK_LOCKDEP - -#ifdef SPINLOCK_LOCKDEP -# include "lockdep.h" -#endif - -class Spinlock { -private: - pthread_spinlock_t _s; - int nlock; - - // don't allow copying. - void operator=(Spinlock &M) {} - Spinlock( const Spinlock &M ) {} - -#ifdef SPINLOCK_LOCKDEP - const char *name; - int id; - bool lockdep; - bool backtrace; // gather backtrace on lock acquisition - - void _register() { - if (lockdep && g_lockdep) - id = lockdep_register(name); - } - void _will_lock() { // about to lock - if (lockdep && g_lockdep) - id = lockdep_will_lock(name, id); - } - void _locked() { // just locked - if (lockdep && g_lockdep) - id = lockdep_locked(name, id, backtrace); - } - void _will_unlock() { // about to unlock - if (lockdep && g_lockdep) - id = lockdep_will_unlock(name, id); - } -#else - void _register() {} - void _will_lock() {} // about to lock - void _locked() {} // just locked - void _will_unlock() {} // about to unlock -#endif - -public: - Spinlock(const char *n, bool ld=true, bool bt=false) : - nlock(0) -#ifdef SPINLOCK_LOCKDEP - , name(n), id(-1), lockdep(ld), backtrace(bt) -#endif - { - pthread_spin_init(&_s, 0); - _register(); - } - ~Spinlock() { - assert(nlock == 0); - pthread_spin_destroy(&_s); - } - - bool is_locked() { - return (nlock > 0); - } - - bool try_lock() { - int r = pthread_spin_trylock(&_s); - if (r == 0) { - _locked(); - nlock++; - } - return r == 0; - } - - void lock() { - _will_lock(); - int r = pthread_spin_lock(&_s); - _locked(); - assert(r == 0); - nlock++; - } - - void unlock() { - assert(nlock > 0); - --nlock; - _will_unlock(); - int r = pthread_spin_unlock(&_s); - assert(r == 0); - } - -public: - class Locker { - Spinlock &spinlock; - - public: - Locker(Spinlock& m) : spinlock(m) { - spinlock.lock(); - } - ~Locker() { - spinlock.unlock(); - } - }; -}; - -} - -#endif diff --git a/src/include/atomic.h b/src/include/atomic.h index 28cc7df095183..6fe43b9bf88e7 100644 --- a/src/include/atomic.h +++ b/src/include/atomic.h @@ -20,10 +20,10 @@ # include "acconfig.h" #endif - #ifndef NO_ATOMIC_OPS //libatomic_ops implementation #include + namespace ceph { class atomic_t { AO_t val; @@ -48,49 +48,64 @@ namespace ceph { // at some point. this hack can go away someday... return AO_load_full((AO_t *)&val); } + private: + // forbid copying + atomic_t(const atomic_t &other); + atomic_t &operator=(const atomic_t &rhs); }; } #else /* * crappy slow implementation that uses a pthreads spinlock. */ -#include "include/Spinlock.h" +#include #include "include/assert.h" namespace ceph { class atomic_t { - Spinlock lock; - long nref; + pthread_spin_lock lock; + signed long val; public: - atomic_t(int i=0) : lock("atomic_t::lock", false /* no lockdep */), nref(i) {} - atomic_t(const atomic_t& other); + atomic_t(int i=0) + : val(i) { + pthread_spin_init(&lock); + } + ~atomic_t() { + pthread_spin_destroy(&lock); + } int inc() { - lock.lock(); - int r = ++nref; - lock.unlock(); + pthread_spin_lock(&lock); + int r = ++val; + pthread_spin_unlock(&lock); return r; } int dec() { - lock.lock(); - assert(nref > 0); - int r = --nref; - lock.unlock(); + pthread_spin_lock(&lock); + int r = --val; + pthread_spin_unlock(&lock); return r; } void add(int d) { - lock.lock(); - nref += d; - lock.unlock(); + pthread_spin_lock(&lock); + val += d; + pthread_spin_unlock(&lock); } void sub(int d) { - lock.lock(); - assert(nref >= d); - nref -= d; - lock.unlock(); + pthread_spin_lock(&lock); + val -= d; + pthread_spin_unlock(&lock); } int read() const { - return nref; + signed long ret; + pthread_spin_lock(&lock); + ret = val; + pthread_spin_unlock(&lock); + return ret; } + private: + // forbid copying + atomic_t(const atomic_t &other); + atomic_t &operator=(const atomic_t &rhs); }; } #endif diff --git a/src/msg/SimpleMessenger.cc b/src/msg/SimpleMessenger.cc index 99f300a642bda..046a92c80127d 100644 --- a/src/msg/SimpleMessenger.cc +++ b/src/msg/SimpleMessenger.cc @@ -310,9 +310,7 @@ void SimpleMessenger::dispatch_entry() dispatch_queue.lock.Unlock(); //done with the pipe queue for a while pipe->in_qlen--; - dispatch_queue.qlen_lock.lock(); - dispatch_queue.qlen--; - dispatch_queue.qlen_lock.unlock(); + dispatch_queue.qlen.dec(); pipe->pipe_lock.Unlock(); // done with the pipe's message queue now { @@ -566,9 +564,7 @@ void SimpleMessenger::Pipe::queue_received(Message *m, int priority) // increment queue length counters in_qlen++; - messenger->dispatch_queue.qlen_lock.lock(); - ++messenger->dispatch_queue.qlen; - messenger->dispatch_queue.qlen_lock.unlock(); + messenger->dispatch_queue.qlen.inc(); return; @@ -1378,9 +1374,7 @@ void SimpleMessenger::Pipe::discard_queue() dout(20) << " dequeued pipe " << dendl; // adjust qlen - q.qlen_lock.lock(); - q.qlen -= in_qlen; - q.qlen_lock.unlock(); + q.qlen.sub(in_qlen); for (list::iterator p = sent.begin(); p != sent.end(); p++) { dout(20) << " discard " << *p << dendl; diff --git a/src/msg/SimpleMessenger.h b/src/msg/SimpleMessenger.h index e5f4fdfbb83cd..4bd8c7afbbc9a 100644 --- a/src/msg/SimpleMessenger.h +++ b/src/msg/SimpleMessenger.h @@ -26,7 +26,7 @@ using namespace std; using namespace __gnu_cxx; #include "common/Mutex.h" -#include "include/Spinlock.h" +#include "include/atomic.h" #include "common/Cond.h" #include "common/Thread.h" #include "common/Throttle.h" @@ -368,8 +368,7 @@ private: map > queued_pipes; map::iterator> queued_pipe_iters; - int qlen; - Spinlock qlen_lock; + atomic_t qlen; enum { D_CONNECT, D_BAD_REMOTE_RESET, D_BAD_RESET }; list connect_q; @@ -386,10 +385,7 @@ private: } int get_queue_len() { - qlen_lock.lock(); - int l = qlen; - qlen_lock.unlock(); - return l; + return qlen.read(); } void queue_connect(Connection *con) { @@ -415,7 +411,6 @@ private: lock("SimpleMessenger::DispatchQeueu::lock"), stop(false), qlen(0), - qlen_lock("SimpleMessenger::DispatchQueue::qlen_lock"), local_pipe(NULL) {} ~DispatchQueue() { -- 2.39.5