From 5ca9287b53605b47049e8a3ff4043702dda0be90 Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Thu, 19 Mar 2020 07:21:48 -0400 Subject: [PATCH 1/2] Refactor core removing function Signed-off-by: Michal Mielewczyk --- modules/cas_cache/layer_cache_management.c | 140 ++++++++++----------- 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/modules/cas_cache/layer_cache_management.c b/modules/cas_cache/layer_cache_management.c index 9e61b49..c28fbad 100644 --- a/modules/cas_cache/layer_cache_management.c +++ b/modules/cas_cache/layer_cache_management.c @@ -1132,14 +1132,29 @@ error_affter_lock: return result; } -/* Flush cache and destroy exported object */ -int _cache_mngt_remove_core_prepare(ocf_cache_t cache, ocf_core_t core, - struct kcas_remove_core *cmd, bool destroy) +static int _cache_mngt_remove_core_flush(ocf_cache_t cache, + struct kcas_remove_core *cmd) { 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); @@ -1150,41 +1165,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 (core_active) { - /* 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 (!cmd->force_no_flush) + result = _cache_mngt_core_flush_uninterruptible(core); - if (flush_result) - result = destroy ? -KCAS_ERR_REMOVED_DIRTY : flush_result; - - return result; + return result ? -KCAS_ERR_REMOVED_DIRTY : 0; } -/**************************************************************** - * 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; @@ -1196,7 +1224,7 @@ 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; @@ -1205,34 +1233,10 @@ int cache_mngt_remove_core_from_cache(struct kcas_remove_core *cmd) if (result) return result; - if (!cmd->force_no_flush) { - /* First check state and flush data (if requested by user) - under read lock */ - /* 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 = _cache_mngt_remove_core_flush(cache, cmd); + if (result) + 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; @@ -1248,18 +1252,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); - if (result == -KCAS_ERR_REMOVED_DIRTY) { - flush_result = result; - result = 0; - } else if (result) { + prepare_result = _cache_mngt_remove_core_prepare(cache, core, cmd); + if (prepare_result && prepare_result != -KCAS_ERR_REMOVED_DIRTY) 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 { @@ -1267,7 +1267,7 @@ 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); @@ -1277,8 +1277,8 @@ int cache_mngt_remove_core_from_cache(struct kcas_remove_core *cmd) mark_core_id_free(cache_priv->core_id_bitmap, cmd->core_id); } - if (!result && flush_result) - result = flush_result; + if (!result && prepare_result) + result = prepare_result; unlock: ocf_mngt_cache_unlock(cache); From 91c8c02860a27c49086284bf36a223e7d9b0cf3d Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Thu, 19 Mar 2020 13:32:43 -0400 Subject: [PATCH 2/2] Simplify functions managing core ids Signed-off-by: Michal Mielewczyk --- modules/cas_cache/layer_cache_management.c | 26 +++++++++++++--------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/modules/cas_cache/layer_cache_management.c b/modules/cas_cache/layer_cache_management.c index c28fbad..1317e3e 100644 --- a/modules/cas_cache/layer_cache_management.c +++ b/modules/cas_cache/layer_cache_management.c @@ -538,13 +538,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); } @@ -998,16 +1008,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; } @@ -1044,7 +1052,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); @@ -1105,8 +1112,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_priv->core_id_bitmap, core_id); + mark_core_id_used(cache, core_id); ocf_mngt_cache_unlock(cache); ocf_mngt_cache_put(cache); @@ -1227,7 +1233,6 @@ int cache_mngt_remove_core_from_cache(struct kcas_remove_core *cmd) 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) @@ -1273,8 +1278,7 @@ int cache_mngt_remove_core_from_cache(struct kcas_remove_core *cmd) wait_for_completion(&context.cmpl); if (!result && !cmd->detach) { - cache_priv = ocf_cache_get_priv(cache); - mark_core_id_free(cache_priv->core_id_bitmap, cmd->core_id); + mark_core_id_free(cache, cmd->core_id); } if (!result && prepare_result)