From d550c8f4efab926fbe965b97fee4c5c8c75a5dab Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Wed, 30 Mar 2022 19:52:52 +0200 Subject: [PATCH 1/5] Fix minor coding style issues Signed-off-by: Robert Baldyga --- src/mngt/ocf_mngt_cache.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mngt/ocf_mngt_cache.c b/src/mngt/ocf_mngt_cache.c index b5ee5a1..2aae9f1 100644 --- a/src/mngt/ocf_mngt_cache.c +++ b/src/mngt/ocf_mngt_cache.c @@ -433,8 +433,8 @@ static void _ocf_mngt_load_add_cores(ocf_pipeline_t pipeline, ocf_core_log(core, log_err, "Size of core volume doesn't match with" " the size stored in cache metadata!"); - error = -OCF_ERR_CORE_SIZE_MISMATCH; - goto err; + error = -OCF_ERR_CORE_SIZE_MISMATCH; + goto err; } hd_lines = ocf_bytes_2_lines(cache, length); From 9c751dd2b8ae9ae6ee2ea239b6f68ebe6c04ed00 Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Wed, 30 Mar 2022 19:52:35 +0200 Subject: [PATCH 2/5] Manage valid_core_bitmap properly Set bit only on core addition and clean it on core removal. This allows to avoid conf metadata modification in load / standby load paths, which effectively prevents issues with metadata mismatch during consequent standby activate attempts after initial activate failure. Previously the first attempt changed the metadata, so on comparison with metadata on drive failed on any following attempt, leading to inability to activate the cache. Signed-off-by: Robert Baldyga --- src/mngt/ocf_mngt_cache.c | 11 +++-------- src/mngt/ocf_mngt_common.c | 5 +++-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/mngt/ocf_mngt_cache.c b/src/mngt/ocf_mngt_cache.c index 2aae9f1..d658d04 100644 --- a/src/mngt/ocf_mngt_cache.c +++ b/src/mngt/ocf_mngt_cache.c @@ -312,7 +312,7 @@ static void _ocf_mngt_close_all_uninitialized_cores( int j, i; for (j = cache->conf_meta->core_count, i = 0; j > 0; ++i) { - if (!env_bit_test(i, cache->conf_meta->valid_core_bitmap)) + if (!cache->core[i].added) continue; volume = &(cache->core[i].volume); @@ -326,8 +326,7 @@ static void _ocf_mngt_close_all_uninitialized_cores( env_free(cache->core[i].counters); cache->core[i].counters = NULL; - - env_bit_clear(i, cache->conf_meta->valid_core_bitmap); + cache->core[i].added = false; } cache->conf_meta->core_count = 0; @@ -401,7 +400,6 @@ static void _ocf_mngt_load_add_cores(ocf_pipeline_t pipeline, } } - env_bit_set(core_id, cache->conf_meta->valid_core_bitmap); core->added = true; cache->conf_meta->core_count++; core->volume.cache = cache; @@ -1963,10 +1961,7 @@ 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_all(cache, core, core_id) { - if (!env_bit_test(core_id, cache->conf_meta->valid_core_bitmap)) - continue; - + for_each_core(cache, core, core_id) { cache_mngt_core_remove_from_cache(core); if (attached) cache_mngt_core_remove_from_cleaning_pol(core); diff --git a/src/mngt/ocf_mngt_common.c b/src/mngt/ocf_mngt_common.c index 3668e65..d779779 100644 --- a/src/mngt/ocf_mngt_common.c +++ b/src/mngt/ocf_mngt_common.c @@ -106,6 +106,7 @@ void cache_mngt_core_deinit_attached_meta(ocf_core_t core) void cache_mngt_core_remove_from_meta(ocf_core_t core) { ocf_cache_t cache = ocf_core_get_cache(core); + ocf_core_id_t core_id = ocf_core_get_id(core); ocf_metadata_start_exclusive_access(&cache->metadata.lock); @@ -116,6 +117,8 @@ void cache_mngt_core_remove_from_meta(ocf_core_t core) ocf_mngt_core_clear_uuid_metadata(core); core->conf_meta->seq_no = OCF_SEQ_NO_INVALID; + env_bit_clear(core_id, cache->conf_meta->valid_core_bitmap); + ocf_metadata_end_exclusive_access(&cache->metadata.lock); } @@ -123,13 +126,11 @@ void cache_mngt_core_remove_from_meta(ocf_core_t core) void cache_mngt_core_remove_from_cache(ocf_core_t core) { ocf_cache_t cache = ocf_core_get_cache(core); - ocf_core_id_t core_id = ocf_core_get_id(core); ocf_core_seq_cutoff_deinit(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) env_bit_clear(ocf_cache_state_incomplete, &cache->cache_state); From 25434cb8d1b845f4a2a4857736109a023f49cc5a Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Wed, 30 Mar 2022 22:42:10 +0200 Subject: [PATCH 3/5] Explicitly validate valid_core_bitmap consistency Signed-off-by: Robert Baldyga --- src/metadata/metadata_superblock.c | 37 ++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/metadata/metadata_superblock.c b/src/metadata/metadata_superblock.c index 7a5760c..23e94ae 100644 --- a/src/metadata/metadata_superblock.c +++ b/src/metadata/metadata_superblock.c @@ -206,6 +206,41 @@ static void _ocf_metadata_validate_superblock(ocf_pipeline_t pipeline, ocf_pipeline_next(pipeline); } +static void _ocf_metadata_validate_core_config(ocf_pipeline_t pipeline, + void *priv, ocf_pipeline_arg_t arg) +{ + struct ocf_metadata_context *context = priv; + ocf_ctx_t ctx = context->cache->owner; + struct ocf_metadata_ctrl *ctrl; + struct ocf_superblock_config *superblock; + struct ocf_core_meta_config *core_config; + ocf_core_id_t core_id; + bool valid_in_bitmap; + + ctrl = (struct ocf_metadata_ctrl *)context->ctrl; + superblock = METADATA_MEM_POOL(ctrl, metadata_segment_sb_config); + core_config = METADATA_MEM_POOL(ctrl, metadata_segment_core_config); + + for (core_id = 0; core_id < OCF_CORE_MAX; core_id++) { + valid_in_bitmap = env_bit_test(core_id, + superblock->valid_core_bitmap); + + if (valid_in_bitmap && !core_config[core_id].valid) { + ocf_log(ctx, log_err, "Core is marked as valid in " + "bitmap but not in core metadata!\n"); + OCF_PL_FINISH_RET(pipeline, -OCF_ERR_INVAL); + } + + if (core_config[core_id].valid && !valid_in_bitmap) { + ocf_log(ctx, log_err, "Core is marked as valid in " + "core metadata but not in bitmap!\n"); + OCF_PL_FINISH_RET(pipeline, -OCF_ERR_INVAL); + } + } + + ocf_pipeline_next(pipeline); +} + static void ocf_metadata_load_sb_restore( struct ocf_metadata_context *context) { @@ -282,6 +317,7 @@ struct ocf_pipeline_properties ocf_metadata_load_sb_pipeline_props = { ocf_metadata_load_sb_load_segment_args), OCF_PL_STEP_FOREACH(ocf_metadata_check_crc, ocf_metadata_load_sb_load_segment_args), + OCF_PL_STEP(_ocf_metadata_validate_core_config), OCF_PL_STEP_TERMINATOR(), }, }; @@ -347,6 +383,7 @@ struct ocf_pipeline_properties ocf_metadata_load_sb_recov_pipeline_props = { ocf_metadata_load_sb_recov_load_segment_args), OCF_PL_STEP_FOREACH(ocf_metadata_check_crc, ocf_metadata_load_sb_recov_load_segment_args), + OCF_PL_STEP(_ocf_metadata_validate_core_config), OCF_PL_STEP_TERMINATOR(), }, }; From 9ebb0de878765887c46c58e6c4cf339722f3dd6e Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Thu, 31 Mar 2022 10:00:24 +0200 Subject: [PATCH 4/5] Do not modify core_count on cache load / activate Increment core_count only on core addition, and decrement it only on core removal. Signed-off-by: Robert Baldyga --- src/mngt/ocf_mngt_cache.c | 9 --------- src/mngt/ocf_mngt_common.c | 3 +-- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/mngt/ocf_mngt_cache.c b/src/mngt/ocf_mngt_cache.c index d658d04..2add57e 100644 --- a/src/mngt/ocf_mngt_cache.c +++ b/src/mngt/ocf_mngt_cache.c @@ -328,8 +328,6 @@ static void _ocf_mngt_close_all_uninitialized_cores( cache->core[i].counters = NULL; cache->core[i].added = false; } - - cache->conf_meta->core_count = 0; } /** @@ -350,9 +348,6 @@ static void _ocf_mngt_load_add_cores(ocf_pipeline_t pipeline, OCF_ASSERT_PLUGGED(cache); - /* Count value will be re-calculated on the basis of 'valid' flag */ - cache->conf_meta->core_count = 0; - /* Check in metadata which cores were saved in cache metadata */ for_each_core_metadata(cache, core, core_id) { struct ocf_metadata_uuid *muuid; @@ -401,7 +396,6 @@ static void _ocf_mngt_load_add_cores(ocf_pipeline_t pipeline, } core->added = true; - cache->conf_meta->core_count++; core->volume.cache = cache; if (ocf_mngt_core_init_front_volume(core)) @@ -1969,7 +1963,6 @@ static void _ocf_mngt_cache_stop_remove_cores(ocf_cache_t cache, bool attached) if (--no == 0) break; } - ENV_BUG_ON(cache->conf_meta->core_count != 0); } static void ocf_mngt_cache_stop_remove_cores(ocf_pipeline_t pipeline, @@ -2995,8 +2988,6 @@ static void _ocf_mngt_cache_unplug(ocf_cache_t cache, bool stop, struct _ocf_mngt_cache_unplug_context *context, _ocf_mngt_cache_unplug_end_t cmpl, void *priv) { - ENV_BUG_ON(stop && cache->conf_meta->core_count != 0); - context->cmpl = cmpl; context->priv = priv; context->cache = cache; diff --git a/src/mngt/ocf_mngt_common.c b/src/mngt/ocf_mngt_common.c index d779779..5305521 100644 --- a/src/mngt/ocf_mngt_common.c +++ b/src/mngt/ocf_mngt_common.c @@ -118,6 +118,7 @@ void cache_mngt_core_remove_from_meta(ocf_core_t core) core->conf_meta->seq_no = OCF_SEQ_NO_INVALID; env_bit_clear(core_id, cache->conf_meta->valid_core_bitmap); + cache->conf_meta->core_count--; ocf_metadata_end_exclusive_access(&cache->metadata.lock); } @@ -134,8 +135,6 @@ void cache_mngt_core_remove_from_cache(ocf_core_t core) if (!core->opened && --cache->ocf_core_inactive_count == 0) env_bit_clear(ocf_cache_state_incomplete, &cache->cache_state); - - cache->conf_meta->core_count--; } void ocf_mngt_cache_put(ocf_cache_t cache) From 09b73461b4f72df6439fa195dc9364c698ff8b66 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 31 Mar 2022 13:16:36 +0200 Subject: [PATCH 5/5] Always modify valid_core_map together with core_count .. to assure that superblock config state on drive is consistent Signed-off-by: Adam Rutkowski --- src/mngt/ocf_mngt_core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mngt/ocf_mngt_core.c b/src/mngt/ocf_mngt_core.c index 4490bc9..8f22dd2 100644 --- a/src/mngt/ocf_mngt_core.c +++ b/src/mngt/ocf_mngt_core.c @@ -134,6 +134,7 @@ 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); + cache->conf_meta->core_count--; core->conf_meta->valid = false; core->added = false; core->opened = false; @@ -356,9 +357,6 @@ static void _ocf_mngt_cache_add_core_flush_sb_complete(void *priv, int error) if (error) OCF_PL_FINISH_RET(context->pipeline, -OCF_ERR_WRITE_CACHE); - /* Increase value of added cores */ - context->cache->conf_meta->core_count++; - ocf_pipeline_next(context->pipeline); } @@ -463,6 +461,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); + context->cache->conf_meta->core_count++; + core->conf_meta->valid = true; core->added = true; core->opened = true;