From dfb2e1a8d5214d8a0fc5a8b0a92217769e316f1a Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Fri, 14 Jun 2024 16:43:49 +0200 Subject: [PATCH] cleaner: Check mapping after taking cache line lock Signed-off-by: Robert Baldyga --- src/cleaning/acp.c | 1 - src/cleaning/alru.c | 1 - src/engine/engine_common.c | 2 +- src/mngt/ocf_mngt_flush.c | 1 - src/ocf_lru.c | 1 - src/utils/utils_cleaner.c | 76 +++++++++++++++++--------------------- src/utils/utils_cleaner.h | 1 - 7 files changed, 35 insertions(+), 48 deletions(-) diff --git a/src/cleaning/acp.c b/src/cleaning/acp.c index fc56a3a..2672562 100644 --- a/src/cleaning/acp.c +++ b/src/cleaning/acp.c @@ -651,7 +651,6 @@ static void _acp_flush(struct acp_context *acp) .cmpl_context = acp, .cmpl_fn = _acp_flush_end, .lock_cacheline = false, - .lock_metadata = true, .cmpl_queue = true, .io_queue = cache->cleaner.io_queue, }; diff --git a/src/cleaning/alru.c b/src/cleaning/alru.c index 5f27edb..6bc6f4e 100644 --- a/src/cleaning/alru.c +++ b/src/cleaning/alru.c @@ -940,7 +940,6 @@ void cleaning_alru_perform_cleaning(ocf_cache_t cache, ocf_cleaner_end_t cmpl) fctx->attribs.cmpl_context = fctx; fctx->attribs.cmpl_fn = alru_clean_complete; fctx->attribs.lock_cacheline = true; - fctx->attribs.lock_metadata = false; fctx->attribs.io_queue = cache->cleaner.io_queue; fctx->attribs.cmpl_queue = true; diff --git a/src/engine/engine_common.c b/src/engine/engine_common.c index b54c865..24643f6 100644 --- a/src/engine/engine_common.c +++ b/src/engine/engine_common.c @@ -1,5 +1,6 @@ /* * Copyright(c) 2012-2022 Intel Corporation + * Copyright(c) 2024 Huawei Technologies Co., Ltd. * SPDX-License-Identifier: BSD-3-Clause */ @@ -555,7 +556,6 @@ void ocf_engine_clean(struct ocf_request *req) /* Initialize attributes for cleaner */ struct ocf_cleaner_attribs attribs = { .lock_cacheline = false, - .lock_metadata = false, .cmpl_context = req, .cmpl_fn = _ocf_engine_clean_end, diff --git a/src/mngt/ocf_mngt_flush.c b/src/mngt/ocf_mngt_flush.c index e9d225e..2cfc565 100644 --- a/src/mngt/ocf_mngt_flush.c +++ b/src/mngt/ocf_mngt_flush.c @@ -444,7 +444,6 @@ static void _ocf_mngt_flush_container( fc->req = req; fc->attribs.lock_cacheline = true; - fc->attribs.lock_metadata = false; fc->attribs.cmpl_context = fc; fc->attribs.cmpl_fn = _ocf_mngt_flush_portion_end; fc->attribs.io_queue = cache->mngt_queue; diff --git a/src/ocf_lru.c b/src/ocf_lru.c index 69eed2c..16eaa74 100644 --- a/src/ocf_lru.c +++ b/src/ocf_lru.c @@ -591,7 +591,6 @@ void ocf_lru_clean(ocf_cache_t cache, struct ocf_user_part *user_part, struct ocf_part_cleaning_ctx *ctx = &user_part->cleaning; struct ocf_cleaner_attribs attribs = { .lock_cacheline = false, - .lock_metadata = true, .cmpl_context = ctx, .cmpl_fn = ocf_lru_clean_end, diff --git a/src/utils/utils_cleaner.c b/src/utils/utils_cleaner.c index 46aa80c..4ca79cb 100644 --- a/src/utils/utils_cleaner.c +++ b/src/utils/utils_cleaner.c @@ -339,7 +339,7 @@ static int _ocf_cleaner_update_metadata(struct ocf_request *req) /* Update metadata */ for (i = 0; i < req->core_line_count; i++, iter++) { - if (iter->status == LOOKUP_MISS) + if (!iter->flush) continue; if (iter->invalid) { @@ -384,7 +384,7 @@ static void _ocf_cleaner_flush_cores_io_end(struct ocf_map_info *map, if (error) { /* Flush error, set error for all cache line of this core */ for (i = 0; i < req->core_line_count; i++, iter++) { - if (iter->status == LOOKUP_MISS) + if (!iter->flush) continue; if (iter->core_id == map->core_id) @@ -434,7 +434,7 @@ static int _ocf_cleaner_fire_flush_cores(struct ocf_request *req) continue; } - if (iter->status == LOOKUP_MISS) + if (!iter->flush) continue; if (core_id == iter->core_id) @@ -603,7 +603,7 @@ static int _ocf_cleaner_fire_core(struct ocf_request *req) continue; } - if (iter->status == LOOKUP_MISS) + if (!iter->flush) continue; ocf_hb_cline_prot_lock_rd(&cache->metadata.lock, @@ -677,7 +677,7 @@ static int _ocf_cleaner_fire_cache(struct ocf_request *req) core = ocf_cache_get_core(cache, iter->core_id); if (!core) continue; - if (iter->status == LOOKUP_MISS) + if (!iter->flush) continue; OCF_DEBUG_PARAM(req->cache, "Cache read, line = %u", @@ -722,6 +722,33 @@ static int _ocf_cleaner_fire_cache(struct ocf_request *req) return 0; } +static int _ocf_cleaner_check_map(struct ocf_request *req) +{ + ocf_core_id_t core_id; + uint64_t core_line; + int i; + + for (i = 0; i < req->core_line_count; ++i) { + ocf_metadata_get_core_info(req->cache, req->map[i].coll_idx, + &core_id, &core_line); + + if (core_id != req->map[i].core_id) + continue; + + if (core_line != req->map[i].core_line) + continue; + + if (!metadata_test_dirty(req->cache, req->map[i].coll_idx)) + continue; + + req->map[i].flush = true; + } + + _ocf_cleaner_fire_cache(req); + + return 0; +} + static int _ocf_cleaner_do_fire(struct ocf_request *req, uint32_t count) { int result; @@ -729,7 +756,7 @@ static int _ocf_cleaner_do_fire(struct ocf_request *req, uint32_t count) /* Set counts of cache IOs */ env_atomic_set(&req->req_remaining, count); - req->engine_handler = _ocf_cleaner_fire_cache; + req->engine_handler = _ocf_cleaner_check_map; req->core_line_count = count; /* Handle cache lines locks */ @@ -738,7 +765,7 @@ static int _ocf_cleaner_do_fire(struct ocf_request *req, uint32_t count) if (result >= 0) { if (result == OCF_LOCK_ACQUIRED) { OCF_DEBUG_MSG(req->cache, "Lock acquired"); - _ocf_cleaner_fire_cache(req); + _ocf_cleaner_check_map(req); } else { OCF_DEBUG_MSG(req->cache, "NO Lock"); } @@ -793,7 +820,6 @@ void ocf_cleaner_fire(struct ocf_cache *cache, int err; ocf_core_id_t core_id; uint64_t core_sector; - bool skip; /* Allocate master request */ master = _ocf_cleaner_alloc_master_req(cache, max, attribs); @@ -850,40 +876,6 @@ void ocf_cleaner_fire(struct ocf_cache *cache, ocf_metadata_get_core_info(cache, cache_line, &core_id, &core_sector); - if (attribs->lock_metadata) { - ocf_hb_cline_prot_lock_rd(&cache->metadata.lock, - req->lock_idx, core_id, core_sector); - } - - skip = false; - - /* when line already cleaned - rare condition under heavy - * I/O workload. - */ - if (!metadata_test_dirty(cache, cache_line)) { - OCF_DEBUG_MSG(cache, "Not dirty"); - skip = true; - } - - if (!skip && !metadata_test_valid_any(cache, cache_line)) { - OCF_DEBUG_MSG(cache, "No any valid"); - - /* - * Extremely disturbing cache line state - * Cache line (sector) cannot be dirty and not valid - */ - ENV_BUG(); - skip = true; - } - - if (attribs->lock_metadata) { - ocf_hb_cline_prot_unlock_rd(&cache->metadata.lock, - req->lock_idx, core_id, core_sector); - } - - if (skip) - continue; - if (unlikely(!cache->core[core_id].opened)) { OCF_DEBUG_MSG(cache, "Core object inactive"); continue; diff --git a/src/utils/utils_cleaner.h b/src/utils/utils_cleaner.h index da85909..7c90fd5 100644 --- a/src/utils/utils_cleaner.h +++ b/src/utils/utils_cleaner.h @@ -28,7 +28,6 @@ typedef int (*ocf_cleaner_get_item)(struct ocf_cache *cache, */ struct ocf_cleaner_attribs { uint8_t lock_cacheline : 1; /*!< Cleaner to lock cachelines on its own */ - uint8_t lock_metadata : 1; /*!< Cleaner to lock metadata on its own */ uint8_t cmpl_queue : 1; /*!< Completion needs to be called from the queue context */