From c088faf711dcf434620df4bac07feb31a3d486e9 Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Fri, 22 Apr 2022 00:26:34 +0200 Subject: [PATCH 1/5] doc: Add disable_cleaner requirements documentation Signed-off-by: Robert Baldyga --- doc/requirements/disable_cleaner | 67 ++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 doc/requirements/disable_cleaner diff --git a/doc/requirements/disable_cleaner b/doc/requirements/disable_cleaner new file mode 100644 index 0000000..b9df7b4 --- /dev/null +++ b/doc/requirements/disable_cleaner @@ -0,0 +1,67 @@ +--- +group: disable_cleaner +--- + +Disabling cleaner is a feature that is intented to be used with read cache +(Write-Thourgh and Write-Around modes), where no dirty data is produced, so +the cleaning is not necessary. Its major benefit is metadata memory footprint +reduction, which is result of not allocating "cleaning" metatada section, +that constitutes about 20% of total metadata capacity. + +While it is possible to use disable_cleaner option Write-Back and Write-Only +modes, it may result in performance degradation due to necessity to perform +cleaning on eviction. + +-------------------------------------------------------------------------------- +-------------------------------------------------------------------------------- +title: Setting cleaner_disabled mode +id: set_cleaner_disabled +--- + +It shall be possible to select cleaner_disabled mode during cache attach +operation by setting apropirate field in attach config. + +-------------------------------------------------------------------------------- +-------------------------------------------------------------------------------- +title: Loading cache cleaner_disabled mode +id: load_cleaner_disabled +--- + +The cleaner_disabled setting should be preserved in cache metadata, i.e. on the +cache load/recovery the setting should be restored to the value it had before +the cache stop. + +-------------------------------------------------------------------------------- +-------------------------------------------------------------------------------- +title: Metadata "cleaning" section allocation +id: cleaning_section_alocation +--- + +When disable_cleaner option is selected the "cleaning" section in OCF metadata +shall not be allocated neither in DRAM nor on the cache drive. + +-------------------------------------------------------------------------------- +-------------------------------------------------------------------------------- +title: Starting with NOP cleaning policy +id: starting_with_nop_policy +--- + +When attaching/loading cache with disable_cleaner option selected, the cleaning +policy shall always be NOP. + +-------------------------------------------------------------------------------- +-------------------------------------------------------------------------------- +title: NOP cleaning policy enforcement +id: nop_enforcement +--- + +When disable_cleaner option is selected it shall not be possible to change the +cleaning policy to other than NOP. + +-------------------------------------------------------------------------------- +-------------------------------------------------------------------------------- +title: Default setting +id: default_setting +--- + +By default the disable_cleaner option shall not be selected. From 761ff2f053318dc106d22236192a8d2170eb47e6 Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Wed, 27 Apr 2022 16:07:56 +0200 Subject: [PATCH 2/5] pyocf: Add test designs for disable_cleaner option Signed-off-by: Robert Baldyga --- .../tests/management/test_disable_cleaner.py | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 tests/functional/tests/management/test_disable_cleaner.py diff --git a/tests/functional/tests/management/test_disable_cleaner.py b/tests/functional/tests/management/test_disable_cleaner.py new file mode 100644 index 0000000..4782f3b --- /dev/null +++ b/tests/functional/tests/management/test_disable_cleaner.py @@ -0,0 +1,108 @@ +# +# Copyright(c) 2022 Intel Corporation +# SPDX-License-Identifier: BSD-3-Clause +# + +import pytest + + +@pytest.mark.skip(reason="not implemented") +def test_attach_cleaner_disabled(pyocf_ctx): + """ + title: Attach cache with cleaner_disabled option set. + description: | + Check that cache can be attached when cleaner_disabled option is + selected and that "cleaning" metadata section is not allocated. + pass_criteria: + - Cache attaches properly. + - The "cleaning" metadata section is not allocated. + - Cache stops properly. + steps: + - Start the cache with default config. + - Prepare default attach config and set cleaner_disabled field to true. + - Attach cache device using prepared config. + - Verify that cache was attached properly. + - Verify that "cleaning" metadata section was not allocated. + - Stop the cache. + - Verify that the cache stopped properly. + requirements: + - disable_cleaner::set_cleaner_disabled + - disable_cleaner::cleaning_section_alocation + """ + pass + + +@pytest.mark.skip(reason="not implemented") +def test_load_cleaner_disabled(pyocf_ctx): + """ + title: Load cache in cleaner_disabled mode. + description: | + Check that loading the cache that was previously attached with + cleaner_disabled option preserves cleaner_disabled setting. + pass_criteria: + - Cache loads properly. + - The "cleaning" metadata section is not allocated. + - Cache stops properly. + steps: + - Start the cache with default config. + - Prepare default attach config and set cleaner_disabled field to true. + - Attach cache device using prepared config. + - Stop the cache. + - Load the cache. + - Verify that cache was loaded properly. + - Verify that "cleaning" metadata section was not allocated. + - Stop the cache. + - Verify that the cache stopped properly. + requirements: + - disable_cleaner::load_cleaner_disabled + - disable_cleaner::cleaning_section_alocation + """ + pass + + +@pytest.mark.skip(reason="not implemented") +def test_cleaner_disabled_nop(pyocf_ctx): + """ + title: NOP enfocement in cleaner_disabled mode.. + description: | + Check that after attaching cache with cleaner_diabled option set, the + cleaning policy is by default set to NOP and that it is not possible + to change it. + pass_criteria: + - Cleaning policy is set to NOP after cache attach. + - It is not possible to change cleaning policy to other than NOP. + steps: + - Start the cache with default config. + - Prepare default attach config and set cleaner_disabled field to true. + - Attach cache device using prepared config. + - Verify that cleaning policy is NOP. + - Try to set cleaning policy to [ALRU, ACP] and verify that operation failed. + - Try to set cleaning policy to NOP and verify that operation succeeded. + - Stop the cache. + requirements: + - disable_cleaner::starting_with_nop_policy + - disable_cleaner::nop_enforcement + """ + pass + + +@pytest.mark.skip(reason="not implemented") +def test_attach_cleaner_disabled_non_default(pyocf_ctx): + """ + title: Attach cache with default config does not set clener_disabled. + description: | + Check that when attaching cache with default attach config the + cleaner_disabled option is not selected. + pass_criteria: + - Cache attaches properly. + - The "cleaning" metadata section is not allocated. + - Cache stops properly. + steps: + - Start the cache with default config. + - Attach cache device using default config. + - Verify that "cleaning" metadata section was allocated. + - Stop the cache. + requirements: + - disable_cleaner::default_setting + """ + pass From d4df912f46260ec3dffb5a7e491c1e6f6efcec73 Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Thu, 14 Apr 2022 21:30:31 +0200 Subject: [PATCH 3/5] Add option to disable cleaner This allows to avoid allocating cleaner metadata section and effectively save up to 20% of metadata memory footprint. Signed-off-by: Robert Baldyga --- inc/ocf_err.h | 5 ++++- inc/ocf_mngt.h | 16 ++++++++++++++++ src/metadata/metadata.c | 14 +++++++++++++- src/metadata/metadata.h | 5 ++++- src/metadata/metadata_cleaning_policy.c | 2 ++ src/metadata/metadata_superblock.c | 11 +++++++++++ src/metadata/metadata_superblock.h | 1 + src/mngt/ocf_mngt_cache.c | 12 +++++++++++- src/mngt/ocf_mngt_flush.c | 3 +++ 9 files changed, 65 insertions(+), 4 deletions(-) diff --git a/inc/ocf_err.h b/inc/ocf_err.h index 53bc626..a8d0563 100644 --- a/inc/ocf_err.h +++ b/inc/ocf_err.h @@ -155,7 +155,10 @@ typedef enum { /** Operation only allowed in standby mode **/ OCF_ERR_CACHE_NOT_STANDBY, - OCF_ERR_MAX = OCF_ERR_CACHE_NOT_STANDBY, + /** Operation not allowed when cleaner is disabled **/ + OCF_ERR_CLEANER_DISABLED, + + OCF_ERR_MAX = OCF_ERR_CLEANER_DISABLED, } ocf_error_t; diff --git a/inc/ocf_mngt.h b/inc/ocf_mngt.h index de43f14..f8b5af2 100644 --- a/inc/ocf_mngt.h +++ b/inc/ocf_mngt.h @@ -424,6 +424,21 @@ struct ocf_mngt_cache_attach_config { * @brief If set, cache device will be discarded on cache start */ bool discard_on_start; + + /** + * @brief If set, asynchronous cleaning will be disabled + * + * Has similar effect to setting cleaning policy to NOP, but + * additionally prevents allocating "cleaning" metadata section, + * which can reduce memory footprint by up to 20%. + * + * When this option is selected, any attempt to change the cleaning + * policy will fail. + * + * @note This option is meaningful only with ocf_mngt_cache_attach(). + * When used with ocf_mngt_cache_load() it's ignored. + */ + bool disable_cleaner; }; /** @@ -441,6 +456,7 @@ static inline void ocf_mngt_cache_attach_config_set_default( cfg->open_cores = true; cfg->force = false; cfg->discard_on_start = true; + cfg->disable_cleaner = false; cfg->device.perform_test = true; cfg->device.volume_params = NULL; } diff --git a/src/metadata/metadata.c b/src/metadata/metadata.c index 90cf478..5779aa2 100644 --- a/src/metadata/metadata.c +++ b/src/metadata/metadata.c @@ -638,7 +638,8 @@ static void ocf_metadata_flush_unlock_collision_page( * Initialize hash metadata interface */ int ocf_metadata_init_variable_size(struct ocf_cache *cache, - uint64_t device_size, ocf_cache_line_size_t line_size) + uint64_t device_size, ocf_cache_line_size_t line_size, + bool cleaner_disabled) { int result = 0; uint32_t i = 0; @@ -688,6 +689,12 @@ int ocf_metadata_init_variable_size(struct ocf_cache *cache, raw->raw_type = metadata_raw_type_atomic; } + if (i == metadata_segment_cleaning && cleaner_disabled) { + raw->entry_size = 0; + raw->entries_in_page = 1; + continue; + } + /* Entry size configuration */ raw->entry_size = ocf_metadata_get_element_size(i, line_size); @@ -715,6 +722,9 @@ int ocf_metadata_init_variable_size(struct ocf_cache *cache, */ for (i = metadata_segment_variable_size_start; i < metadata_segment_max; i++) { + if (i == metadata_segment_cleaning && cleaner_disabled) + continue; + if (i == metadata_segment_collision) { lock_page = ocf_metadata_flush_lock_collision_page; @@ -779,6 +789,7 @@ finalize: cache->conf_meta->cachelines = ctrl->cachelines; cache->conf_meta->line_size = line_size; + cache->conf_meta->cleaner_disabled = cleaner_disabled; ocf_metadata_raw_info(cache, ctrl); @@ -1620,6 +1631,7 @@ static void ocf_metadata_load_properties_cmpl( properties.shutdown_status = superblock->clean_shutdown; properties.dirty_flushed = superblock->dirty_flushed; properties.cache_name = superblock->name; + properties.cleaner_disabled = superblock->cleaner_disabled; OCF_CMPL_RET(priv, 0, &properties); } diff --git a/src/metadata/metadata.h b/src/metadata/metadata.h index d7d03ff..f26115d 100644 --- a/src/metadata/metadata.h +++ b/src/metadata/metadata.h @@ -41,10 +41,12 @@ int ocf_metadata_init(struct ocf_cache *cache, * @param cache - Cache instance * @param device_size - Device size in bytes * @param cache_line_size Cache line size + * @param cleaner_disabled Cleaner is disabled * @return 0 - Operation success otherwise failure */ int ocf_metadata_init_variable_size(struct ocf_cache *cache, - uint64_t device_size, ocf_cache_line_size_t cache_line_size); + uint64_t device_size, ocf_cache_line_size_t line_size, + bool cleaner_disabled); /** * @brief Initialize collision table @@ -201,6 +203,7 @@ struct ocf_metadata_load_properties { ocf_cache_mode_t cache_mode; ocf_cache_line_size_t line_size; char *cache_name; + bool cleaner_disabled; }; typedef void (*ocf_metadata_load_properties_end_t)(void *priv, int error, diff --git a/src/metadata/metadata_cleaning_policy.c b/src/metadata/metadata_cleaning_policy.c index 1f52943..3f9454e 100644 --- a/src/metadata/metadata_cleaning_policy.c +++ b/src/metadata/metadata_cleaning_policy.c @@ -18,6 +18,8 @@ ocf_metadata_get_cleaning_policy(struct ocf_cache *cache, struct ocf_metadata_ctrl *ctrl = (struct ocf_metadata_ctrl *) cache->metadata.priv; + ENV_BUG_ON(cache->conf_meta->cleaner_disabled); + return ocf_metadata_raw_wr_access(cache, &(ctrl->raw_desc[metadata_segment_cleaning]), line); } diff --git a/src/metadata/metadata_superblock.c b/src/metadata/metadata_superblock.c index 6a16611..f237971 100644 --- a/src/metadata/metadata_superblock.c +++ b/src/metadata/metadata_superblock.c @@ -110,8 +110,11 @@ static void ocf_metadata_store_segment(ocf_pipeline_t pipeline, int ocf_metadata_validate_superblock(ocf_ctx_t ctx, struct ocf_superblock_config *superblock) { + const char *segment_name; uint32_t crc; + segment_name = ocf_metadata_segment_names[metadata_segment_sb_config]; + if (superblock->magic_number != CACHE_MAGIC_NUMBER) { ocf_log(ctx, log_info, "Cannot detect pre-existing metadata\n"); return -OCF_ERR_NO_METADATA; @@ -178,6 +181,14 @@ int ocf_metadata_validate_superblock(ocf_ctx_t ctx, return -OCF_ERR_INVAL; } + if (superblock->cleaner_disabled && + superblock->cleaning_policy_type != ocf_cleaning_nop) { + ocf_log(ctx, log_err, "Loading %s: cleaner is disabled, but " + "cleaning policy is not set to nop!\n", + segment_name); + return -OCF_ERR_INVAL; + } + if (superblock->promotion_policy_type < 0 || superblock->promotion_policy_type >= ocf_promotion_max) { diff --git a/src/metadata/metadata_superblock.h b/src/metadata/metadata_superblock.h index e421760..e629157 100644 --- a/src/metadata/metadata_superblock.h +++ b/src/metadata/metadata_superblock.h @@ -45,6 +45,7 @@ struct ocf_superblock_config { unsigned long valid_core_bitmap[(OCF_CORE_MAX / (sizeof(unsigned long) * 8)) + 1]; + bool cleaner_disabled; ocf_cleaning_t cleaning_policy_type; struct cleaning_policy_config cleaning[CLEANING_POLICY_TYPE_MAX]; diff --git a/src/mngt/ocf_mngt_cache.c b/src/mngt/ocf_mngt_cache.c index 6da3835..cbb6466 100644 --- a/src/mngt/ocf_mngt_cache.c +++ b/src/mngt/ocf_mngt_cache.c @@ -162,6 +162,9 @@ struct ocf_cache_attach_context { uint8_t dirty_flushed; /*!< is dirty data fully flushed */ + + bool cleaner_disabled; + /*!< is cleaner disabled */ } metadata; struct { @@ -1117,6 +1120,7 @@ static void _ocf_mngt_load_read_properties_end(void *priv, int error, context->metadata.shutdown_status = properties->shutdown_status; context->metadata.dirty_flushed = properties->dirty_flushed; context->metadata.line_size = properties->line_size; + context->metadata.cleaner_disabled = properties->cleaner_disabled; cache->conf_meta->cache_mode = properties->cache_mode; ocf_pipeline_next(context->pipeline); @@ -1134,6 +1138,7 @@ static void _ocf_mngt_init_properties(ocf_pipeline_t pipeline, context->metadata.dirty_flushed = DIRTY_FLUSHED; context->metadata.line_size = context->cfg.cache_line_size ?: cache->metadata.line_size; + context->metadata.cleaner_disabled = context->cfg.disable_cleaner; ocf_pipeline_next(pipeline); } @@ -1172,7 +1177,8 @@ static void _ocf_mngt_attach_prepare_metadata(ocf_pipeline_t pipeline, * Initialize variable size metadata segments */ ret = ocf_metadata_init_variable_size(cache, context->volume_size, - context->metadata.line_size); + context->metadata.line_size, + context->metadata.cleaner_disabled); if (ret) OCF_PL_FINISH_RET(pipeline, ret); @@ -1227,6 +1233,9 @@ static void _ocf_mngt_attach_init_services(ocf_pipeline_t pipeline, ocf_cache_t cache = context->cache; ocf_error_t result; + if (context->metadata.cleaner_disabled) + __set_cleaning_policy(cache, ocf_cleaning_nop); + result = __init_cleaning_policy(cache); if (result) { ocf_cache_log(cache, log_err, @@ -2186,6 +2195,7 @@ static void _ocf_mngt_standby_init_properties(ocf_pipeline_t pipeline, context->metadata.dirty_flushed = DIRTY_FLUSHED; context->metadata.line_size = context->cfg.cache_line_size ?: cache->metadata.line_size; + context->metadata.cleaner_disabled = context->cfg.disable_cleaner; ocf_pipeline_next(pipeline); } diff --git a/src/mngt/ocf_mngt_flush.c b/src/mngt/ocf_mngt_flush.c index 8960c87..e45bf22 100644 --- a/src/mngt/ocf_mngt_flush.c +++ b/src/mngt/ocf_mngt_flush.c @@ -1043,6 +1043,9 @@ void ocf_mngt_cache_cleaning_set_policy(ocf_cache_t cache, OCF_CMPL_RET(priv, 0); } + if (cache->conf_meta->cleaner_disabled) + OCF_CMPL_RET(priv, -OCF_ERR_CLEANER_DISABLED); + ret = ocf_pipeline_create(&pipeline, cache, &_ocf_mngt_cache_set_cleaning_policy); if (ret) From 94aca1e8e4c8ffdc92b630f99fb4eaf36bee855b Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Fri, 22 Apr 2022 09:17:50 +0200 Subject: [PATCH 4/5] pyocf: Remove non-existing field from CacheAttachConfig This field has been moved to CacheDeviceConfig. Signed-off-by: Robert Baldyga --- tests/functional/pyocf/types/cache.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/pyocf/types/cache.py b/tests/functional/pyocf/types/cache.py index 0619ddb..451886b 100644 --- a/tests/functional/pyocf/types/cache.py +++ b/tests/functional/pyocf/types/cache.py @@ -76,7 +76,6 @@ class CacheAttachConfig(Structure): ("_open_cores", c_bool), ("_force", c_bool), ("_discard_on_start", c_bool), - ("_volume_params", c_void_p), ] From ed012411d3792e632d08344064e6ed680a40fc3f Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Fri, 22 Apr 2022 09:18:41 +0200 Subject: [PATCH 5/5] Add disable_cleaner field to CacheAttachConfig Signed-off-by: Robert Baldyga --- tests/functional/pyocf/types/cache.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/pyocf/types/cache.py b/tests/functional/pyocf/types/cache.py index 451886b..6f1b1d3 100644 --- a/tests/functional/pyocf/types/cache.py +++ b/tests/functional/pyocf/types/cache.py @@ -76,6 +76,7 @@ class CacheAttachConfig(Structure): ("_open_cores", c_bool), ("_force", c_bool), ("_discard_on_start", c_bool), + ("_disable_cleaner", c_bool) ]