Merge pull request #362 from mmichal10/fix-core-removing

Fix core removing

modules/cas_cache/layer_cache_management.c

@@ -559,13 +559,23 @@ static uint16_t find_free_core_id(uint64_t *bitmap)
 	return ret;
 }
 
-static void mark_core_id_used(uint64_t *bitmap, uint16_t core_id)
+static void mark_core_id_used(ocf_cache_t cache, uint16_t core_id)
 {
+	uint64_t *bitmap;
+	struct cache_priv *cache_priv = ocf_cache_get_priv(cache);
+
+	bitmap = cache_priv->core_id_bitmap;
+
 	set_bit(core_id, (unsigned long *)bitmap);
 }
 
-static void mark_core_id_free(uint64_t *bitmap, uint16_t core_id)
+static void mark_core_id_free(ocf_cache_t cache, uint16_t core_id)
 {
+	uint64_t *bitmap;
+	struct cache_priv *cache_priv = ocf_cache_get_priv(cache);
+
+	bitmap = cache_priv->core_id_bitmap;
+
 	clear_bit(core_id, (unsigned long *)bitmap);
 }
 
@@ -1019,16 +1029,14 @@ static void _cache_mngt_log_core_device_path(ocf_core_t core)
 
 static int _cache_mngt_core_device_loaded_visitor(ocf_core_t core, void *cntx)
 {
-	struct cache_priv *cache_priv;
 	uint16_t core_id = OCF_CORE_ID_INVALID;
 	ocf_cache_t cache = ocf_core_get_cache(core);
-	cache_priv = ocf_cache_get_priv(cache);
 
 	_cache_mngt_log_core_device_path(core);
 
 	core_id_from_name(&core_id, ocf_core_get_name(core));
 
-	mark_core_id_used(cache_priv->core_id_bitmap, core_id);
+	mark_core_id_used(cache, core_id);
 
 	return 0;
 }
