From 9b9b965b550c1e92d2da40b92e95d6f03d06a106 Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Fri, 1 Mar 2019 15:52:05 +0100 Subject: [PATCH] Add cache locking functions in trylock variant - Add cache trylock and read trylock functions. - Introduce new error code -OCF_ERR_NO_LOCK. - Change trylock functions in env to return this code in case of lock contention. [ENV CHANGES REQUIRED] Following functions should return 0 on success or -OCF_ERR_NO_LOCK in case of lock contention: - env_mutex_trylock() - env_rwsem_up_read_trylock() - env_rwsem_up_write_trylock() Signed-off-by: Robert Baldyga --- env/posix/ocf_env.h | 23 +++------- inc/ocf_err.h | 15 +++--- inc/ocf_mngt.h | 28 +++++++++++ src/cleaning/cleaning.c | 2 +- src/metadata/metadata.h | 4 +- src/mngt/ocf_mngt_common.c | 46 +++++++++++++------ .../cleaning.c/ocf_cleaner_run_test.c | 2 +- 7 files changed, 78 insertions(+), 42 deletions(-) diff --git a/env/posix/ocf_env.h b/env/posix/ocf_env.h index 0fce943..7c0ff53 100644 --- a/env/posix/ocf_env.h +++ b/env/posix/ocf_env.h @@ -35,6 +35,7 @@ #include "ocf_env_list.h" #include "ocf_env_headers.h" +#include "ocf/ocf_err.h" /* linux sector 512-bytes */ #define ENV_SECTOR_SHIFT 9 @@ -146,9 +147,7 @@ static inline int env_mutex_lock_interruptible(env_mutex *mutex) static inline int env_mutex_trylock(env_mutex *mutex) { - if (pthread_mutex_trylock(&mutex->m) == 0) - return 1; - return 0; + return pthread_mutex_trylock(&mutex->m) ? -OCF_ERR_NO_LOCK : 0; } static inline void env_mutex_unlock(env_mutex *mutex) @@ -158,7 +157,7 @@ static inline void env_mutex_unlock(env_mutex *mutex) static inline int env_mutex_is_locked(env_mutex *mutex) { - if (env_mutex_trylock(mutex)) { + if (env_mutex_trylock(mutex) == 0) { env_mutex_unlock(mutex); return 1; } @@ -223,12 +222,7 @@ static inline void env_rwsem_down_read(env_rwsem *s) static inline int env_rwsem_down_read_trylock(env_rwsem *s) { - int result = pthread_rwlock_tryrdlock(&s->lock); - - if (result == 0) - return 1; - else - return 0; + return pthread_rwlock_tryrdlock(&s->lock) ? -OCF_ERR_NO_LOCK : 0; } static inline void env_rwsem_up_write(env_rwsem *s) @@ -243,17 +237,12 @@ static inline void env_rwsem_down_write(env_rwsem *s) static inline int env_rwsem_down_write_trylock(env_rwsem *s) { - int result = pthread_rwlock_trywrlock(&s->lock); - - if (result == 0) - return 1; - else - return 0; + return pthread_rwlock_trywrlock(&s->lock) ? -OCF_ERR_NO_LOCK : 0; } static inline int env_rwsem_is_locked(env_rwsem *s) { - if (env_rwsem_down_read_trylock(s)) { + if (env_rwsem_down_read_trylock(s) == 0) { env_rwsem_up_read(s); return 0; } diff --git a/inc/ocf_err.h b/inc/ocf_err.h index 180b9bf..2a79c91 100644 --- a/inc/ocf_err.h +++ b/inc/ocf_err.h @@ -18,21 +18,24 @@ typedef enum { /** Invalid input parameter value */ OCF_ERR_INVAL = 1000000, - /** Invalid volume type */ - OCF_ERR_INVAL_VOLUME_TYPE, - /** Operation interrupted */ OCF_ERR_INTR, + /** Out of memory */ + OCF_ERR_NO_MEM, + + /** Lock not acquired */ + OCF_ERR_NO_LOCK, + + /** Invalid volume type */ + OCF_ERR_INVAL_VOLUME_TYPE, + /** Unknown error occurred */ OCF_ERR_UNKNOWN, /*!< To many caches */ OCF_ERR_TOO_MANY_CACHES, - /** Out of memory */ - OCF_ERR_NO_MEM, - /** Not enough RAM to start cache */ OCF_ERR_NO_FREE_RAM, diff --git a/inc/ocf_mngt.h b/inc/ocf_mngt.h index 53b8477..c384f1d 100644 --- a/inc/ocf_mngt.h +++ b/inc/ocf_mngt.h @@ -124,6 +124,34 @@ int ocf_mngt_cache_lock(ocf_cache_t cache); */ int ocf_mngt_cache_read_lock(ocf_cache_t cache); +/** + * @brief Lock cache for management oparations (write lock, exclusive) + * + * @param[in] cache Handle to cache + * + * @retval 0 Cache successfully locked + * @retval -OCF_ERR_CACHE_NOT_EXIST Can not lock cache - cache is already + * stopping + * @retval -OCF_ERR_CACHE_IN_USE Can not lock cache - cache is in use + * @retval -OCF_ERR_NO_LOCK Lock not acquired + */ +int ocf_mngt_cache_trylock(ocf_cache_t cache); + +/** + * @brief Lock cache for read - assures cache config does not change while + * lock is being held, while allowing other users to acquire + * read lock in parallel. + * + * @param[in] cache Handle to cache + * + * @retval 0 Cache successfully locked + * @retval -OCF_ERR_CACHE_NOT_EXIST Can not lock cache - cache is already + * stopping + * @retval -OCF_ERR_CACHE_IN_USE Can not lock cache - cache is in use + * @retval -OCF_ERR_NO_LOCK Lock not acquired + */ +int ocf_mngt_cache_read_trylock(ocf_cache_t cache); + /** * @brief Write-unlock cache * diff --git a/src/cleaning/cleaning.c b/src/cleaning/cleaning.c index dc8496e..5964f81 100644 --- a/src/cleaning/cleaning.c +++ b/src/cleaning/cleaning.c @@ -133,7 +133,7 @@ void ocf_cleaner_run(ocf_cleaner_t cleaner, ocf_queue_t queue) } /* Sleep in case there is management operation in progress. */ - if (env_rwsem_down_write_trylock(&cache->lock) == 0) { + if (env_rwsem_down_write_trylock(&cache->lock)) { cleaner->end(cleaner, SLEEP_TIME_MS); return; } diff --git a/src/metadata/metadata.h b/src/metadata/metadata.h index f79a5d9..53ee0df 100644 --- a/src/metadata/metadata.h +++ b/src/metadata/metadata.h @@ -48,7 +48,7 @@ static inline void ocf_metadata_unlock(struct ocf_cache *cache, int rw) static inline int ocf_metadata_try_lock(struct ocf_cache *cache, int rw) { - int result = -1; + int result = 0; if (rw == OCF_METADATA_WR) { result = env_rwsem_down_write_trylock( @@ -60,7 +60,7 @@ static inline int ocf_metadata_try_lock(struct ocf_cache *cache, int rw) ENV_BUG(); } - if (!result) + if (result) return -1; return 0; diff --git a/src/mngt/ocf_mngt_common.c b/src/mngt/ocf_mngt_common.c index 9f2fd4f..f8f6d31 100644 --- a/src/mngt/ocf_mngt_common.c +++ b/src/mngt/ocf_mngt_common.c @@ -232,21 +232,27 @@ bool ocf_mngt_is_cache_locked(ocf_cache_t cache) return false; } +static void _ocf_mngt_cache_unlock(ocf_cache_t cache, + void (*unlock_fn)(env_rwsem *s)) +{ + unlock_fn(&cache->lock); + ocf_mngt_cache_put(cache); +} + void ocf_mngt_cache_unlock(ocf_cache_t cache) { OCF_CHECK_NULL(cache); - env_rwsem_up_write(&cache->lock); - ocf_mngt_cache_put(cache); + _ocf_mngt_cache_unlock(cache, env_rwsem_up_write); } void ocf_mngt_cache_read_unlock(ocf_cache_t cache) { OCF_CHECK_NULL(cache); - env_rwsem_up_read(&cache->lock); - ocf_mngt_cache_put(cache); + _ocf_mngt_cache_unlock(cache, env_rwsem_up_read); } -int _ocf_mngt_cache_lock(ocf_cache_t cache, bool read) +static int _ocf_mngt_cache_lock(ocf_cache_t cache, int (*lock_fn)(env_rwsem *s), + void (*unlock_fn)(env_rwsem *s)) { int ret; @@ -254,10 +260,7 @@ int _ocf_mngt_cache_lock(ocf_cache_t cache, bool read) env_atomic_inc(&cache->ref_count); env_atomic_inc(&cache->lock_waiter); - if (read) - ret = env_rwsem_down_read_interruptible(&cache->lock); - else - ret = env_rwsem_down_write_interruptible(&cache->lock); + ret = lock_fn(&cache->lock); env_atomic_dec(&cache->lock_waiter); if (ret) { @@ -283,10 +286,7 @@ int _ocf_mngt_cache_lock(ocf_cache_t cache, bool read) return 0; unlock: - if (read) - ocf_mngt_cache_read_unlock(cache); - else - ocf_mngt_cache_unlock(cache); + _ocf_mngt_cache_unlock(cache, unlock_fn); return ret; } @@ -294,13 +294,29 @@ unlock: int ocf_mngt_cache_lock(ocf_cache_t cache) { OCF_CHECK_NULL(cache); - return _ocf_mngt_cache_lock(cache, false); + return _ocf_mngt_cache_lock(cache, env_rwsem_down_write_interruptible, + env_rwsem_up_write); } int ocf_mngt_cache_read_lock(ocf_cache_t cache) { OCF_CHECK_NULL(cache); - return _ocf_mngt_cache_lock(cache, true); + return _ocf_mngt_cache_lock(cache, env_rwsem_down_read_interruptible, + env_rwsem_up_read); +} + +int ocf_mngt_cache_trylock(ocf_cache_t cache) +{ + OCF_CHECK_NULL(cache); + return _ocf_mngt_cache_lock(cache, env_rwsem_down_write_trylock, + env_rwsem_up_write); +} + +int ocf_mngt_cache_read_trylock(ocf_cache_t cache) +{ + OCF_CHECK_NULL(cache); + return _ocf_mngt_cache_lock(cache, env_rwsem_down_read_trylock, + env_rwsem_up_read); } /* if cache is either fully initialized or during recovery */ diff --git a/tests/unit/tests/cleaning/cleaning.c/ocf_cleaner_run_test.c b/tests/unit/tests/cleaning/cleaning.c/ocf_cleaner_run_test.c index 72b4613..674f24b 100644 --- a/tests/unit/tests/cleaning/cleaning.c/ocf_cleaner_run_test.c +++ b/tests/unit/tests/cleaning/cleaning.c/ocf_cleaner_run_test.c @@ -247,7 +247,7 @@ static void ocf_cleaner_run_test01(void **state) will_return(__wrap_ocf_mngt_is_cache_locked, 0); expect_function_call(__wrap_env_rwsem_down_write_trylock); - will_return(__wrap_env_rwsem_down_write_trylock, 1); + will_return(__wrap_env_rwsem_down_write_trylock, 0); expect_function_call(__wrap__ocf_cleaner_run_check_dirty_inactive); will_return(__wrap__ocf_cleaner_run_check_dirty_inactive, 0);