Merge pull request #67 from robertbaldyga/cache-trylock

Add cache locking functions in trylock variant

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;
 	}

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,
 

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
  *

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;
 	}

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;

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 */

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);