@@ -1065,7 +1073,6 @@ int cache_mngt_add_core_to_cache(const char *cache_name, size_t name_len,
 	ocf_core_t core;
 	ocf_core_id_t core_id;
 	int result, remove_core_result;
-	struct cache_priv *cache_priv;
 
 	result = ocf_mngt_cache_get_by_name(cas_ctx, cache_name,
 					name_len, &cache);
@@ -1126,8 +1133,7 @@ int cache_mngt_add_core_to_cache(const char *cache_name, size_t name_len,
 	if (result)
 		goto error_after_create_exported_object;
 
-	cache_priv = ocf_cache_get_priv(cache);
+	mark_core_id_used(cache, core_id);
-	mark_core_id_used(cache_priv->core_id_bitmap, core_id);
 
 	ocf_mngt_cache_unlock(cache);
 	ocf_mngt_cache_put(cache);
@@ -1153,14 +1159,29 @@ error_affter_lock:
 	return result;
 }
 
-/* Flush cache and destroy exported object */
+static int _cache_mngt_remove_core_flush(ocf_cache_t cache,
-int _cache_mngt_remove_core_prepare(ocf_cache_t cache, ocf_core_t core,
+		struct kcas_remove_core *cmd)
-		struct kcas_remove_core *cmd, bool destroy)
 {
 	int result = 0;
-	int flush_result = 0;
+	ocf_core_t core;
 	bool core_active;
-	bool flush_interruptible = !destroy;
+
+	if (cmd->force_no_flush)
+		return 0;
+
+	/* Getting cache for the second time is workaround to make flush error
+	   handling easier and avoid dealing with synchronizing issues */
+	result = ocf_mngt_cache_get(cache);
+	if (result)
+		return result;
+
+	result = _cache_mngt_read_lock_sync(cache);
+	if (result)
+		goto put;
+
+	result = get_core_by_id(cache, cmd->core_id, &core);
+	if (result < 0)
+		goto unlock;
 
 	core_active = (ocf_core_get_state(core) == ocf_core_state_active);
 
@@ -1171,41 +1192,54 @@ int _cache_mngt_remove_core_prepare(ocf_cache_t cache, ocf_core_t core,
 		return -OCF_ERR_CORE_IN_INACTIVE_STATE;
 	}
 
-	if (core_active && destroy) {
+	if (core_active) {
+		return _cache_mngt_core_flush_sync(core,
+				true, _cache_read_unlock_put_cmpl);
+	} else if (!ocf_mngt_core_is_dirty(core)) {
+		result = 0;
+		goto unlock;
+	} else {
+		printk(KERN_WARNING OCF_PREFIX_SHORT
+				"Cannot remove dirty inactive core "
+				"without force option\n");
+		result = -OCF_ERR_CORE_IN_INACTIVE_STATE;
+		goto unlock;
+	}
+
+unlock:
+	ocf_mngt_cache_read_unlock(cache);
+put:
+	ocf_mngt_cache_put(cache);
+	return result;
+}
+
+static int _cache_mngt_remove_core_prepare(ocf_cache_t cache, ocf_core_t core,
+		struct kcas_remove_core *cmd)
+{
+	int result = 0;
+	bool core_active;
+
+	core_active = ocf_core_get_state(core) == ocf_core_state_active;
+
+	if (cmd->detach && !core_active) {
+		printk(KERN_WARNING OCF_PREFIX_SHORT
+				"Cannot detach core which "
+				"is already inactive!\n");
+		return -OCF_ERR_CORE_IN_INACTIVE_STATE;
+	}
+
+	if (core_active) {
 		result = block_dev_destroy_exported_object(core);
 		if (result)
 			return result;
 	}
 
-	if (!cmd->force_no_flush) {
+	if (!cmd->force_no_flush)
-		if (core_active) {
+		result = _cache_mngt_core_flush_uninterruptible(core);
-			/* Flush core */
-			if (flush_interruptible)
-				flush_result = _cache_mngt_core_flush_sync(core,
-						flush_interruptible, _cache_read_unlock_put_cmpl);
-			else
-				flush_result = _cache_mngt_core_flush_uninterruptible(core);
-		} else if (!ocf_mngt_core_is_dirty(core)) {
-			/* Clean core is always "flushed" */
-			flush_result = 0;
-		} else {
-			printk(KERN_WARNING OCF_PREFIX_SHORT
-					"Cannot remove dirty inactive core "
-					"without force option\n");
-			return -OCF_ERR_CORE_IN_INACTIVE_STATE;
-		}
-	}
 
-	if (flush_result)
+	return result ? -KCAS_ERR_REMOVED_DIRTY : 0;
-		result = destroy ? -KCAS_ERR_REMOVED_DIRTY : flush_result;
-
-	return result;
 }
 
-/****************************************************************
- * Function for removing a CORE object from the cache instance
- ****************************************************************/
-
 static void _cache_mngt_remove_core_complete(void *priv, int error)
 {
 	struct _cache_mngt_sync_context *context = priv;
@@ -1217,43 +1251,18 @@ static void _cache_mngt_remove_core_complete(void *priv, int error)
 int cache_mngt_remove_core_from_cache(struct kcas_remove_core *cmd)
 {
 	struct _cache_mngt_sync_context context;
-	int result, flush_result = 0;
+	int result, prepare_result = 0;
 	ocf_cache_t cache;
 	ocf_core_t core;
-	struct cache_priv *cache_priv;
 
 	result = mngt_get_cache_by_id(cas_ctx, cmd->cache_id, &cache);
 	if (result)
 		return result;
 
-	if (!cmd->force_no_flush) {
+	result = _cache_mngt_remove_core_flush(cache, cmd);
-		/* First check state and flush data (if requested by user)
+	if (result)
-		   under read lock */
+		goto put;
-		/* Getting cache twice is workaround to make flush error handling easier
-		   and avoid dealing with synchronizing issues */
-		result = ocf_mngt_cache_get(cache);
-		if (result)
-			goto put;
-		result = _cache_mngt_read_lock_sync(cache);
-		if (result) {
-			ocf_mngt_cache_put(cache);
-			goto put;
-		}
 
-		result = get_core_by_id(cache, cmd->core_id, &core);
-		if (result < 0) {
-			ocf_mngt_cache_unlock(cache);
-			ocf_mngt_cache_put(cache);
-			goto put;
-		}
-
-		result = _cache_mngt_remove_core_prepare(cache, core, cmd,
-				false);
-		if (result)
-			goto put;
-	}
-
-	/* Acquire write lock */
 	result = _cache_mngt_lock_sync(cache);
 	if (result)
 		goto put;
@@ -1269,18 +1278,14 @@ int cache_mngt_remove_core_from_cache(struct kcas_remove_core *cmd)
 	 * destroyed, instead of trying rolling this back we rather detach core
 	 * and then inform user about error.
 	 */
-	result = _cache_mngt_remove_core_prepare(cache, core, cmd, true);
+	prepare_result = _cache_mngt_remove_core_prepare(cache, core, cmd);
-	if (result == -KCAS_ERR_REMOVED_DIRTY) {
+	if (prepare_result && prepare_result != -KCAS_ERR_REMOVED_DIRTY)
-		flush_result = result;
-		result = 0;
-	} else if (result) {
 		goto unlock;
-	}
 
 	init_completion(&context.cmpl);
 	context.result = &result;
 
-	if (cmd->detach || flush_result) {
+	if (cmd->detach || prepare_result == -KCAS_ERR_REMOVED_DIRTY) {
 		ocf_mngt_cache_detach_core(core,
 				_cache_mngt_remove_core_complete, &context);
 	} else {
@@ -1288,18 +1293,17 @@ int cache_mngt_remove_core_from_cache(struct kcas_remove_core *cmd)
 				_cache_mngt_remove_core_complete, &context);
 	}
 
-	if (!cmd->force_no_flush && !flush_result)
+	if (!cmd->force_no_flush && !prepare_result)
 		BUG_ON(ocf_mngt_core_is_dirty(core));
 
 	wait_for_completion(&context.cmpl);
 
 	if (!result && !cmd->detach) {
-		cache_priv = ocf_cache_get_priv(cache);
+		mark_core_id_free(cache, cmd->core_id);
-		mark_core_id_free(cache_priv->core_id_bitmap, cmd->core_id);
 	}
 
-	if (!result && flush_result)
+	if (!result && prepare_result)
-		result = flush_result;
+		result = prepare_result;
 
 unlock:
 	ocf_mngt_cache_unlock(cache);