diff --git a/src/engine/engine_common.c b/src/engine/engine_common.c index e14b1b6..53a5fcc 100644 --- a/src/engine/engine_common.c +++ b/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) { /* diff --git a/src/metadata/metadata_hash.c b/src/metadata/metadata_hash.c index ee170c7..41dfe22 100644 --- a/src/metadata/metadata_hash.c +++ b/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), diff --git a/src/metadata/metadata_misc.h b/src/metadata/metadata_misc.h index 74ec801..51cdad7 100644 --- a/src/metadata/metadata_misc.h +++ b/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, diff --git a/src/ocf_request.h b/src/ocf_request.h index c87e69d..1fa924b 100644 --- a/src/ocf_request.h +++ b/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; diff --git a/src/utils/utils_cleaner.c b/src/utils/utils_cleaner.c index e9a2f08..ad9c458 100644 --- a/src/utils/utils_cleaner.c +++ b/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) { diff --git a/tests/unit/tests/metadata/metadata_collision.c/metadata_collision.c b/tests/unit/tests/metadata/metadata_collision.c/metadata_collision.c new file mode 100644 index 0000000..a07dce4 --- /dev/null +++ b/tests/unit/tests/metadata/metadata_collision.c/metadata_collision.c @@ -0,0 +1,65 @@ +/* + * src/metadata/metadata_collision.c + * ocf_metadata_hash_func + * + * ocf_metadata_get_hash + * + */ + +#undef static + +#undef inline + + +#include +#include +#include +#include +#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); +}