From 2f1036508662c5965f277f351c280c4f96b5667a Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Tue, 21 Jan 2020 17:12:26 -0500 Subject: [PATCH 1/2] Flush metadata after setting dirty status of each sector. After second dirty write to cache line which was already dirty, metadata flush was not triggered. In case of dirty shutdown, this led to data corruption. Signed-off-by: Michal Mielewczyk --- src/metadata/metadata_status.h | 15 ++++++---- src/utils/utils_cache_line.c | 50 ++++++++++++++++++---------------- 2 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/metadata/metadata_status.h b/src/metadata/metadata_status.h index 375179b..3693cdb 100644 --- a/src/metadata/metadata_status.h +++ b/src/metadata/metadata_status.h @@ -229,20 +229,23 @@ static inline bool metadata_clear_dirty_sec_changed( /* * Marks given cache line's bits as dirty * - * @return true if the cache line was clean and became dirty - * @return false if the cache line was dirty before marking bits + * @return true if any cache line's sector became dirty + * @return false for other cases */ static inline bool metadata_set_dirty_sec_changed( struct ocf_cache *cache, ocf_cache_line_t line, - uint8_t start, uint8_t stop) + uint8_t start, uint8_t stop, bool *line_was_dirty) { - bool was_dirty; + bool sec_changed; OCF_METADATA_BITS_LOCK_WR(); - was_dirty = cache->metadata.iface.set_dirty(cache, line, start, stop); + sec_changed = !cache->metadata.iface.test_dirty(cache, line, + start, stop, true); + *line_was_dirty = cache->metadata.iface.set_dirty(cache, line, start, + stop); OCF_METADATA_BITS_UNLOCK_WR(); - return !was_dirty; + return sec_changed; } /******************************************************************************* diff --git a/src/utils/utils_cache_line.c b/src/utils/utils_cache_line.c index 5e693e5..d9bcc20 100644 --- a/src/utils/utils_cache_line.c +++ b/src/utils/utils_cache_line.c @@ -139,32 +139,36 @@ void set_cache_line_dirty(struct ocf_cache *cache, uint8_t start_bit, ocf_cache_line_t line = req->map[map_idx].coll_idx; ocf_part_id_t part_id = ocf_metadata_get_partition_id(cache, line); uint8_t evp_type = cache->conf_meta->eviction_policy_type; + bool line_was_dirty; - if (metadata_set_dirty_sec_changed(cache, line, start_bit, end_bit)) { - /* - * If this is first dirty cline set dirty timestamp - */ - env_atomic64_cmpxchg(&req->core->runtime_meta->dirty_since, - 0, env_get_tick_count()); - - /* - * Update the number of dirty cached data for that - * core object - */ - env_atomic_inc(&req->core->runtime_meta->dirty_clines); - - /* - * increment dirty clines statistic for given cline - */ - env_atomic_inc(&req->core->runtime_meta-> - part_counters[part_id].dirty_clines); - - if (likely(evict_policy_ops[evp_type].dirty_cline)) - evict_policy_ops[evp_type].dirty_cline(cache, part_id, line); - + if (metadata_set_dirty_sec_changed(cache, line, start_bit, end_bit, + &line_was_dirty)) { ocf_metadata_flush_mark(cache, req, map_idx, DIRTY, start_bit, - end_bit); + end_bit); + if (!line_was_dirty) { + /* + * If this is first dirty cline set dirty timestamp + */ + env_atomic64_cmpxchg(&req->core->runtime_meta->dirty_since, + 0, env_get_tick_count()); + + /* + * Update the number of dirty cached data for that + * core object + */ + env_atomic_inc(&req->core->runtime_meta->dirty_clines); + + /* + * increment dirty clines statistic for given cline + */ + env_atomic_inc(&req->core->runtime_meta-> + part_counters[part_id].dirty_clines); + + if (likely(evict_policy_ops[evp_type].dirty_cline)) + evict_policy_ops[evp_type].dirty_cline(cache, part_id, line); + } } + ocf_cleaning_set_hot_cache_line(cache, line); } From d9c987e0682223bbcf127df9a9f2e289f177d6a3 Mon Sep 17 00:00:00 2001 From: Michal Mielewczyk Date: Thu, 23 Jan 2020 03:41:31 -0500 Subject: [PATCH 2/2] Flush metadata after changing status of each sector In case of cleaning metadata used to be flushed only when status of whole cache line changed to clean. This patch ensures that metadata flush is triggered after changin status of each single sector is cache line. Signed-off-by: Michal Mielewczyk --- src/metadata/metadata_status.h | 21 +++++-------- src/utils/utils_cache_line.c | 57 ++++++++++++++++++---------------- 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/metadata/metadata_status.h b/src/metadata/metadata_status.h index 3693cdb..10faec8 100644 --- a/src/metadata/metadata_status.h +++ b/src/metadata/metadata_status.h @@ -200,30 +200,25 @@ static inline bool metadata_test_and_clear_dirty_sec( /* * Marks given cache line's bits as clean * - * @return true if the cache line was dirty and became clean + * @return true if any cache line's sector was dirty and became clean * @return false for other cases */ static inline bool metadata_clear_dirty_sec_changed( struct ocf_cache *cache, ocf_cache_line_t line, - uint8_t start, uint8_t stop) + uint8_t start, uint8_t stop, bool *line_is_clean) { - bool was_dirty, is_dirty = false; + bool sec_changed; OCF_METADATA_BITS_LOCK_WR(); - was_dirty = cache->metadata.iface.test_dirty(cache, line, - cache->metadata.settings.sector_start, - cache->metadata.settings.sector_end, - false); - - if (was_dirty) { - is_dirty = cache->metadata.iface.clear_dirty(cache, line, - start, stop); - } + sec_changed = cache->metadata.iface.test_dirty(cache, line, + start, stop, false); + *line_is_clean = !cache->metadata.iface.clear_dirty(cache, line, + start, stop); OCF_METADATA_BITS_UNLOCK_WR(); - return was_dirty && !is_dirty; + return sec_changed; } /* diff --git a/src/utils/utils_cache_line.c b/src/utils/utils_cache_line.c index d9bcc20..8b73471 100644 --- a/src/utils/utils_cache_line.c +++ b/src/utils/utils_cache_line.c @@ -102,35 +102,40 @@ void set_cache_line_clean(struct ocf_cache *cache, uint8_t start_bit, ocf_cache_line_t line = req->map[map_idx].coll_idx; ocf_part_id_t part_id = ocf_metadata_get_partition_id(cache, line); uint8_t evp_type = cache->conf_meta->eviction_policy_type; + bool line_is_clean; - if (metadata_clear_dirty_sec_changed(cache, line, start_bit, end_bit)) { - /* - * Update the number of dirty cached data for that - * core object - */ - if (env_atomic_dec_and_test(&req->core->runtime_meta-> - dirty_clines)) { - /* - * If this is last dirty cline reset dirty - * timestamp - */ - env_atomic64_set(&req->core->runtime_meta-> - dirty_since, 0); - } - - /* - * decrement dirty clines statistic for given cline - */ - env_atomic_dec(&req->core->runtime_meta-> - part_counters[part_id].dirty_clines); - - if (likely(evict_policy_ops[evp_type].clean_cline)) - evict_policy_ops[evp_type].clean_cline(cache, part_id, line); - - ocf_purge_cleaning_policy(cache, line); + if (metadata_clear_dirty_sec_changed(cache, line, start_bit, end_bit, + &line_is_clean)) { ocf_metadata_flush_mark(cache, req, map_idx, CLEAN, start_bit, - end_bit); + end_bit); + if (line_is_clean) { + /* + * Update the number of dirty cached data for that + * core object + */ + if (env_atomic_dec_and_test(&req->core->runtime_meta-> + dirty_clines)) { + /* + * If this is last dirty cline reset dirty + * timestamp + */ + env_atomic64_set(&req->core->runtime_meta-> + dirty_since, 0); + } + + /* + * decrement dirty clines statistic for given cline + */ + env_atomic_dec(&req->core->runtime_meta-> + part_counters[part_id].dirty_clines); + + if (likely(evict_policy_ops[evp_type].clean_cline)) + evict_policy_ops[evp_type].clean_cline(cache, part_id, line); + + ocf_purge_cleaning_policy(cache, line); + } } + } void set_cache_line_dirty(struct ocf_cache *cache, uint8_t start_bit,