Merge pull request #220 from arutk/metadata_offset_hash

New hash function formula

src/engine/engine_common.c

@@ -38,19 +38,19 @@ void ocf_engine_lookup_map_entry(struct ocf_cache *cache,
 		uint64_t core_line)
 {
 	ocf_cache_line_t line;
-	ocf_cache_line_t hash_key;
+	ocf_cache_line_t hash;
 
-	hash_key = ocf_metadata_hash_func(cache, core_line, core_id);
+	hash = ocf_metadata_hash_func(cache, core_line, core_id);
 
 	/* Initially assume that we have cache miss.
 	 * Hash points to proper bucket.
 	 */
-	entry->hash_key = hash_key;
+	entry->hash = hash;
 	entry->status = LOOKUP_MISS;
 	entry->coll_idx = cache->device->collision_table_entries;
 	entry->core_line = core_line;
 
-	line = ocf_metadata_get_hash(cache, hash_key);
+	line = ocf_metadata_get_hash(cache, hash);
 
 	while (line != cache->device->collision_table_entries) {
 		ocf_core_id_t curr_core_id;
@@ -347,7 +347,7 @@ void ocf_engine_map(struct ocf_request *req)
 
 		if (entry->status != LOOKUP_HIT) {
 			ocf_engine_map_cache_line(req, entry->core_line,
-					entry->hash_key, &entry->coll_idx);
+					entry->hash, &entry->coll_idx);
 
 			if (req->info.mapping_error) {
 				/*

src/metadata/metadata_hash.c

@@ -83,8 +83,7 @@ static ocf_cache_line_t ocf_metadata_hash_get_entries(
 		return cache_lines;
 
 	case metadata_segment_hash:
-		return OCF_DIV_ROUND_UP(cache_lines / 4, OCF_HASH_PRIME) *
-				OCF_HASH_PRIME - 1;
+		return OCF_DIV_ROUND_UP(cache_lines, 4);
 
 	case metadata_segment_sb_config:
 		return OCF_DIV_ROUND_UP(sizeof(struct ocf_superblock_config),

src/metadata/metadata_misc.h

@@ -6,21 +6,19 @@
 #ifndef __METADATA_MISC_H__
 #define __METADATA_MISC_H__
 
-/*
- * Hash function needs number that has no common factors with both number
- * of cores and number of entries in metadata hash container (hash lists).
- * This can be easily achived by picking prime which is bigger than maximum
- * number of cores and ensuring that count of hash table entries is not
- * divisible by this number. Let's choose 4099, which is smallest prime
- * greater than OCF_CORE_MAX (which is 4096).
+/* Hash function intentionally returns consecutive (modulo @hash_table_entries)
+ * values for consecutive @core_line_num. This way it is trivial to sort all
+ * core lines within a single request in ascending hash value order. This kind
+ * of sorting is required to assure that (future) hash bucket metadata locks are
+ * always acquired in fixed order, eliminating the risk of dead locks.
  */
-#define OCF_HASH_PRIME 4099
-
 static inline ocf_cache_line_t ocf_metadata_hash_func(ocf_cache_t cache,
 		uint64_t core_line_num, ocf_core_id_t core_id)
 {
-	return (ocf_cache_line_t) ((core_line_num * OCF_HASH_PRIME + core_id) %
-			cache->device->hash_table_entries);
+	const unsigned int entries = cache->device->hash_table_entries;
+
+	return (ocf_cache_line_t) ((core_line_num  + (core_id * (entries / 32)))
+			% entries);
 }
 
 void ocf_metadata_sparse_cache_line(struct ocf_cache *cache,

src/ocf_request.h

@@ -50,9 +50,11 @@ struct ocf_req_info {
 };
 
 struct ocf_map_info {
-	/* If HIT -> pointer to hash_key and coll_idx */
-	unsigned int hash_key;
-	unsigned int coll_idx;
+	ocf_cache_line_t hash;
+	/*!< target LBA & core id hash */
+
+	ocf_cache_line_t coll_idx;
+	/*!< Index in collision table (in case of hit) */
 
 	uint64_t core_line;
 

src/utils/utils_cleaner.c

@@ -489,7 +489,7 @@ static void _ocf_cleaner_core_io_for_dirty_range(struct ocf_request *req,
 
 	addr = (ocf_line_size(cache) * iter->core_line)
 			+ SECTORS_TO_BYTES(begin);
-	offset = (ocf_line_size(cache) * iter->hash_key)
+	offset = (ocf_line_size(cache) * iter->hash)
 			+ SECTORS_TO_BYTES(begin);
 
 	io = ocf_new_core_io(core, req->io_queue, addr,
@@ -666,7 +666,7 @@ static int _ocf_cleaner_fire_cache(struct ocf_request *req)
 		addr *= ocf_line_size(cache);
 		addr += cache->device->metadata_offset;
 
-		offset = ocf_line_size(cache) * iter->hash_key;
+		offset = ocf_line_size(cache) * iter->hash;
 
 		part_id = ocf_metadata_get_partition_id(cache, iter->coll_idx);
 
@@ -773,7 +773,7 @@ static int _ocf_cleaner_do_fire(struct ocf_request *req,  uint32_t i_out,
 		req->map[i_out].core_id = OCF_CORE_MAX;
 		req->map[i_out].core_line = ULLONG_MAX;
 		req->map[i_out].status = LOOKUP_MISS;
-		req->map[i_out].hash_key = i_out;
+		req->map[i_out].hash = i_out;
 	}
 
 	if (do_sort) {
@@ -781,7 +781,7 @@ static int _ocf_cleaner_do_fire(struct ocf_request *req,  uint32_t i_out,
 		env_sort(req->map, req->core_line_count, sizeof(req->map[0]),
 			_ocf_cleaner_cmp_private, NULL);
 		for (i = 0; i < req->core_line_count; i++)
-			req->map[i].hash_key = i;
+			req->map[i].hash = i;
 	}
 
 	/* issue actual request */
@@ -916,7 +916,7 @@ void ocf_cleaner_fire(struct ocf_cache *cache,
 		req->map[i_out].core_line = core_sector;
 		req->map[i_out].coll_idx = cache_line;
 		req->map[i_out].status = LOOKUP_HIT;
-		req->map[i_out].hash_key = i_out;
+		req->map[i_out].hash = i_out;
 		i_out++;
 
 		if (max == i_out) {

tests/unit/tests/metadata/metadata_collision.c/metadata_collision.c

@@ -0,0 +1,65 @@
+/*
+ * <tested_file_path>src/metadata/metadata_collision.c</tested_file_path>
+ * <tested_function>ocf_metadata_hash_func</tested_function>
+ * <functions_to_leave>
+ * 	ocf_metadata_get_hash
+ * </functions_to_leave>
+ */
+
+#undef static
+
+#undef inline
+
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include "print_desc.h"
+
+#include "ocf/ocf.h"
+#include "metadata.h"
+#include "../utils/utils_cache_line.h"
+
+#include "metadata/metadata_collision.c/metadata_collision_generated_warps.c"
+
+static void metadata_hash_func_test01(void **state)
+{
+	struct ocf_cache cache;
+	bool wrap = false;
+	ocf_cache_line_t i;
+	ocf_cache_line_t hash_cur, hash_next;
+	unsigned c;
+	ocf_core_id_t core_ids[] = {0, 1, 2, 100, OCF_CORE_MAX};
+	ocf_core_id_t core_id;
+
+	print_test_description("Verify that hash function increments by 1 and generates"
+				"collision after 'hash_table_entries' successive core lines");
+
+	cache.device = malloc(sizeof(*cache.device));
+	cache.device->hash_table_entries = 10;
+
+	for (c = 0; c < sizeof(core_ids) / sizeof(core_ids[0]); c++) {
+		core_id = core_ids[c];
+		for (i = 0; i < cache.device->hash_table_entries + 1; i++) {
+			hash_cur = ocf_metadata_hash_func(&cache, i, core_id);
+			hash_next = ocf_metadata_hash_func(&cache, i + 1, core_id);
+			/* make sure hash value is within expected range */
+			assert(hash_cur < cache.device->hash_table_entries);
+			assert(hash_next < cache.device->hash_table_entries);
+			/* hash should either increment by 1 or overflow to 0 */
+			assert(hash_next == (hash_cur + 1) % cache.device->hash_table_entries);
+		}
+	}
+}
+
+int main(void)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(metadata_hash_func_test01)
+	};
+
+	print_message("Unit test of src/metadata/metadata_collision.c");
+
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}