From 92f9c3d5328e16b88a2481c0f46c40950d4e3e69 Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Mon, 26 Jul 2021 14:26:36 +0200 Subject: [PATCH 1/8] UT framework: fix misspelled variable name Signed-off-by: Michal Mielewczyk --- tests/unit/framework/prepare_sources_for_testing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/framework/prepare_sources_for_testing.py b/tests/unit/framework/prepare_sources_for_testing.py index 83ac35d..6652dec 100755 --- a/tests/unit/framework/prepare_sources_for_testing.py +++ b/tests/unit/framework/prepare_sources_for_testing.py @@ -576,7 +576,7 @@ class UnitTestsSourcesGenerator(object): delimiter = "" try: if "{" in ret[-1] or "{" in ret[-2]: - delimter = "{" + delimiter = "{" else: delimiter = ";" except IndexError: From 6dc29ee85e32fbc8712fee1ead69ebe1e75d79cd Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Mon, 26 Jul 2021 13:28:50 +0200 Subject: [PATCH 2/8] Refactor includes Signed-off-by: Michal Mielewczyk --- src/metadata/metadata_segment.h | 1 + src/metadata/metadata_superblock.h | 1 + src/utils/utils_pipeline.h | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/metadata/metadata_segment.h b/src/metadata/metadata_segment.h index 6a3fb88..12470fd 100644 --- a/src/metadata/metadata_segment.h +++ b/src/metadata/metadata_segment.h @@ -6,6 +6,7 @@ #ifndef __METADATA_SEGMENT_OPS_H__ #define __METADATA_SEGMENT_OPS_H__ +#include "../utils/utils_pipeline.h" #include "metadata_raw.h" #include diff --git a/src/metadata/metadata_superblock.h b/src/metadata/metadata_superblock.h index 032dab3..4602b3b 100644 --- a/src/metadata/metadata_superblock.h +++ b/src/metadata/metadata_superblock.h @@ -8,6 +8,7 @@ #include #include "metadata_segment.h" +#include "../promotion/promotion.h" #define CACHE_MAGIC_NUMBER 0x187E1CA6 diff --git a/src/utils/utils_pipeline.h b/src/utils/utils_pipeline.h index fb18ea2..5d4e801 100644 --- a/src/utils/utils_pipeline.h +++ b/src/utils/utils_pipeline.h @@ -6,7 +6,7 @@ #ifndef __UTILS_PIPELINE_H__ #define __UTILS_PIPELINE_H__ -#include "../ocf_cache_priv.h" +#include "ocf/ocf.h" enum ocf_pipeline_step_type { ocf_pipeline_step_single, From 00aa56dd28683b21f7d9efe8a5c0fd59d0dd344a Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Mon, 19 Jul 2021 15:36:23 +0200 Subject: [PATCH 3/8] Wrapper functions for cleaning ops The change should unify access to cleaning policy resources and facilitate synchronization when switching cleaning policies Signed-off-by: Michal Mielewczyk --- src/cleaning/cleaning.c | 36 +----- src/cleaning/cleaning_ops.h | 233 +++++++++++++++++++++++++++++++++++ src/mngt/ocf_mngt_core.c | 1 + src/utils/utils_cache_line.h | 1 + 4 files changed, 236 insertions(+), 35 deletions(-) create mode 100644 src/cleaning/cleaning_ops.h diff --git a/src/cleaning/cleaning.c b/src/cleaning/cleaning.c index 89217e7..6af7dbb 100644 --- a/src/cleaning/cleaning.c +++ b/src/cleaning/cleaning.c @@ -13,41 +13,7 @@ #include "../mngt/ocf_mngt_common.h" #include "../metadata/metadata.h" #include "../ocf_queue_priv.h" - -struct cleaning_policy_ops cleaning_policy_ops[ocf_cleaning_max] = { - [ocf_cleaning_nop] = { - .name = "nop", - .perform_cleaning = cleaning_nop_perform_cleaning, - }, - [ocf_cleaning_alru] = { - .setup = cleaning_policy_alru_setup, - .init_cache_block = cleaning_policy_alru_init_cache_block, - .purge_cache_block = cleaning_policy_alru_purge_cache_block, - .purge_range = cleaning_policy_alru_purge_range, - .set_hot_cache_line = cleaning_policy_alru_set_hot_cache_line, - .initialize = cleaning_policy_alru_initialize, - .deinitialize = cleaning_policy_alru_deinitialize, - .set_cleaning_param = cleaning_policy_alru_set_cleaning_param, - .get_cleaning_param = cleaning_policy_alru_get_cleaning_param, - .perform_cleaning = cleaning_alru_perform_cleaning, - .name = "alru", - }, - [ocf_cleaning_acp] = { - .setup = cleaning_policy_acp_setup, - .init_cache_block = cleaning_policy_acp_init_cache_block, - .purge_cache_block = cleaning_policy_acp_purge_block, - .purge_range = cleaning_policy_acp_purge_range, - .set_hot_cache_line = cleaning_policy_acp_set_hot_cache_line, - .initialize = cleaning_policy_acp_initialize, - .deinitialize = cleaning_policy_acp_deinitialize, - .set_cleaning_param = cleaning_policy_acp_set_cleaning_param, - .get_cleaning_param = cleaning_policy_acp_get_cleaning_param, - .add_core = cleaning_policy_acp_add_core, - .remove_core = cleaning_policy_acp_remove_core, - .perform_cleaning = cleaning_policy_acp_perform_cleaning, - .name = "acp", - }, -}; +#include "cleaning_ops.c" int ocf_start_cleaner(ocf_cache_t cache) { diff --git a/src/cleaning/cleaning_ops.h b/src/cleaning/cleaning_ops.h new file mode 100644 index 0000000..6210d27 --- /dev/null +++ b/src/cleaning/cleaning_ops.h @@ -0,0 +1,233 @@ +/* + * Copyright(c) 2012-2021 Intel Corporation + * SPDX-License-Identifier: BSD-3-Clause-Clear + */ + +#include "alru.h" +#include "nop.h" +#include "acp.h" +#include "../metadata/metadata_superblock.h" +#include "../metadata/metadata_structs.h" +#include "../ocf_cache_priv.h" + +struct cleaning_policy_ops { + void (*setup)(ocf_cache_t cache); + int (*initialize)(ocf_cache_t cache, int init_metadata); + void (*deinitialize)(ocf_cache_t cache); + int (*add_core)(ocf_cache_t cache, ocf_core_id_t core_id); + void (*remove_core)(ocf_cache_t cache, ocf_core_id_t core_id); + void (*init_cache_block)(ocf_cache_t cache, uint32_t cache_line); + void (*purge_cache_block)(ocf_cache_t cache, uint32_t cache_line); + int (*purge_range)(ocf_cache_t cache, int core_id, + uint64_t start_byte, uint64_t end_byte); + void (*set_hot_cache_line)(ocf_cache_t cache, uint32_t cache_line); + int (*set_cleaning_param)(ocf_cache_t cache, uint32_t param_id, + uint32_t param_value); + int (*get_cleaning_param)(ocf_cache_t cache, uint32_t param_id, + uint32_t *param_value); + void (*perform_cleaning)(ocf_cache_t cache, ocf_cleaner_end_t cmpl); + const char *name; +}; + + +static struct cleaning_policy_ops cleaning_policy_ops[ocf_cleaning_max] = { + [ocf_cleaning_nop] = { + .name = "nop", + .perform_cleaning = cleaning_nop_perform_cleaning, + }, + [ocf_cleaning_alru] = { + .setup = cleaning_policy_alru_setup, + .init_cache_block = cleaning_policy_alru_init_cache_block, + .purge_cache_block = cleaning_policy_alru_purge_cache_block, + .purge_range = cleaning_policy_alru_purge_range, + .set_hot_cache_line = cleaning_policy_alru_set_hot_cache_line, + .initialize = cleaning_policy_alru_initialize, + .deinitialize = cleaning_policy_alru_deinitialize, + .set_cleaning_param = cleaning_policy_alru_set_cleaning_param, + .get_cleaning_param = cleaning_policy_alru_get_cleaning_param, + .perform_cleaning = cleaning_alru_perform_cleaning, + .name = "alru", + }, + [ocf_cleaning_acp] = { + .setup = cleaning_policy_acp_setup, + .init_cache_block = cleaning_policy_acp_init_cache_block, + .purge_cache_block = cleaning_policy_acp_purge_block, + .purge_range = cleaning_policy_acp_purge_range, + .set_hot_cache_line = cleaning_policy_acp_set_hot_cache_line, + .initialize = cleaning_policy_acp_initialize, + .deinitialize = cleaning_policy_acp_deinitialize, + .set_cleaning_param = cleaning_policy_acp_set_cleaning_param, + .get_cleaning_param = cleaning_policy_acp_get_cleaning_param, + .add_core = cleaning_policy_acp_add_core, + .remove_core = cleaning_policy_acp_remove_core, + .perform_cleaning = cleaning_policy_acp_perform_cleaning, + .name = "acp", + }, +}; + +static inline void ocf_cleaning_setup(ocf_cache_t cache, ocf_cleaning_t policy) +{ + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].setup)) + return; + + cleaning_policy_ops[policy].setup(cache); +} + +static inline int ocf_cleaning_initialize(ocf_cache_t cache, + ocf_cleaning_t policy, int init_metadata) +{ + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].initialize)) + return 0; + + return cleaning_policy_ops[policy].initialize(cache, init_metadata); +} + +static inline void ocf_cleaning_deinitialize(ocf_cache_t cache) +{ + ocf_cleaning_t policy; + + policy = cache->conf_meta->cleaning_policy_type; + + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].deinitialize)) + return; + + cleaning_policy_ops[policy].deinitialize(cache); +} + +static inline int ocf_cleaning_add_core(ocf_cache_t cache, + ocf_core_id_t core_id) +{ + ocf_cleaning_t policy; + + policy = cache->conf_meta->cleaning_policy_type; + + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].add_core)) + goto unlock; + + return cleaning_policy_ops[policy].add_core(cache, core_id); +} + +static inline void ocf_cleaning_remove_core(ocf_cache_t cache, + ocf_core_id_t core_id) +{ + ocf_cleaning_t policy; + + policy = cache->conf_meta->cleaning_policy_type; + + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].remove_core)) + goto unlock; + + cleaning_policy_ops[policy].remove_core(cache, core_id); +} + +static inline void ocf_cleaning_init_cache_block(ocf_cache_t cache, + ocf_cache_line_t cache_line) +{ + ocf_cleaning_t policy; + + policy = cache->conf_meta->cleaning_policy_type; + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].init_cache_block)) + goto unlock; + + cleaning_policy_ops[policy].init_cache_block(cache, cache_line); +} + +static inline void ocf_cleaning_purge_cache_block(ocf_cache_t cache, + ocf_cache_line_t cache_line) +{ + ocf_cleaning_t policy; + + policy = cache->conf_meta->cleaning_policy_type; + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].purge_cache_block)) + goto unlock; + + cleaning_policy_ops[policy].purge_cache_block(cache, cache_line); +} + +static inline void ocf_cleaning_purge_range(ocf_cache_t cache, + ocf_core_id_t core_id, uint64_t start_byte, uint64_t end_byte) +{ + ocf_cleaning_t policy; + + policy = cache->conf_meta->cleaning_policy_type; + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].purge_range)) + goto unlock; + + cleaning_policy_ops[policy].purge_range(cache, core_id, start_byte, + end_byte); +} + +static inline void ocf_cleaning_set_hot_cache_line(ocf_cache_t cache, + ocf_cache_line_t cache_line) +{ + ocf_cleaning_t policy; + + policy = cache->conf_meta->cleaning_policy_type; + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].set_hot_cache_line)) + goto unlock; + + cleaning_policy_ops[policy].set_hot_cache_line(cache, cache_line); +} + +static inline int ocf_cleaning_set_param(ocf_cache_t cache, + ocf_cleaning_t policy, uint32_t param_id, uint32_t param_value) +{ + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (!cleaning_policy_ops[policy].set_cleaning_param) + return -OCF_ERR_INVAL; + + return cleaning_policy_ops[policy].set_cleaning_param(cache, param_id, + param_value); +} + +static inline int ocf_cleaning_get_param(ocf_cache_t cache, + ocf_cleaning_t policy, uint32_t param_id, uint32_t *param_value) +{ + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (!cleaning_policy_ops[policy].get_cleaning_param) + return -OCF_ERR_INVAL; + + return cleaning_policy_ops[policy].get_cleaning_param(cache, param_id, + param_value); +} + +static inline void ocf_cleaning_perform_cleaning(ocf_cache_t cache, + ocf_cleaner_end_t cmpl) +{ + ocf_cleaning_t policy; + + policy = cache->conf_meta->cleaning_policy_type; + ENV_BUG_ON(policy >= ocf_cleaning_max); + + if (unlikely(!cleaning_policy_ops[policy].perform_cleaning)) + goto unlock; + + cleaning_policy_ops[policy].perform_cleaning(cache, cmpl); +} + +static inline const char *ocf_cleaning_get_name(ocf_cleaning_t policy) +{ + ENV_BUG_ON(!cleaning_policy_ops[policy].name); + + return cleaning_policy_ops[policy].name; +} diff --git a/src/mngt/ocf_mngt_core.c b/src/mngt/ocf_mngt_core.c index 6c7f7c9..472ea73 100644 --- a/src/mngt/ocf_mngt_core.c +++ b/src/mngt/ocf_mngt_core.c @@ -12,6 +12,7 @@ #include "../utils/utils_pipeline.h" #include "../ocf_stats_priv.h" #include "../ocf_def_priv.h" +#include "../cleaning/cleaning_ops.c" static ocf_seq_no_t _ocf_mngt_get_core_seq_no(ocf_cache_t cache) { diff --git a/src/utils/utils_cache_line.h b/src/utils/utils_cache_line.h index 30581c1..0321559 100644 --- a/src/utils/utils_cache_line.h +++ b/src/utils/utils_cache_line.h @@ -12,6 +12,7 @@ #include "../engine/cache_engine.h" #include "../ocf_request.h" #include "../ocf_def_priv.h" +#include "../cleaning/cleaning_ops.c" /** * @file utils_cache_line.h From 26194fc536e412ef078bbb1b063ac45f0854d3ac Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Mon, 19 Jul 2021 15:52:04 +0200 Subject: [PATCH 4/8] Use cleaning ops wrapper functions Signed-off-by: Michal Mielewczyk --- src/cleaning/cleaning.c | 10 ++------- src/cleaning/cleaning.h | 21 ------------------ src/engine/engine_common.c | 8 +------ src/metadata/metadata.c | 9 +------- src/mngt/ocf_mngt_cache.c | 26 ++++++----------------- src/mngt/ocf_mngt_common.c | 7 +----- src/mngt/ocf_mngt_core.c | 19 +++++------------ src/mngt/ocf_mngt_flush.c | 41 ++++++++++++------------------------ src/utils/utils_cache_line.c | 14 ------------ src/utils/utils_cache_line.h | 12 ++--------- src/utils/utils_user_part.c | 11 ++-------- 11 files changed, 34 insertions(+), 144 deletions(-) diff --git a/src/cleaning/cleaning.c b/src/cleaning/cleaning.c index 6af7dbb..94421f0 100644 --- a/src/cleaning/cleaning.c +++ b/src/cleaning/cleaning.c @@ -13,7 +13,7 @@ #include "../mngt/ocf_mngt_common.h" #include "../metadata/metadata.h" #include "../ocf_queue_priv.h" -#include "cleaning_ops.c" +#include "cleaning_ops.h" int ocf_start_cleaner(ocf_cache_t cache) { @@ -82,7 +82,6 @@ static void ocf_cleaner_run_complete(ocf_cleaner_t cleaner, uint32_t interval) void ocf_cleaner_run(ocf_cleaner_t cleaner, ocf_queue_t queue) { ocf_cache_t cache; - ocf_cleaning_t clean_type; OCF_CHECK_NULL(cleaner); OCF_CHECK_NULL(queue); @@ -110,13 +109,8 @@ void ocf_cleaner_run(ocf_cleaner_t cleaner, ocf_queue_t queue) return; } - clean_type = cache->conf_meta->cleaning_policy_type; - - ENV_BUG_ON(clean_type >= ocf_cleaning_max); - ocf_queue_get(queue); cleaner->io_queue = queue; - cleaning_policy_ops[clean_type].perform_cleaning(cache, - ocf_cleaner_run_complete); + ocf_cleaning_perform_cleaning(cache, ocf_cleaner_run_complete); } diff --git a/src/cleaning/cleaning.h b/src/cleaning/cleaning.h index 9c08913..6a73cae 100644 --- a/src/cleaning/cleaning.h +++ b/src/cleaning/cleaning.h @@ -38,27 +38,6 @@ struct cleaning_policy_meta { } meta; }; -struct cleaning_policy_ops { - void (*setup)(ocf_cache_t cache); - int (*initialize)(ocf_cache_t cache, int init_metadata); - void (*deinitialize)(ocf_cache_t cache); - int (*add_core)(ocf_cache_t cache, ocf_core_id_t core_id); - void (*remove_core)(ocf_cache_t cache, ocf_core_id_t core_id); - void (*init_cache_block)(ocf_cache_t cache, uint32_t cache_line); - void (*purge_cache_block)(ocf_cache_t cache, uint32_t cache_line); - int (*purge_range)(ocf_cache_t cache, int core_id, - uint64_t start_byte, uint64_t end_byte); - void (*set_hot_cache_line)(ocf_cache_t cache, uint32_t cache_line); - int (*set_cleaning_param)(ocf_cache_t cache, uint32_t param_id, - uint32_t param_value); - int (*get_cleaning_param)(ocf_cache_t cache, uint32_t param_id, - uint32_t *param_value); - void (*perform_cleaning)(ocf_cache_t cache, ocf_cleaner_end_t cmpl); - const char *name; -}; - -extern struct cleaning_policy_ops cleaning_policy_ops[ocf_cleaning_max]; - struct ocf_cleaner { void *cleaning_policy_context; ocf_queue_t io_queue; diff --git a/src/engine/engine_common.c b/src/engine/engine_common.c index 7937f29..a789b13 100644 --- a/src/engine/engine_common.c +++ b/src/engine/engine_common.c @@ -300,7 +300,6 @@ void ocf_map_cache_line(struct ocf_request *req, { ocf_cache_t cache = req->cache; ocf_core_id_t core_id = ocf_core_get_id(req->core); - ocf_cleaning_t clean_policy_type; unsigned int hash_index = req->map[idx].hash; uint64_t core_line = req->core_line_first + idx; @@ -311,13 +310,8 @@ void ocf_map_cache_line(struct ocf_request *req, ocf_metadata_end_collision_shared_access(cache, cache_line); /* Update dirty cache-block list */ - clean_policy_type = cache->conf_meta->cleaning_policy_type; - ENV_BUG_ON(clean_policy_type >= ocf_cleaning_max); - - if (cleaning_policy_ops[clean_policy_type].init_cache_block != NULL) - cleaning_policy_ops[clean_policy_type]. - init_cache_block(cache, cache_line); + ocf_cleaning_init_cache_block(cache, cache_line); req->map[idx].coll_idx = cache_line; } diff --git a/src/metadata/metadata.c b/src/metadata/metadata.c index 462b9eb..ff6452e 100644 --- a/src/metadata/metadata.c +++ b/src/metadata/metadata.c @@ -1200,19 +1200,12 @@ static void _recovery_invalidate_clean_sec(struct ocf_cache *cache, static void _recovery_reset_cline_metadata(struct ocf_cache *cache, ocf_cache_line_t cline) { - ocf_cleaning_t clean_policy_type; ocf_metadata_set_core_info(cache, cline, OCF_CORE_MAX, ULLONG_MAX); metadata_clear_valid(cache, cline); - clean_policy_type = cache->conf_meta->cleaning_policy_type; - - ENV_BUG_ON(clean_policy_type >= ocf_cleaning_max); - - if (cleaning_policy_ops[clean_policy_type].init_cache_block != NULL) - cleaning_policy_ops[clean_policy_type]. - init_cache_block(cache, cline); + ocf_cleaning_init_cache_block(cache, cline); } static void _recovery_rebuild_metadata(ocf_pipeline_t pipeline, diff --git a/src/mngt/ocf_mngt_cache.c b/src/mngt/ocf_mngt_cache.c index f738392..b9d60cc 100644 --- a/src/mngt/ocf_mngt_cache.c +++ b/src/mngt/ocf_mngt_cache.c @@ -202,29 +202,20 @@ static ocf_error_t __init_cleaning_policy(ocf_cache_t cache) { ocf_cleaning_t cleaning_policy = ocf_cleaning_default; int i; - ocf_error_t result = 0; OCF_ASSERT_PLUGGED(cache); - for (i = 0; i < ocf_cleaning_max; i++) { - if (cleaning_policy_ops[i].setup) - cleaning_policy_ops[i].setup(cache); - } + for (i = 0; i < ocf_cleaning_max; i++) + ocf_cleaning_setup(cache, i); cache->conf_meta->cleaning_policy_type = ocf_cleaning_default; - if (cleaning_policy_ops[cleaning_policy].initialize) - result = cleaning_policy_ops[cleaning_policy].initialize(cache, 1); - return result; + return ocf_cleaning_initialize(cache, cleaning_policy, 1); } static void __deinit_cleaning_policy(ocf_cache_t cache) { - ocf_cleaning_t cleaning_policy; - - cleaning_policy = cache->conf_meta->cleaning_policy_type; - if (cleaning_policy_ops[cleaning_policy].deinitialize) - cleaning_policy_ops[cleaning_policy].deinitialize(cache); + ocf_cleaning_deinitialize(cache); } static void __setup_promotion_policy(ocf_cache_t cache) @@ -470,13 +461,11 @@ void _ocf_mngt_load_init_instance_complete(void *priv, int error) __populate_free(cache); cleaning_policy = cache->conf_meta->cleaning_policy_type; - if (!cleaning_policy_ops[cleaning_policy].initialize) - goto out; if (context->metadata.shutdown_status == ocf_metadata_clean_shutdown) - result = cleaning_policy_ops[cleaning_policy].initialize(cache, 0); + result = ocf_cleaning_initialize(cache, cleaning_policy, 0); else - result = cleaning_policy_ops[cleaning_policy].initialize(cache, 1); + result = ocf_cleaning_initialize(cache, cleaning_policy, 1); if (result) { ocf_cache_log(cache, log_err, @@ -484,7 +473,6 @@ void _ocf_mngt_load_init_instance_complete(void *priv, int error) OCF_PL_FINISH_RET(context->pipeline, result); } -out: ocf_pipeline_next(context->pipeline); } @@ -2074,7 +2062,7 @@ static void _ocf_mngt_cache_load_log(ocf_cache_t cache) ocf_cache_log(cache, log_info, "Cache mode : %s\n", _ocf_cache_mode_get_name(cache_mode)); ocf_cache_log(cache, log_info, "Cleaning policy : %s\n", - cleaning_policy_ops[cleaning_type].name); + ocf_cleaning_get_name(cleaning_type)); ocf_cache_log(cache, log_info, "Promotion policy : %s\n", ocf_promotion_policies[promotion_type].name); ocf_core_visit(cache, _ocf_mngt_cache_load_core_log, diff --git a/src/mngt/ocf_mngt_common.c b/src/mngt/ocf_mngt_common.c index f031199..24bbc73 100644 --- a/src/mngt/ocf_mngt_common.c +++ b/src/mngt/ocf_mngt_common.c @@ -36,15 +36,10 @@ void cache_mngt_core_remove_from_cleaning_pol(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_cleaning_t clean_pol_type; ocf_metadata_start_exclusive_access(&cache->metadata.lock); - clean_pol_type = cache->conf_meta->cleaning_policy_type; - if (cleaning_policy_ops[clean_pol_type].remove_core) { - cleaning_policy_ops[clean_pol_type]. - remove_core(cache, core_id); - } + ocf_cleaning_remove_core(cache, core_id); ocf_metadata_end_exclusive_access(&cache->metadata.lock); } diff --git a/src/mngt/ocf_mngt_core.c b/src/mngt/ocf_mngt_core.c index 472ea73..1732bc4 100644 --- a/src/mngt/ocf_mngt_core.c +++ b/src/mngt/ocf_mngt_core.c @@ -12,7 +12,7 @@ #include "../utils/utils_pipeline.h" #include "../ocf_stats_priv.h" #include "../ocf_def_priv.h" -#include "../cleaning/cleaning_ops.c" +#include "../cleaning/cleaning_ops.h" static ocf_seq_no_t _ocf_mngt_get_core_seq_no(ocf_cache_t cache) { @@ -124,14 +124,12 @@ static void _ocf_mngt_cache_add_core_handle_error( ocf_core_t core = context->core; ocf_core_id_t core_id; ocf_volume_t volume; - ocf_cleaning_t clean_type; if (!core) return; core_id = ocf_core_get_id(core); volume = &core->volume; - clean_type = cache->conf_meta->cleaning_policy_type; if (context->flags.counters_allocated) { env_bit_clear(core_id, @@ -144,11 +142,8 @@ static void _ocf_mngt_cache_add_core_handle_error( core->counters = NULL; } - if (context->flags.clean_pol_added) { - if (cleaning_policy_ops[clean_type].remove_core) - cleaning_policy_ops[clean_type].remove_core(cache, - core_id); - } + if (context->flags.clean_pol_added) + ocf_cleaning_remove_core(cache, core_id); if (context->flags.cutoff_initialized) ocf_core_seq_cutoff_deinit(core); @@ -376,7 +371,6 @@ static void ocf_mngt_cache_add_core_insert(ocf_pipeline_t pipeline, ocf_volume_t volume; ocf_volume_type_t type; ocf_seq_no_t core_sequence_no; - ocf_cleaning_t clean_type; uint64_t length; int i, result = 0; @@ -428,11 +422,8 @@ static void ocf_mngt_cache_add_core_insert(ocf_pipeline_t pipeline, core->conf_meta->length = length; - clean_type = cache->conf_meta->cleaning_policy_type; - if (ocf_cache_is_device_attached(cache) && - cleaning_policy_ops[clean_type].add_core) { - result = cleaning_policy_ops[clean_type].add_core(cache, - core_id); + if (ocf_cache_is_device_attached(cache)) { + result = ocf_cleaning_add_core(cache, core_id); if (result) OCF_PL_FINISH_RET(pipeline, result); diff --git a/src/mngt/ocf_mngt_flush.c b/src/mngt/ocf_mngt_flush.c index 6b4a549..141e8a5 100644 --- a/src/mngt/ocf_mngt_flush.c +++ b/src/mngt/ocf_mngt_flush.c @@ -915,24 +915,21 @@ int ocf_mngt_cache_cleaning_set_policy(ocf_cache_t cache, ocf_cleaning_t type) if (type == old_type) { ocf_cache_log(cache, log_info, "Cleaning policy %s is already " - "set\n", cleaning_policy_ops[old_type].name); + "set\n", ocf_cleaning_get_name(old_type)); return 0; } ocf_metadata_start_exclusive_access(&cache->metadata.lock); - if (cleaning_policy_ops[old_type].deinitialize) - cleaning_policy_ops[old_type].deinitialize(cache); + ocf_cleaning_deinitialize(cache); - if (cleaning_policy_ops[type].initialize) { - if (cleaning_policy_ops[type].initialize(cache, 1)) { - /* - * If initialization of new cleaning policy failed, - * we set cleaning policy to nop. - */ - type = ocf_cleaning_nop; - ret = -OCF_ERR_INVAL; - } + if (ocf_cleaning_initialize(cache, type, 1)) { + /* + * If initialization of new cleaning policy failed, + * we set cleaning policy to nop. + */ + type = ocf_cleaning_nop; + ret = -OCF_ERR_INVAL; } cache->conf_meta->cleaning_policy_type = type; @@ -940,8 +937,8 @@ int ocf_mngt_cache_cleaning_set_policy(ocf_cache_t cache, ocf_cleaning_t type) ocf_metadata_end_exclusive_access(&cache->metadata.lock); ocf_cache_log(cache, log_info, "Changing cleaning policy from " - "%s to %s\n", cleaning_policy_ops[old_type].name, - cleaning_policy_ops[type].name); + "%s to %s\n", ocf_cleaning_get_name(old_type), + ocf_cleaning_get_name(type)); return ret; } @@ -966,13 +963,9 @@ int ocf_mngt_cache_cleaning_set_param(ocf_cache_t cache, ocf_cleaning_t type, if (type < 0 || type >= ocf_cleaning_max) return -OCF_ERR_INVAL; - if (!cleaning_policy_ops[type].set_cleaning_param) - return -OCF_ERR_INVAL; - ocf_metadata_start_exclusive_access(&cache->metadata.lock); - ret = cleaning_policy_ops[type].set_cleaning_param(cache, - param_id, param_value); + ret = ocf_cleaning_set_param(cache, type, param_id, param_value); ocf_metadata_end_exclusive_access(&cache->metadata.lock); @@ -982,19 +975,11 @@ int ocf_mngt_cache_cleaning_set_param(ocf_cache_t cache, ocf_cleaning_t type, int ocf_mngt_cache_cleaning_get_param(ocf_cache_t cache, ocf_cleaning_t type, uint32_t param_id, uint32_t *param_value) { - int ret; - OCF_CHECK_NULL(cache); OCF_CHECK_NULL(param_value); if (type < 0 || type >= ocf_cleaning_max) return -OCF_ERR_INVAL; - if (!cleaning_policy_ops[type].get_cleaning_param) - return -OCF_ERR_INVAL; - - ret = cleaning_policy_ops[type].get_cleaning_param(cache, - param_id, param_value); - - return ret; + return ocf_cleaning_get_param(cache, type, param_id, param_value); } diff --git a/src/utils/utils_cache_line.c b/src/utils/utils_cache_line.c index 879c0fc..281ff59 100644 --- a/src/utils/utils_cache_line.c +++ b/src/utils/utils_cache_line.c @@ -6,19 +6,6 @@ #include "utils_cache_line.h" #include "../promotion/promotion.h" -void ocf_cleaning_set_hot_cache_line(struct ocf_cache *cache, - ocf_cache_line_t line) -{ - ocf_cleaning_t cleaning_type = cache->conf_meta->cleaning_policy_type; - - ENV_BUG_ON(cleaning_type >= ocf_cleaning_max); - - if (cleaning_policy_ops[cleaning_type].set_hot_cache_line) { - cleaning_policy_ops[cleaning_type]. - set_hot_cache_line(cache, line); - } -} - static void __set_cache_line_invalid(struct ocf_cache *cache, uint8_t start_bit, uint8_t end_bit, ocf_cache_line_t line, ocf_core_id_t core_id, ocf_part_id_t part_id) @@ -178,6 +165,5 @@ void set_cache_line_dirty(struct ocf_cache *cache, uint8_t start_bit, } } - ocf_cleaning_set_hot_cache_line(cache, line); } diff --git a/src/utils/utils_cache_line.h b/src/utils/utils_cache_line.h index 0321559..eccc962 100644 --- a/src/utils/utils_cache_line.h +++ b/src/utils/utils_cache_line.h @@ -12,7 +12,7 @@ #include "../engine/cache_engine.h" #include "../ocf_request.h" #include "../ocf_def_priv.h" -#include "../cleaning/cleaning_ops.c" +#include "../cleaning/cleaning_ops.h" /** * @file utils_cache_line.h @@ -69,9 +69,6 @@ static inline uint64_t ocf_lines_2_bytes(struct ocf_cache *cache, return lines * ocf_line_size(cache); } -void ocf_cleaning_set_hot_cache_line(struct ocf_cache *cache, - ocf_cache_line_t line); - /** * @brief Set cache line invalid * @@ -162,13 +159,8 @@ void set_cache_line_dirty(struct ocf_cache *cache, uint8_t start_bit, static inline void ocf_purge_cleaning_policy(struct ocf_cache *cache, ocf_cache_line_t line) { - ocf_cleaning_t clean_type = cache->conf_meta->cleaning_policy_type; - - ENV_BUG_ON(clean_type >= ocf_cleaning_max); - /* Remove from cleaning policy */ - if (cleaning_policy_ops[clean_type].purge_cache_block != NULL) - cleaning_policy_ops[clean_type].purge_cache_block(cache, line); + ocf_cleaning_purge_cache_block(cache, line); } /** diff --git a/src/utils/utils_user_part.c b/src/utils/utils_user_part.c index eadb3c9..056f8a0 100644 --- a/src/utils/utils_user_part.c +++ b/src/utils/utils_user_part.c @@ -99,9 +99,6 @@ void ocf_user_part_move(struct ocf_request *req) ocf_cache_line_t line; ocf_part_id_t id_old, id_new; uint32_t i; - ocf_cleaning_t type = cache->conf_meta->cleaning_policy_type; - - ENV_BUG_ON(type >= ocf_cleaning_max); entry = &req->map[0]; for (i = 0; i < req->core_line_count; i++, entry++) { @@ -141,9 +138,7 @@ void ocf_user_part_move(struct ocf_request *req) * TODO: Consider adding update_cache_line() ops * to cleaning policy to let policies handle this. */ - if (cleaning_policy_ops[type].purge_cache_block) - cleaning_policy_ops[type]. - purge_cache_block(cache, line); + ocf_cleaning_purge_cache_block(cache, line); } ocf_lru_repart(cache, line, &cache->user_parts[id_old].part, @@ -155,9 +150,7 @@ void ocf_user_part_move(struct ocf_request *req) */ if (metadata_test_dirty(cache, line)) { /* Add cline back to cleaning policy */ - if (cleaning_policy_ops[type].set_hot_cache_line) - cleaning_policy_ops[type]. - set_hot_cache_line(cache, line); + ocf_cleaning_set_hot_cache_line(cache, line); env_atomic_inc(&req->core->runtime_meta-> part_counters[id_new].dirty_clines); From f33a6e5ce0b5ab34c76257819fd2160920df6e2b Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Wed, 21 Jul 2021 12:51:39 +0200 Subject: [PATCH 5/8] Make switching cleaning policy asynchronous Making the operation asynchronous will allow to use refcnt utility as an synchronization mechanism between processing cachelines and deinitializing cleaning policy. Signed-off-by: Michal Mielewczyk --- inc/ocf_mngt.h | 19 ++++-- src/mngt/ocf_mngt_flush.c | 131 ++++++++++++++++++++++++++++++-------- 2 files changed, 119 insertions(+), 31 deletions(-) diff --git a/inc/ocf_mngt.h b/inc/ocf_mngt.h index afd9650..6c3962a 100644 --- a/inc/ocf_mngt.h +++ b/inc/ocf_mngt.h @@ -730,6 +730,15 @@ static inline bool ocf_mngt_cache_mode_has_lazy_write(ocf_cache_mode_t mode) */ int ocf_mngt_cache_set_mode(ocf_cache_t cache, ocf_cache_mode_t mode); +/** + * @brief Completion callback of switch cleaning policy operation + * + * @param[in] priv Callback context + * @param[in] error Error code (zero on success) + */ +typedef void (*ocf_mngt_cache_set_cleaning_policy_end_t)( void *priv, + int error); + /** * @brief Set cleaning policy in given cache * @@ -737,12 +746,14 @@ int ocf_mngt_cache_set_mode(ocf_cache_t cache, ocf_cache_mode_t mode); * use function ocf_mngt_cache_save(). * * @param[in] cache Cache handle - * @param[in] type Cleaning policy type + * @param[out] new_policy New cleaning policy + * @param[in] cmpl Completion callback + * @param[in] priv Completion callback context * - * @retval 0 Policy has been set successfully - * @retval Non-zero Error occurred and policy has not been set */ -int ocf_mngt_cache_cleaning_set_policy(ocf_cache_t cache, ocf_cleaning_t type); +void ocf_mngt_cache_cleaning_set_policy(ocf_cache_t cache, + ocf_cleaning_t new_policy, + ocf_mngt_cache_set_cleaning_policy_end_t cmpl, void *priv); /** * @brief Get current cleaning policy from given cache diff --git a/src/mngt/ocf_mngt_flush.c b/src/mngt/ocf_mngt_flush.c index 141e8a5..afa365d 100644 --- a/src/mngt/ocf_mngt_flush.c +++ b/src/mngt/ocf_mngt_flush.c @@ -901,46 +901,123 @@ void ocf_mngt_cache_flush_interrupt(ocf_cache_t cache) cache->flushing_interrupted = 1; } -int ocf_mngt_cache_cleaning_set_policy(ocf_cache_t cache, ocf_cleaning_t type) +struct ocf_mngt_cache_set_cleaning_context { - ocf_cleaning_t old_type; - int ret = 0; + /* pipeline for switching cleaning policy */ + ocf_pipeline_t pipeline; + /* target cache */ + ocf_cache_t cache; + /* new cleaning policy */ + ocf_cleaning_t new_policy; + /* old cleaning policy */ + ocf_cleaning_t old_policy; + /* completion function */ + ocf_mngt_cache_set_cleaning_policy_end_t cmpl; + /* completion function */ + void *priv; +}; - OCF_CHECK_NULL(cache); - - if (type < 0 || type >= ocf_cleaning_max) - return -OCF_ERR_INVAL; - - old_type = cache->conf_meta->cleaning_policy_type; - - if (type == old_type) { - ocf_cache_log(cache, log_info, "Cleaning policy %s is already " - "set\n", ocf_cleaning_get_name(old_type)); - return 0; - } +static void _ocf_mngt_deinit_clean_policy(ocf_pipeline_t pipeline, void *priv, + ocf_pipeline_arg_t arg) +{ + struct ocf_mngt_cache_set_cleaning_context *context = priv; + ocf_cache_t cache = context->cache; ocf_metadata_start_exclusive_access(&cache->metadata.lock); ocf_cleaning_deinitialize(cache); - if (ocf_cleaning_initialize(cache, type, 1)) { - /* - * If initialization of new cleaning policy failed, - * we set cleaning policy to nop. - */ - type = ocf_cleaning_nop; - ret = -OCF_ERR_INVAL; + ocf_pipeline_next(context->pipeline); +} + +static void _ocf_mngt_init_clean_policy(ocf_pipeline_t pipeline, void *priv, + ocf_pipeline_arg_t arg) +{ + int result; + struct ocf_mngt_cache_set_cleaning_context *context = priv; + ocf_cache_t cache = context->cache; + ocf_cleaning_t old_policy = context->old_policy; + ocf_cleaning_t new_policy = context->new_policy; + ocf_cleaning_t emergency_policy = ocf_cleaning_nop; + + result = ocf_cleaning_initialize(cache, new_policy, 1); + if (result) { + ocf_cache_log(cache, log_info, "Failed to initialize %s cleaning " + "policy. Setting %s instead\n", + ocf_cleaning_get_name(new_policy), + ocf_cleaning_get_name(emergency_policy)); + new_policy = emergency_policy; + } else { + ocf_cache_log(cache, log_info, "Changing cleaning policy from " + "%s to %s\n", ocf_cleaning_get_name(old_policy), + ocf_cleaning_get_name(new_policy)); } - cache->conf_meta->cleaning_policy_type = type; + cache->conf_meta->cleaning_policy_type = new_policy; ocf_metadata_end_exclusive_access(&cache->metadata.lock); - ocf_cache_log(cache, log_info, "Changing cleaning policy from " - "%s to %s\n", ocf_cleaning_get_name(old_type), - ocf_cleaning_get_name(type)); + OCF_PL_NEXT_ON_SUCCESS_RET(pipeline, result); +} - return ret; +static void _ocf_mngt_set_cleaning_finish(ocf_pipeline_t pipeline, void *priv, + int error) +{ + struct ocf_mngt_cache_set_cleaning_context *context = priv; + + context->cmpl(context->priv, error); + + ocf_pipeline_destroy(pipeline); +} + +static +struct ocf_pipeline_properties _ocf_mngt_cache_set_cleaning_policy = { + .priv_size = sizeof(struct ocf_mngt_cache_set_cleaning_context), + .finish = _ocf_mngt_set_cleaning_finish, + .steps = { + OCF_PL_STEP(_ocf_mngt_deinit_clean_policy), + OCF_PL_STEP(_ocf_mngt_init_clean_policy), + OCF_PL_STEP_TERMINATOR(), + }, +}; + +void ocf_mngt_cache_cleaning_set_policy(ocf_cache_t cache, + ocf_cleaning_t new_policy, + ocf_mngt_cache_set_cleaning_policy_end_t cmpl, void *priv) +{ + struct ocf_mngt_cache_set_cleaning_context *context; + ocf_pipeline_t pipeline; + ocf_cleaning_t old_policy; + int ret = 0; + + OCF_CHECK_NULL(cache); + + if (new_policy < 0 || new_policy >= ocf_cleaning_max) + OCF_CMPL_RET(priv, -OCF_ERR_INVAL); + + old_policy = cache->conf_meta->cleaning_policy_type; + + if (new_policy == old_policy) { + ocf_cache_log(cache, log_info, "Cleaning policy %s is already " + "set\n", ocf_cleaning_get_name(old_policy)); + OCF_CMPL_RET(priv, 0); + } + + ret = ocf_pipeline_create(&pipeline, cache, + &_ocf_mngt_cache_set_cleaning_policy); + if (ret) + OCF_CMPL_RET(priv, ret); + + context = ocf_pipeline_get_priv(pipeline); + + context->cmpl = cmpl; + context->cache = cache; + context->pipeline = pipeline; + context->new_policy = new_policy; + context->old_policy = old_policy; + context->priv = priv; + + OCF_PL_NEXT_RET(pipeline); } int ocf_mngt_cache_cleaning_get_policy(ocf_cache_t cache, ocf_cleaning_t *type) From b83da68f85110fa782cb46ce78584ab3ddae5367 Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Tue, 20 Jul 2021 10:49:30 +0200 Subject: [PATCH 6/8] Cleaner context ref counter To prevent deinitializing cleaner context (i.e. during switching policy) during processing requests, access to cleaner should be protected with reference counter Signed-off-by: Michal Mielewczyk --- src/cleaning/cleaning.h | 2 ++ src/cleaning/cleaning_ops.h | 48 ++++++++++++++++++++++++++++++++++++- src/mngt/ocf_mngt_cache.c | 2 ++ src/mngt/ocf_mngt_flush.c | 17 ++++++++++--- 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/cleaning/cleaning.h b/src/cleaning/cleaning.h index 6a73cae..007dac0 100644 --- a/src/cleaning/cleaning.h +++ b/src/cleaning/cleaning.h @@ -10,6 +10,7 @@ #include "nop_structs.h" #include "acp_structs.h" #include "ocf/ocf_cleaner.h" +#include "../utils/utils_refcnt.h" #define CLEANING_POLICY_CONFIG_BYTES 256 #define CLEANING_POLICY_TYPE_MAX 4 @@ -39,6 +40,7 @@ struct cleaning_policy_meta { }; struct ocf_cleaner { + struct ocf_refcnt refcnt __attribute__((aligned(64))); void *cleaning_policy_context; ocf_queue_t io_queue; ocf_cleaner_end_t end; diff --git a/src/cleaning/cleaning_ops.h b/src/cleaning/cleaning_ops.h index 6210d27..f700dc8 100644 --- a/src/cleaning/cleaning_ops.h +++ b/src/cleaning/cleaning_ops.h @@ -9,6 +9,7 @@ #include "../metadata/metadata_superblock.h" #include "../metadata/metadata_structs.h" #include "../ocf_cache_priv.h" +#include "../utils/utils_refcnt.h" struct cleaning_policy_ops { void (*setup)(ocf_cache_t cache); @@ -104,6 +105,10 @@ static inline int ocf_cleaning_add_core(ocf_cache_t cache, ocf_core_id_t core_id) { ocf_cleaning_t policy; + int result = 0; + + if (unlikely(!ocf_refcnt_inc(&cache->cleaner.refcnt))) + return -OCF_ERR_NO_LOCK; policy = cache->conf_meta->cleaning_policy_type; @@ -112,7 +117,12 @@ static inline int ocf_cleaning_add_core(ocf_cache_t cache, if (unlikely(!cleaning_policy_ops[policy].add_core)) goto unlock; - return cleaning_policy_ops[policy].add_core(cache, core_id); + result = cleaning_policy_ops[policy].add_core(cache, core_id); + +unlock: + ocf_refcnt_dec(&cache->cleaner.refcnt); + + return result; } static inline void ocf_cleaning_remove_core(ocf_cache_t cache, @@ -120,6 +130,9 @@ static inline void ocf_cleaning_remove_core(ocf_cache_t cache, { ocf_cleaning_t policy; + if (unlikely(!ocf_refcnt_inc(&cache->cleaner.refcnt))) + return; + policy = cache->conf_meta->cleaning_policy_type; ENV_BUG_ON(policy >= ocf_cleaning_max); @@ -128,6 +141,9 @@ static inline void ocf_cleaning_remove_core(ocf_cache_t cache, goto unlock; cleaning_policy_ops[policy].remove_core(cache, core_id); + +unlock: + ocf_refcnt_dec(&cache->cleaner.refcnt); } static inline void ocf_cleaning_init_cache_block(ocf_cache_t cache, @@ -135,6 +151,9 @@ static inline void ocf_cleaning_init_cache_block(ocf_cache_t cache, { ocf_cleaning_t policy; + if (unlikely(!ocf_refcnt_inc(&cache->cleaner.refcnt))) + return; + policy = cache->conf_meta->cleaning_policy_type; ENV_BUG_ON(policy >= ocf_cleaning_max); @@ -142,6 +161,9 @@ static inline void ocf_cleaning_init_cache_block(ocf_cache_t cache, goto unlock; cleaning_policy_ops[policy].init_cache_block(cache, cache_line); + +unlock: + ocf_refcnt_dec(&cache->cleaner.refcnt); } static inline void ocf_cleaning_purge_cache_block(ocf_cache_t cache, @@ -149,6 +171,9 @@ static inline void ocf_cleaning_purge_cache_block(ocf_cache_t cache, { ocf_cleaning_t policy; + if (unlikely(!ocf_refcnt_inc(&cache->cleaner.refcnt))) + return; + policy = cache->conf_meta->cleaning_policy_type; ENV_BUG_ON(policy >= ocf_cleaning_max); @@ -156,6 +181,9 @@ static inline void ocf_cleaning_purge_cache_block(ocf_cache_t cache, goto unlock; cleaning_policy_ops[policy].purge_cache_block(cache, cache_line); + +unlock: + ocf_refcnt_dec(&cache->cleaner.refcnt); } static inline void ocf_cleaning_purge_range(ocf_cache_t cache, @@ -163,6 +191,9 @@ static inline void ocf_cleaning_purge_range(ocf_cache_t cache, { ocf_cleaning_t policy; + if (unlikely(!ocf_refcnt_inc(&cache->cleaner.refcnt))) + return; + policy = cache->conf_meta->cleaning_policy_type; ENV_BUG_ON(policy >= ocf_cleaning_max); @@ -171,6 +202,9 @@ static inline void ocf_cleaning_purge_range(ocf_cache_t cache, cleaning_policy_ops[policy].purge_range(cache, core_id, start_byte, end_byte); + +unlock: + ocf_refcnt_dec(&cache->cleaner.refcnt); } static inline void ocf_cleaning_set_hot_cache_line(ocf_cache_t cache, @@ -178,6 +212,9 @@ static inline void ocf_cleaning_set_hot_cache_line(ocf_cache_t cache, { ocf_cleaning_t policy; + if (unlikely(!ocf_refcnt_inc(&cache->cleaner.refcnt))) + return; + policy = cache->conf_meta->cleaning_policy_type; ENV_BUG_ON(policy >= ocf_cleaning_max); @@ -185,6 +222,9 @@ static inline void ocf_cleaning_set_hot_cache_line(ocf_cache_t cache, goto unlock; cleaning_policy_ops[policy].set_hot_cache_line(cache, cache_line); + +unlock: + ocf_refcnt_dec(&cache->cleaner.refcnt); } static inline int ocf_cleaning_set_param(ocf_cache_t cache, @@ -216,6 +256,9 @@ static inline void ocf_cleaning_perform_cleaning(ocf_cache_t cache, { ocf_cleaning_t policy; + if (unlikely(!ocf_refcnt_inc(&cache->cleaner.refcnt))) + return; + policy = cache->conf_meta->cleaning_policy_type; ENV_BUG_ON(policy >= ocf_cleaning_max); @@ -223,6 +266,9 @@ static inline void ocf_cleaning_perform_cleaning(ocf_cache_t cache, goto unlock; cleaning_policy_ops[policy].perform_cleaning(cache, cmpl); + +unlock: + ocf_refcnt_dec(&cache->cleaner.refcnt); } static inline const char *ocf_cleaning_get_name(ocf_cleaning_t policy) diff --git a/src/mngt/ocf_mngt_cache.c b/src/mngt/ocf_mngt_cache.c index b9d60cc..54a2a67 100644 --- a/src/mngt/ocf_mngt_cache.c +++ b/src/mngt/ocf_mngt_cache.c @@ -205,6 +205,8 @@ static ocf_error_t __init_cleaning_policy(ocf_cache_t cache) OCF_ASSERT_PLUGGED(cache); + ocf_refcnt_init(&cache->cleaner.refcnt); + for (i = 0; i < ocf_cleaning_max; i++) ocf_cleaning_setup(cache, i); diff --git a/src/mngt/ocf_mngt_flush.c b/src/mngt/ocf_mngt_flush.c index afa365d..afa1ab7 100644 --- a/src/mngt/ocf_mngt_flush.c +++ b/src/mngt/ocf_mngt_flush.c @@ -917,6 +917,16 @@ struct ocf_mngt_cache_set_cleaning_context void *priv; }; +static void _ocf_mngt_cleaning_deinit_complete(void *priv) +{ + struct ocf_mngt_cache_set_cleaning_context *context = priv; + ocf_cache_t cache = context->cache; + + ocf_cleaning_deinitialize(cache); + + ocf_pipeline_next(context->pipeline); +} + static void _ocf_mngt_deinit_clean_policy(ocf_pipeline_t pipeline, void *priv, ocf_pipeline_arg_t arg) { @@ -925,9 +935,9 @@ static void _ocf_mngt_deinit_clean_policy(ocf_pipeline_t pipeline, void *priv, ocf_metadata_start_exclusive_access(&cache->metadata.lock); - ocf_cleaning_deinitialize(cache); - - ocf_pipeline_next(context->pipeline); + ocf_refcnt_freeze(&cache->cleaner.refcnt); + ocf_refcnt_register_zero_cb(&cache->cleaner.refcnt, + _ocf_mngt_cleaning_deinit_complete, context); } static void _ocf_mngt_init_clean_policy(ocf_pipeline_t pipeline, void *priv, @@ -955,6 +965,7 @@ static void _ocf_mngt_init_clean_policy(ocf_pipeline_t pipeline, void *priv, cache->conf_meta->cleaning_policy_type = new_policy; + ocf_refcnt_unfreeze(&cache->cleaner.refcnt); ocf_metadata_end_exclusive_access(&cache->metadata.lock); OCF_PL_NEXT_ON_SUCCESS_RET(pipeline, result); From a94677f433c6eedf9a2c14219e8810c935c59881 Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Tue, 20 Jul 2021 13:40:28 +0200 Subject: [PATCH 7/8] Fix cleaning UT Signed-off-by: Michal Mielewczyk --- tests/unit/tests/cleaning/cleaning.c/ocf_cleaner_run_test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/tests/cleaning/cleaning.c/ocf_cleaner_run_test.c b/tests/unit/tests/cleaning/cleaning.c/ocf_cleaner_run_test.c index 0a944b7..42db6f5 100644 --- a/tests/unit/tests/cleaning/cleaning.c/ocf_cleaner_run_test.c +++ b/tests/unit/tests/cleaning/cleaning.c/ocf_cleaner_run_test.c @@ -43,7 +43,7 @@ */ -int __wrap_cleaning_alru_perform_cleaning(struct ocf_cache *cache, ocf_cleaner_end_t cmpl) +int __wrap_ocf_cleaning_perform_cleaning(struct ocf_cache *cache, ocf_cleaner_end_t cmpl) { function_called(); return mock(); @@ -132,8 +132,8 @@ static void ocf_cleaner_run_test01(void **state) expect_function_call(__wrap__ocf_cleaner_run_check_dirty_inactive); will_return(__wrap__ocf_cleaner_run_check_dirty_inactive, 0); - expect_function_call(__wrap_cleaning_alru_perform_cleaning); - will_return(__wrap_cleaning_alru_perform_cleaning, 0); + expect_function_call(__wrap_ocf_cleaning_perform_cleaning); + will_return(__wrap_ocf_cleaning_perform_cleaning, 0); ocf_cleaner_set_cmpl(&cache->cleaner, cleaner_complete); From 1620c544a13d8d4b2117f0af6b8ebb90c5b2618a Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Fri, 23 Jul 2021 10:29:49 +0200 Subject: [PATCH 8/8] pyocf: update cleaning policy switching test Signed-off-by: Michal Mielewczyk --- tests/functional/pyocf/types/cache.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/functional/pyocf/types/cache.py b/tests/functional/pyocf/types/cache.py index 820a794..10ef466 100644 --- a/tests/functional/pyocf/types/cache.py +++ b/tests/functional/pyocf/types/cache.py @@ -228,14 +228,16 @@ class Cache: def set_cleaning_policy(self, cleaning_policy: CleaningPolicy): self.write_lock() - status = self.owner.lib.ocf_mngt_cache_cleaning_set_policy( - self.cache_handle, cleaning_policy + c = OcfCompletion([("priv", c_void_p), ("error", c_int)]) + self.owner.lib.ocf_mngt_cache_cleaning_set_policy( + self.cache_handle, cleaning_policy, c, None ) + c.wait() self.write_unlock() - if status: - raise OcfError("Error changing cleaning policy", status) + if c.results["error"]: + raise OcfError("Error changing cleaning policy", c.results["error"]) def set_cleaning_policy_param( self, cleaning_policy: CleaningPolicy, param_id, param_value @@ -704,8 +706,7 @@ lib.ocf_mngt_cache_remove_core.argtypes = [c_void_p, c_void_p, c_void_p] lib.ocf_mngt_cache_add_core.argtypes = [c_void_p, c_void_p, c_void_p, c_void_p] lib.ocf_cache_get_name.argtypes = [c_void_p] lib.ocf_cache_get_name.restype = c_char_p -lib.ocf_mngt_cache_cleaning_set_policy.argtypes = [c_void_p, c_uint32] -lib.ocf_mngt_cache_cleaning_set_policy.restype = c_int +lib.ocf_mngt_cache_cleaning_set_policy.argtypes = [c_void_p, c_uint32, c_void_p, c_void_p] lib.ocf_mngt_core_set_seq_cutoff_policy_all.argtypes = [c_void_p, c_uint32] lib.ocf_mngt_core_set_seq_cutoff_policy_all.restype = c_int lib.ocf_mngt_core_set_seq_cutoff_threshold_all.argtypes = [c_void_p, c_uint32]