Merge pull request #227 from mmichal10/mark-core-added-false

Don't try to remove invalid cores

If valid cache metadata was read, but environment has changed (i.e. number of cache lines has changed) ocf (in error handling path) was trying to close cores which were not opened. It happened due to cores were marked in cache metadata as added, but any cache inserting operation didn't take place.

In this patch 'added' flag in cache metadata was replaced with more meaningful 'valid' - it is set if given core is stored in cache metadata. Moreover, new 'added' flag was added to core run-time metadata and it is set if given core is added to cache.

src/metadata/metadata_hash.c

@@ -640,7 +640,7 @@ static void ocf_metadata_query_cores_end(struct query_cores_context *context,
 		ocf_metadata_hash_query_cores_data_read(ctx,
 				&context->data.core_config,
 				&core_config, sizeof(core_config));
-		if (core_config.added) {
+		if (core_config.valid) {
 			env_bit_set(i, valid_core_bitmap);
 			++core_count;
 		}
@@ -1313,7 +1313,7 @@ static void ocf_medatata_hash_load_superblock_post(ocf_pipeline_t pipeline,
 	ctrl = (struct ocf_metadata_hash_ctrl *)cache->metadata.iface_priv;
 	sb_config = METADATA_MEM_POOL(ctrl, metadata_segment_sb_config);
 
-	for_each_core(cache, core, core_id) {
+	for_each_core_metadata(cache, core, core_id) {
 		muuid = ocf_metadata_get_core_uuid(cache, core_id);
 		uuid.data = muuid->data;
 		uuid.size = muuid->size;
@@ -1473,7 +1473,7 @@ static void ocf_medatata_hash_flush_superblock_prepare(ocf_pipeline_t pipeline,
 	ocf_core_id_t core_id;
 
 	/* Synchronize core objects types */
-	for_each_core(cache, core, core_id) {
+	for_each_core_metadata(cache, core, core_id) {
 		core->conf_meta->type = ocf_ctx_get_volume_type_id(
 				cache->owner, core->volume.type);
 	}

src/mngt/ocf_mngt_cache.c

@@ -377,18 +377,11 @@ static int _ocf_mngt_init_instance_add_cores(
 
 	OCF_ASSERT_PLUGGED(cache);
 
-	if (cache->conf_meta->cachelines !=
-	    ocf_metadata_get_cachelines_count(cache)) {
-		ocf_cache_log(cache, log_err,
-				"ERROR: Cache device size mismatch!\n");
-		return -OCF_ERR_START_CACHE_FAIL;
-	}
-
-	/* Count value will be re-calculated on the basis of 'added' flag */
+	/* Count value will be re-calculated on the basis of 'valid' flag */
 	cache->conf_meta->core_count = 0;
 
-	/* Check in metadata which cores were added into cache */
-	for_each_core(cache, core, core_id) {
+	/* Check in metadata which cores were saved in cache metadata */
+	for_each_core_metadata(cache, core, core_id) {
 		ocf_volume_t tvolume = NULL;
 
 		if (!core->volume.type)
@@ -423,6 +416,7 @@ static int _ocf_mngt_init_instance_add_cores(
 		}
 
 		env_bit_set(core_id, cache->conf_meta->valid_core_bitmap);
+		core->added = true;
 		cache->conf_meta->core_count++;
 		core->volume.cache = cache;
 
@@ -1303,6 +1297,14 @@ static void _ocf_mngt_attach_load_superblock_complete(void *priv, int error)
 	struct ocf_cache_attach_context *context = priv;
 	ocf_cache_t cache = context->cache;
 
+	if (cache->conf_meta->cachelines !=
+	    ocf_metadata_get_cachelines_count(cache)) {
+		ocf_cache_log(cache, log_err,
+				"ERROR: Cache device size mismatch!\n");
+		OCF_PL_FINISH_RET(context->pipeline,
+				-OCF_ERR_START_CACHE_FAIL);
+	}
+
 	if (error) {
 		ocf_cache_log(cache, log_err,
 				"ERROR: Cannot load cache state\n");
@@ -1903,7 +1905,10 @@ static void _ocf_mngt_cache_stop_remove_cores(ocf_cache_t cache, bool attached)
 	int no = cache->conf_meta->core_count;
 
 	/* All exported objects removed, cleaning up rest. */
-	for_each_core(cache, core, core_id) {
+	for_each_core_all(cache, core, core_id) {
+		if (!env_bit_test(core_id, cache->conf_meta->valid_core_bitmap))
+			continue;
+
 		cache_mngt_core_remove_from_cache(core);
 		if (attached)
 			cache_mngt_core_remove_from_cleaning_pol(core);
@@ -2362,7 +2367,7 @@ static void ocf_mngt_cache_detach_update_metadata(ocf_pipeline_t pipeline,
 	int no = cache->conf_meta->core_count;
 
 	/* remove cacheline metadata and cleaning policy meta for all cores */
-	for_each_core(cache, core, core_id) {
+	for_each_core_metadata(cache, core, core_id) {
 		cache_mngt_core_deinit_attached_meta(core);
 		cache_mngt_core_remove_from_cleaning_pol(core);
 		if (--no == 0)

src/mngt/ocf_mngt_common.c

@@ -99,7 +99,7 @@ void cache_mngt_core_remove_from_meta(ocf_core_t core)
 	OCF_METADATA_LOCK_WR();
 
 	/* In metadata mark data this core was removed from cache */
-	core->conf_meta->added = false;
+	core->conf_meta->valid = false;
 
 	/* Clear UUID of core */
 	ocf_mngt_core_clear_uuid_metadata(core);
@@ -116,6 +116,7 @@ void cache_mngt_core_remove_from_cache(ocf_core_t core)
 
 	env_free(core->counters);
 	core->counters = NULL;
+	core->added = false;
 	env_bit_clear(core_id, cache->conf_meta->valid_core_bitmap);
 
 	if (!core->opened && --cache->ocf_core_inactive_count == 0)

src/mngt/ocf_mngt_core.c

@@ -115,7 +115,8 @@ static void _ocf_mngt_cache_add_core_handle_error(
 	if (context->flags.counters_allocated) {
 		env_bit_clear(core_id,
 				cache->conf_meta->valid_core_bitmap);
-		core->conf_meta->added = false;
+		core->conf_meta->valid = false;
+		core->added = false;
 		core->opened = false;
 
 		env_free(core->counters);
@@ -419,7 +420,8 @@ static void ocf_mngt_cache_add_core_insert(ocf_pipeline_t pipeline,
 
 	/* In metadata mark data this core was added into cache */
 	env_bit_set(core_id, cache->conf_meta->valid_core_bitmap);
-	core->conf_meta->added = true;
+	core->conf_meta->valid = true;
+	core->added = true;
 	core->opened = true;
 
 	/* Set default cache parameters for sequential */

src/ocf_cache_priv.h

@@ -179,11 +179,15 @@ static inline ocf_core_t ocf_cache_get_core(ocf_cache_t cache,
 }
 
 #define for_each_core_all(_cache, _core, _id) \
-	for (_id = 0; _core = &cache->core[_id], _id < OCF_CORE_MAX; _id++)
+	for (_id = 0; _core = &_cache->core[_id], _id < OCF_CORE_MAX; _id++)
 
 #define for_each_core(_cache, _core, _id) \
 	for_each_core_all(_cache, _core, _id) \
-		if (_core->conf_meta->added)
+		if (_core->added)
+
+#define for_each_core_metadata(_cache, _core, _id) \
+	for_each_core_all(_cache, _core, _id) \
+		if (_core->conf_meta->valid)
 
 #define ocf_cache_log_prefix(cache, lvl, prefix, fmt, ...) \
 	ocf_log_prefix(ocf_cache_get_ctx(cache), lvl, "%s" prefix, \

src/ocf_core_priv.h

@@ -30,8 +30,8 @@ struct ocf_core_meta_config {
 
 	uint8_t type;
 
-	/* This bit means that object was added into cache */
-	uint32_t added : 1;
+	/* This bit means that object was saved in cache metadata */
+	uint32_t valid : 1;
 
 	/* Core sequence number used to correlate cache lines with cores
 	 * when recovering from atomic device */
@@ -87,6 +87,8 @@ struct ocf_core {
 
 	/* This bit means that object is open */
 	uint32_t opened : 1;
+	/* This bit means that core is added into cache */
+	uint32_t added : 1;
 
 	struct ocf_counters_core *counters;
 };