From 56edd946e8b7a8c160e577180134756501182695 Mon Sep 17 00:00:00 2001 From: Artemy Kovalyov Date: Sun, 5 Nov 2023 18:52:17 +0200 Subject: [PATCH] UCS/RCACHE: Add support for dynamic region alignment - 4 --- src/ucs/memory/rcache.c | 182 +++++++++++++++++++--------------- test/gtest/ucs/test_rcache.cc | 2 +- 2 files changed, 101 insertions(+), 83 deletions(-) diff --git a/src/ucs/memory/rcache.c b/src/ucs/memory/rcache.c index 11f000cecdf0..20e5f9b90fd7 100644 --- a/src/ucs/memory/rcache.c +++ b/src/ucs/memory/rcache.c @@ -756,6 +756,83 @@ static void ucs_rcache_lru_evict(ucs_rcache_t *rcache) } } +/* Lock must be held */ +static ucs_status_t +ucs_rcache_check_overlap_one(ucs_rcache_t *rcache, ucs_pgt_addr_t *start, + ucs_pgt_addr_t *end, size_t *alignment, int *prot, + int lookup, ucs_rcache_region_t *region) +{ + int mem_prot; + + if ((*start >= region->super.start) && (*end <= region->super.end) && + ucs_rcache_region_test(region, *prot, *alignment) && lookup) { + /* Found a region which contains the given address range */ + ucs_rcache_region_hold(rcache, region); + return UCS_ERR_ALREADY_EXISTS; + } + + UCS_STATS_UPDATE_COUNTER(rcache->stats, UCS_RCACHE_MERGES, 1); + /* + * If we don't provide some of the permissions the other region had, + * we might want to expand our permissions to support them. We can + * do that only if the memory range actually has those permissions. + * This will prevent the users of the other region to kick us out + * the next time. + */ + if (!ucs_test_all_flags(*prot, region->prot)) { + /* A slow path because searching /proc/maps in order to + * check memory protection is very expensive. + * + * TODO: currently rcache is optimized for the case where most of + * the regions have same protection. + */ + mem_prot = UCS_PROFILE_CALL_ALWAYS(ucs_get_mem_prot, *start, *end); + if (!ucs_test_all_flags(mem_prot, *prot)) { + ucs_rcache_region_trace(rcache, region, + "do not merge "UCS_RCACHE_PROT_FMT + " with mem "UCS_RCACHE_PROT_FMT, + UCS_RCACHE_PROT_ARG(*prot), + UCS_RCACHE_PROT_ARG(mem_prot)); + /* The memory protection can not satisfy that of the + * region. However mem_reg still may be able to deal with it. + * Do the safest thing: invalidate cached region + */ + ucs_rcache_region_invalidate_internal( + rcache, region, UCS_RCACHE_REGION_PUT_FLAG_IN_PGTABLE); + return UCS_ERR_CANCELED; + } else if (ucs_test_all_flags(mem_prot, region->prot)) { + *prot |= region->prot; + } else { + /* Could not support other region's permissions - so do not merge + * with it. If anybody will use the other region, this will kick + * out our region, and may potentially lead to ineffective use + * of the cache. We can't solve it as long as we have only one + * page table, since it does not allow overlap. + */ + ucs_rcache_region_trace(rcache, region, + "do not merge mem "UCS_RCACHE_PROT_FMT" with", + UCS_RCACHE_PROT_ARG(mem_prot)); + ucs_rcache_region_invalidate_internal( + rcache, region, UCS_RCACHE_REGION_PUT_FLAG_IN_PGTABLE); + return UCS_ERR_CANCELED; + } + } + + ucs_rcache_region_trace(rcache, region, + "merge 0x%lx..0x%lx "UCS_RCACHE_PROT_FMT" with", + *start, *end, UCS_RCACHE_PROT_ARG(*prot)); + *alignment = ucs_max(*alignment, region->alignment); + *start = ucs_min(*start, region->super.start); + *end = ucs_max(*end, region->super.end); + + /* coverity[double_unlock] */ + /* coverity[double_lock] */ + ucs_rcache_region_invalidate_internal( + rcache, region, UCS_RCACHE_REGION_PUT_FLAG_IN_PGTABLE); + + return UCS_OK; +} + /* Lock must be held */ static ucs_status_t ucs_rcache_check_overlap(ucs_rcache_t *rcache, ucs_pgt_addr_t *start, @@ -766,7 +843,7 @@ ucs_rcache_check_overlap(ucs_rcache_t *rcache, ucs_pgt_addr_t *start, ucs_rcache_region_t *region, *tmp; ucs_pgt_addr_t old_start, old_end; ucs_list_link_t region_list; - int mem_prot; + ucs_status_t status; ucs_trace_func("rcache=%s, *start=0x%lx, *end=0x%lx", rcache->name, *start, *end); @@ -779,93 +856,34 @@ ucs_rcache_check_overlap(ucs_rcache_t *rcache, ucs_pgt_addr_t *start, ucs_rcache_find_regions(rcache, *start, *end - 1, ®ion_list); /* TODO check if any of the regions is locked */ -again: - old_start = *start; - old_end = *end; - - ucs_list_for_each_safe(region, tmp, ®ion_list, tmp_list) { - if ((*start >= region->super.start) && (*end <= region->super.end) && - ucs_rcache_region_test(region, *prot, *alignment) && lookup) { - /* Found a region which contains the given address range */ - ucs_rcache_region_hold(rcache, region); - *region_p = region; - return UCS_ERR_ALREADY_EXISTS; - } + do { + old_start = *start; + old_end = *end; + + ucs_list_for_each_safe(region, tmp, ®ion_list, tmp_list) { + status = ucs_rcache_check_overlap_one(rcache, start, end, alignment, + prot, lookup, region); + if (status == UCS_ERR_ALREADY_EXISTS) { + *region_p = region; + return status; + } - UCS_STATS_UPDATE_COUNTER(rcache->stats, UCS_RCACHE_MERGES, 1); - /* - * If we don't provide some of the permissions the other region had, - * we might want to expand our permissions to support them. We can - * do that only if the memory range actually has those permissions. - * This will prevent the users of the other region to kick us out - * the next time. - */ - if (!ucs_test_all_flags(*prot, region->prot)) { - /* A slow path because searching /proc/maps in order to - * check memory protection is very expensive. - * - * TODO: currently rcache is optimized for the case where most of - * the regions have same protection. - */ - mem_prot = UCS_PROFILE_CALL_ALWAYS(ucs_get_mem_prot, *start, *end); - if (!ucs_test_all_flags(mem_prot, *prot)) { - ucs_rcache_region_trace(rcache, region, - "do not merge "UCS_RCACHE_PROT_FMT - " with mem "UCS_RCACHE_PROT_FMT, - UCS_RCACHE_PROT_ARG(*prot), - UCS_RCACHE_PROT_ARG(mem_prot)); - /* The memory protection can not satisfy that of the - * region. However mem_reg still may be able to deal with it. - * Do the safest thing: invalidate cached region - */ - ucs_rcache_region_invalidate_internal( - rcache, region, UCS_RCACHE_REGION_PUT_FLAG_IN_PGTABLE); - continue; - } else if (ucs_test_all_flags(mem_prot, region->prot)) { - *prot |= region->prot; - } else { - /* Could not support other region's permissions - so do not merge - * with it. If anybody will use the other region, this will kick - * out our region, and may potentially lead to ineffective use - * of the cache. We can't solve it as long as we have only one - * page table, since it does not allow overlap. - */ - ucs_rcache_region_trace(rcache, region, - "do not merge mem "UCS_RCACHE_PROT_FMT" with", - UCS_RCACHE_PROT_ARG(mem_prot)); - ucs_rcache_region_invalidate_internal( - rcache, region, UCS_RCACHE_REGION_PUT_FLAG_IN_PGTABLE); - continue; + if (status == UCS_OK) { + *merged = 1; } } - ucs_rcache_region_trace(rcache, region, - "merge 0x%lx..0x%lx "UCS_RCACHE_PROT_FMT" with", - *start, *end, UCS_RCACHE_PROT_ARG(*prot)); - *alignment = ucs_max(*alignment, region->alignment); - *start = ucs_align_down_pow2(ucs_min(*start, region->super.start), - *alignment); - *end = ucs_align_up_pow2(ucs_max(*end, region->super.end), - *alignment); - *merged = 1; - - /* coverity[double_unlock] */ - /* coverity[double_lock] */ - ucs_rcache_region_invalidate_internal( - rcache, region, UCS_RCACHE_REGION_PUT_FLAG_IN_PGTABLE); - } + *start = ucs_align_down_pow2(*start, *alignment); + *end = ucs_align_up_pow2(*end, *alignment); - /* Overlapping regions might necessitate a larger alignment, which, - * in turn, can result in even more overlapping regions. - */ - ucs_list_head_init(®ion_list); - ucs_rcache_find_regions(rcache, *start, old_start - 1, ®ion_list); - ucs_rcache_find_regions(rcache, old_end, *end - 1, ®ion_list); - - if (!ucs_list_is_empty(®ion_list)) { + /* Overlapping regions might necessitate a larger alignment, which, + * in turn, can result in even more overlapping regions. + */ + ucs_list_head_init(®ion_list); + ucs_rcache_find_regions(rcache, *start, old_start - 1, ®ion_list); + ucs_rcache_find_regions(rcache, old_end, *end - 1, ®ion_list); lookup = 0; - goto again; - } + } while (!ucs_list_is_empty(®ion_list)); return UCS_OK; } diff --git a/test/gtest/ucs/test_rcache.cc b/test/gtest/ucs/test_rcache.cc index 41e76324d63c..a3662a8a5b9d 100644 --- a/test/gtest/ucs/test_rcache.cc +++ b/test/gtest/ucs/test_rcache.cc @@ -402,7 +402,7 @@ UCS_MT_TEST_F(test_rcache, merge, 6) { munmap(mem, size1 + pad + size2); } -UCS_MT_TEST_F(test_rcache, merge_aligned, 6) +UCS_TEST_F(test_rcache, merge_aligned) { /* * 0 4 5 9