cleaner: Check mapping after taking cache line lock

Signed-off-by: Robert Baldyga <robert.baldyga@huawei.com>

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,
 	};

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;
 

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,

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;

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,

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;

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 */