Merge pull request #462 from arutk/no_bits_lock

Get rid of status bits lock

src/concurrency/ocf_metadata_concurrency.c

@@ -17,8 +17,6 @@ int ocf_metadata_concurrency_init(struct ocf_metadata_lock *metadata_lock)
 	for (evp_iter = 0; evp_iter < OCF_NUM_EVICTION_LISTS; evp_iter++)
 		env_rwlock_init(&metadata_lock->eviction[evp_iter]);
 
-	env_rwlock_init(&metadata_lock->status);
-
 	for (global_iter = 0; global_iter < OCF_NUM_GLOBAL_META_LOCKS;
 			global_iter++) {
 		err = env_rwsem_init(&metadata_lock->global[global_iter].sem);
@@ -42,8 +40,6 @@ global_err:
 	while (global_iter--)
 		env_rwsem_destroy(&metadata_lock->global[global_iter].sem);
 
-	env_rwlock_destroy(&metadata_lock->status);
-
 	while (evp_iter--)
 		env_rwlock_destroy(&metadata_lock->eviction[evp_iter]);
 
@@ -62,8 +58,6 @@ void ocf_metadata_concurrency_deinit(struct ocf_metadata_lock *metadata_lock)
 
 	for (i = 0; i < OCF_NUM_GLOBAL_META_LOCKS; i++)
 		env_rwsem_destroy(&metadata_lock->global[i].sem);
-
-	env_rwlock_destroy(&metadata_lock->status);
 }
 
 int ocf_metadata_concurrency_attached_init(

src/concurrency/ocf_metadata_concurrency.h

@@ -127,44 +127,6 @@ void ocf_metadata_end_shared_access(
 		struct ocf_metadata_lock *metadata_lock,
 		unsigned lock_idx);
 
-static inline void ocf_metadata_status_bits_lock(
-		struct ocf_metadata_lock *metadata_lock, int rw)
-{
-	if (rw == OCF_METADATA_WR)
-		env_rwlock_write_lock(&metadata_lock->status);
-	else if (rw == OCF_METADATA_RD)
-		env_rwlock_read_lock(&metadata_lock->status);
-	else
-		ENV_BUG();
-}
-
-static inline void ocf_metadata_status_bits_unlock(
-		struct ocf_metadata_lock *metadata_lock, int rw)
-{
-	if (rw == OCF_METADATA_WR)
-		env_rwlock_write_unlock(&metadata_lock->status);
-	else if (rw == OCF_METADATA_RD)
-		env_rwlock_read_unlock(&metadata_lock->status);
-	else
-		ENV_BUG();
-}
-
-#define OCF_METADATA_BITS_LOCK_RD() \
-		ocf_metadata_status_bits_lock(&cache->metadata.lock, \
-				OCF_METADATA_RD)
-
-#define OCF_METADATA_BITS_UNLOCK_RD() \
-		ocf_metadata_status_bits_unlock(&cache->metadata.lock, \
-				OCF_METADATA_RD)
-
-#define OCF_METADATA_BITS_LOCK_WR() \
-		ocf_metadata_status_bits_lock(&cache->metadata.lock, \
-				OCF_METADATA_WR)
-
-#define OCF_METADATA_BITS_UNLOCK_WR() \
-		ocf_metadata_status_bits_unlock(&cache->metadata.lock, \
-				OCF_METADATA_WR)
-
 void ocf_hb_cline_prot_lock_rd(struct ocf_metadata_lock *metadata_lock,
 		uint32_t lock_idx, uint32_t core_id, uint64_t core_line);
 void ocf_hb_cline_prot_unlock_rd(struct ocf_metadata_lock *metadata_lock,

src/engine/engine_rd.c

@@ -164,12 +164,12 @@ static int _ocf_read_generic_do(struct ocf_request *req)
 			return 0;
 		}
 
-		ocf_hb_req_prot_lock_rd(req);
+		ocf_hb_req_prot_lock_wr(req);
 
 		/* Set valid status bits map */
 		ocf_set_valid_map_info(req);
 
-		ocf_hb_req_prot_unlock_rd(req);
+		ocf_hb_req_prot_unlock_wr(req);
 	}
 
 	if (ocf_engine_needs_repart(req)) {

src/engine/engine_wb.c

@@ -27,22 +27,24 @@ static const struct ocf_io_if _io_if_wb_resume = {
 
 static void _ocf_write_wb_update_bits(struct ocf_request *req)
 {
-	if (ocf_engine_is_miss(req)) {
+	bool miss = ocf_engine_is_miss(req);
-		ocf_hb_req_prot_lock_rd(req);
+	bool clean_any = !ocf_engine_is_dirty_all(req);
+
+	if (!miss && !clean_any) {
+		ocf_req_set_cleaning_hot(req);
+		return;
+	}
+
+	ocf_hb_req_prot_lock_wr(req);
+	if (miss) {
 		/* Update valid status bits */
 		ocf_set_valid_map_info(req);
-
-		ocf_hb_req_prot_unlock_rd(req);
 	}
-
+	if (clean_any) {
-	if (!ocf_engine_is_dirty_all(req)) {
-		ocf_hb_req_prot_lock_wr(req);
-
 		/* set dirty bits, and mark if metadata flushing is required */
 		ocf_set_dirty_map_info(req);
-
-		ocf_hb_req_prot_unlock_wr(req);
 	}
+	ocf_hb_req_prot_unlock_wr(req);
 
 	ocf_req_set_cleaning_hot(req);
 }

src/engine/engine_wo.c

@@ -71,6 +71,10 @@ static int ocf_read_wo_cache_do(struct ocf_request *req)
 		s = ocf_map_line_start_sector(req, line);
 		e = ocf_map_line_end_sector(req, line);
 
+		ocf_hb_cline_prot_lock_rd(&cache->metadata.lock,
+				req->lock_idx, entry->core_id,
+				entry->core_line);
+
 		/* if cacheline mapping is not sequential, send cache IO to
 		 * previous cacheline(s) */
 		phys_prev = phys_curr;
@@ -112,6 +116,10 @@ static int ocf_read_wo_cache_do(struct ocf_request *req)
 						== valid);
 			}
 
+			ocf_hb_cline_prot_unlock_rd(&cache->metadata.lock,
+					req->lock_idx, entry->core_id,
+					entry->core_line);
+
 			if (io && !valid) {
 				/* end of sequential valid region */
 				ocf_read_wo_cache_io(req, io_start,
@@ -126,6 +134,12 @@ static int ocf_read_wo_cache_do(struct ocf_request *req)
 			}
 
 			offset += increment;
+
+			if (i <= e) {
+				ocf_hb_cline_prot_lock_rd(&cache->metadata.lock,
+					req->lock_idx, entry->core_id,
+					entry->core_line);
+			}
 		} while (i <= e);
 	}
 

src/engine/engine_wt.c

@@ -97,40 +97,37 @@ static inline void _ocf_write_wt_submit(struct ocf_request *req)
 
 static void _ocf_write_wt_update_bits(struct ocf_request *req)
 {
-	if (ocf_engine_is_miss(req)) {
+	bool miss = ocf_engine_is_miss(req);
-		ocf_hb_req_prot_lock_rd(req);
+	bool dirty_any = req->info.dirty_any;
+	bool repart = ocf_engine_needs_repart(req);
 
+	if (!miss && !dirty_any && !repart)
+		return;
+
+	ocf_hb_req_prot_lock_wr(req);
+
+	if (miss) {
 		/* Update valid status bits */
 		ocf_set_valid_map_info(req);
-
-		ocf_hb_req_prot_unlock_rd(req);
 	}
 
-	if (req->info.dirty_any) {
+	if (dirty_any) {
-		ocf_hb_req_prot_lock_wr(req);
+		/* Writes goes to both cache and core, need to update
-
+		 * status bits from dirty to clean
-		/* Writes goes to SDD and HDD, need to update status bits from
-		 * dirty to clean
 		 */
-
 		ocf_set_clean_map_info(req);
-
-		ocf_hb_req_prot_unlock_wr(req);
 	}
 
-	if (ocf_engine_needs_repart(req)) {
+	if (repart) {
 		OCF_DEBUG_RQ(req, "Re-Part");
-
-		ocf_hb_req_prot_lock_wr(req);
-
 		/* Probably some cache lines are assigned into wrong
 		 * partition. Need to move it to new one
 		 */
 		ocf_part_move(req);
+	}
 
 	ocf_hb_req_prot_unlock_wr(req);
 }
-}
 
 static int _ocf_write_wt_do(struct ocf_request *req)
 {

src/metadata/metadata_status.h

@@ -29,16 +29,12 @@ bool ocf_metadata_test_and_clear_valid(struct ocf_cache *cache, ocf_cache_line_t
 static inline void metadata_init_status_bits(struct ocf_cache *cache,
 		ocf_cache_line_t line)
 {
-	OCF_METADATA_BITS_LOCK_WR();
-
 	ocf_metadata_clear_dirty(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end);
 	ocf_metadata_clear_valid(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end);
-
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline bool metadata_test_dirty_all(struct ocf_cache *cache,
@@ -46,11 +42,9 @@ static inline bool metadata_test_dirty_all(struct ocf_cache *cache,
 {
 	bool test;
 
-	OCF_METADATA_BITS_LOCK_RD();
 	test = ocf_metadata_test_dirty(cache, line,
 		cache->metadata.settings.sector_start,
 		cache->metadata.settings.sector_end, true);
-	OCF_METADATA_BITS_UNLOCK_RD();
 
 	return test;
 }
@@ -60,11 +54,9 @@ static inline bool metadata_test_dirty(struct ocf_cache *cache,
 {
 	bool test;
 
-	OCF_METADATA_BITS_LOCK_RD();
 	test = ocf_metadata_test_dirty(cache, line,
 		cache->metadata.settings.sector_start,
 		cache->metadata.settings.sector_end, false);
-	OCF_METADATA_BITS_UNLOCK_RD();
 
 	return test;
 }
@@ -72,49 +64,33 @@ static inline bool metadata_test_dirty(struct ocf_cache *cache,
 static inline void metadata_set_dirty(struct ocf_cache *cache,
 		ocf_cache_line_t line)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_set_dirty(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline void metadata_clear_dirty(struct ocf_cache *cache,
 		ocf_cache_line_t line)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_clear_dirty(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline bool metadata_test_and_clear_dirty(
 		struct ocf_cache *cache, ocf_cache_line_t line)
 {
-	bool test;
+	return ocf_metadata_test_and_clear_dirty(cache, line,
-
-	OCF_METADATA_BITS_LOCK_WR();
-	test =	ocf_metadata_test_and_clear_dirty(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end, false);
-	OCF_METADATA_BITS_UNLOCK_WR();
-
-	return test;
 }
 
 static inline bool metadata_test_and_set_dirty(struct ocf_cache *cache,
 		ocf_cache_line_t line)
 {
-	bool test;
+	return ocf_metadata_test_and_set_dirty(cache, line,
-
-	OCF_METADATA_BITS_LOCK_WR();
-	test =	ocf_metadata_test_and_set_dirty(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end, false);
-	OCF_METADATA_BITS_UNLOCK_WR();
-
-	return test;
 }
 
 /*******************************************************************************
@@ -124,27 +100,15 @@ static inline bool metadata_test_and_set_dirty(struct ocf_cache *cache,
 static inline bool metadata_test_dirty_sec(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t start, uint8_t stop)
 {
-	bool test;
+	return ocf_metadata_test_dirty(cache, line,
-
-	OCF_METADATA_BITS_LOCK_RD();
-	test = ocf_metadata_test_dirty(cache, line,
 			start, stop, false);
-	OCF_METADATA_BITS_UNLOCK_RD();
-
-	return test;
 }
 
 static inline bool metadata_test_dirty_all_sec(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t start, uint8_t stop)
 {
-	bool test;
+	return ocf_metadata_test_dirty(cache, line,
-
-	OCF_METADATA_BITS_LOCK_RD();
-	test = ocf_metadata_test_dirty(cache, line,
 			start, stop, true);
-	OCF_METADATA_BITS_UNLOCK_RD();
-
-	return test;
 }
 
 static inline bool metadata_test_dirty_one(struct ocf_cache *cache,
@@ -156,59 +120,39 @@ static inline bool metadata_test_dirty_one(struct ocf_cache *cache,
 static inline bool metadata_test_dirty_out_sec(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t start, uint8_t stop)
 {
-	bool test;
+	return ocf_metadata_test_out_dirty(cache, line, start, stop);
-
-	OCF_METADATA_BITS_LOCK_RD();
-	test = ocf_metadata_test_out_dirty(cache, line, start, stop);
-	OCF_METADATA_BITS_UNLOCK_RD();
-
-	return test;
 }
 
 static inline void metadata_set_dirty_sec(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t start, uint8_t stop)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_set_dirty(cache, line, start, stop);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline void metadata_clear_dirty_sec(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t start, uint8_t stop)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_clear_dirty(cache, line, start, stop);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline void metadata_set_dirty_sec_one(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t pos)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_set_dirty(cache, line, pos, pos);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline void metadata_clear_dirty_sec_one(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t pos)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_clear_dirty(cache, line, pos, pos);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline bool metadata_test_and_clear_dirty_sec(
 		struct ocf_cache *cache, ocf_cache_line_t line,
 		uint8_t start, uint8_t stop)
 {
-	bool test = false;
+	return ocf_metadata_test_and_clear_dirty(cache, line,
-
-	OCF_METADATA_BITS_LOCK_WR();
-	test = ocf_metadata_test_and_clear_dirty(cache, line,
 			start, stop, false);
-	OCF_METADATA_BITS_UNLOCK_WR();
-
-	return test;
 }
 
 /*
@@ -223,15 +167,11 @@ static inline bool metadata_clear_dirty_sec_changed(
 {
 	bool sec_changed;
 
-	OCF_METADATA_BITS_LOCK_WR();
-
 	sec_changed = ocf_metadata_test_dirty(cache, line,
 			start, stop, false);
 	*line_is_clean = !ocf_metadata_clear_dirty(cache, line,
 			start, stop);
 
-	OCF_METADATA_BITS_UNLOCK_WR();
-
 	return sec_changed;
 }
 
@@ -247,12 +187,10 @@ static inline bool metadata_set_dirty_sec_changed(
 {
 	bool sec_changed;
 
-	OCF_METADATA_BITS_LOCK_WR();
 	sec_changed = !ocf_metadata_test_dirty(cache, line,
 			start, stop, true);
 	*line_was_dirty = ocf_metadata_set_dirty(cache, line, start,
 			stop);
-	OCF_METADATA_BITS_UNLOCK_WR();
 
 	return sec_changed;
 }
@@ -264,77 +202,49 @@ static inline bool metadata_set_dirty_sec_changed(
 static inline bool metadata_test_valid_any(struct ocf_cache *cache,
 		ocf_cache_line_t line)
 {
-	bool test;
+	return ocf_metadata_test_valid(cache, line,
-
-	OCF_METADATA_BITS_LOCK_RD();
-	test = ocf_metadata_test_valid(cache, line,
 		cache->metadata.settings.sector_start,
 		cache->metadata.settings.sector_end, false);
-	OCF_METADATA_BITS_UNLOCK_RD();
-
-	return test;
 }
 
 static inline bool metadata_test_valid(struct ocf_cache *cache,
 		ocf_cache_line_t line)
 {
-	bool test;
+	return ocf_metadata_test_valid(cache, line,
-
-	OCF_METADATA_BITS_LOCK_RD();
-	test = ocf_metadata_test_valid(cache, line,
 		cache->metadata.settings.sector_start,
 		cache->metadata.settings.sector_end, true);
-	OCF_METADATA_BITS_UNLOCK_RD();
-
-	return test;
 }
 
 static inline void metadata_set_valid(struct ocf_cache *cache,
 		ocf_cache_line_t line)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_set_valid(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline void metadata_clear_valid(struct ocf_cache *cache,
 		ocf_cache_line_t line)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_clear_valid(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline bool metadata_test_and_clear_valid(
 		struct ocf_cache *cache, ocf_cache_line_t line)
 {
-	bool test = false;
+	return ocf_metadata_test_and_clear_valid(cache, line,
-
-	OCF_METADATA_BITS_LOCK_WR();
-	test =	ocf_metadata_test_and_clear_valid(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end, true);
-	OCF_METADATA_BITS_UNLOCK_WR();
-
-	return test;
 }
 
 static inline bool metadata_test_and_set_valid(struct ocf_cache *cache,
 		ocf_cache_line_t line)
 {
-	bool test = false;
+	return ocf_metadata_test_and_set_valid(cache, line,
-
-	OCF_METADATA_BITS_LOCK_WR();
-	test =	ocf_metadata_test_and_set_valid(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end, true);
-	OCF_METADATA_BITS_UNLOCK_WR();
-
-	return test;
 }
 
 /*******************************************************************************
@@ -344,28 +254,16 @@ static inline bool metadata_test_and_set_valid(struct ocf_cache *cache,
 static inline bool metadata_test_valid_sec(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t start, uint8_t stop)
 {
-	bool test;
+	return ocf_metadata_test_valid(cache, line,
-
-	OCF_METADATA_BITS_LOCK_RD();
-	test = ocf_metadata_test_valid(cache, line,
 			start, stop, true);
-	OCF_METADATA_BITS_UNLOCK_RD();
-
-	return test;
 }
 
 static inline bool metadata_test_valid_any_out_sec(
 		struct ocf_cache *cache, ocf_cache_line_t line,
 		uint8_t start, uint8_t stop)
 {
-	bool test = false;
+	return ocf_metadata_test_out_valid(cache, line,
-
-	OCF_METADATA_BITS_LOCK_RD();
-	test = ocf_metadata_test_out_valid(cache, line,
 			start, stop);
-	OCF_METADATA_BITS_UNLOCK_RD();
-
-	return test;
 }
 
 static inline bool metadata_test_valid_one(struct ocf_cache *cache,
@@ -385,38 +283,26 @@ static inline bool metadata_set_valid_sec_changed(
 		struct ocf_cache *cache, ocf_cache_line_t line,
 		uint8_t start, uint8_t stop)
 {
-	bool was_any_valid;
+	return !ocf_metadata_set_valid(cache, line,
-
-	OCF_METADATA_BITS_LOCK_WR();
-	was_any_valid = ocf_metadata_set_valid(cache, line,
 			start, stop);
-	OCF_METADATA_BITS_UNLOCK_WR();
-
-	return !was_any_valid;
 }
 
 static inline void metadata_clear_valid_sec(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t start, uint8_t stop)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_clear_valid(cache, line, start, stop);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline void metadata_clear_valid_sec_one(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t pos)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_clear_valid(cache, line, pos, pos);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 
 static inline void metadata_set_valid_sec_one(struct ocf_cache *cache,
 		ocf_cache_line_t line, uint8_t pos)
 {
-	OCF_METADATA_BITS_LOCK_WR();
 	ocf_metadata_set_valid(cache, line, pos, pos);
-	OCF_METADATA_BITS_UNLOCK_WR();
 }
 /*
  * Marks given cache line's bits as invalid
@@ -431,8 +317,6 @@ static inline bool metadata_clear_valid_sec_changed(
 {
 	bool was_any_valid;
 
-	OCF_METADATA_BITS_LOCK_WR();
-
 	was_any_valid = ocf_metadata_test_valid(cache, line,
 			cache->metadata.settings.sector_start,
 			cache->metadata.settings.sector_end, false);
@@ -440,8 +324,6 @@ static inline bool metadata_clear_valid_sec_changed(
 	*is_valid = ocf_metadata_clear_valid(cache, line,
 			start, stop);
 
-	OCF_METADATA_BITS_UNLOCK_WR();
-
 	return was_any_valid && !*is_valid;
 }
 

src/metadata/metadata_structs.h

@@ -55,7 +55,6 @@ struct ocf_metadata_lock
 {
 	struct ocf_metadata_global_lock global[OCF_NUM_GLOBAL_META_LOCKS];
 			/*!< global metadata lock (GML) */
-	env_rwlock status; /*!< Fast lock for status bits */
 	env_rwlock eviction[OCF_NUM_EVICTION_LISTS]; /*!< Fast lock for eviction policy */
 	env_rwsem *hash; /*!< Hash bucket locks */
 	env_rwsem *collision_pages; /*!< Collision table page locks */