From bbe80639d4c425cfa02d4a35c1cafbfb293a110d Mon Sep 17 00:00:00 2001 From: Aditya Gurajada Date: Wed, 21 Dec 2022 16:32:08 -0800 Subject: [PATCH] (#507) Rework of platform_buffer_create()/destroy() to init/deinit() interfaces. This commit reworks the buffer_handle{} interfaces to now become platform_buffer_init() and platform_buffer_deinit(). Structures that need a buffer_handle{}, declare a nested sub-struct, which will go through this init / deinit interface to allocate / free memory using existing mmap() interfaces. This removes the need for an input 'heap_id / heap_handle' arg to allocate and free memory. This change does not functionally change anything in these methods. Added small unit-test, platform_apis_test, to exercise these changes. Cleanup structures and fns that used to take 'heap_handle *' which now become unused with this rework. Tighten up backout / error handling in clockcache_init() and deinit() code-flow. --- Makefile | 3 + src/clockcache.c | 84 ++++++++++++------- src/clockcache.h | 20 ++--- src/platform_linux/platform.c | 76 +++++++++-------- src/platform_linux/platform.h | 8 +- src/rc_allocator.c | 66 +++++++-------- src/rc_allocator.h | 29 +++---- src/splinterdb.c | 3 - tests/functional/btree_test.c | 3 +- tests/functional/cache_test.c | 3 +- tests/functional/filter_test.c | 3 +- tests/functional/log_test.c | 10 +-- tests/functional/splinter_test.c | 4 +- tests/functional/test_functionality.c | 36 ++++---- tests/functional/test_functionality.h | 25 +++--- tests/functional/test_splinter_shadow.c | 43 ++++------ tests/functional/test_splinter_shadow.h | 14 ++-- tests/functional/ycsb_test.c | 18 +--- tests/unit/btree_stress_test.c | 3 +- tests/unit/platform_apis_test.c | 104 ++++++++++++++++++++++++ tests/unit/splinter_test.c | 3 +- 21 files changed, 325 insertions(+), 233 deletions(-) create mode 100644 tests/unit/platform_apis_test.c diff --git a/Makefile b/Makefile index 4b8237a41..a7f0688a6 100644 --- a/Makefile +++ b/Makefile @@ -443,6 +443,9 @@ $(BINDIR)/$(UNITDIR)/task_system_test: $(UTIL_SYS) $(OBJDIR)/$(FUNCTIONAL_TESTSDIR)/test_async.o \ $(LIBDIR)/libsplinterdb.so +$(BINDIR)/$(UNITDIR)/platform_apis_test: $(UTIL_SYS) \ + $(PLATFORM_SYS) + ######################################## # Convenience mini unit-test targets unit/util_test: $(BINDIR)/$(UNITDIR)/util_test diff --git a/src/clockcache.c b/src/clockcache.c index 3ce6d3055..f2e03d76b 100644 --- a/src/clockcache.c +++ b/src/clockcache.c @@ -46,7 +46,7 @@ * * clockcache_log, etc. are used to write an output of cache operations to * a log file for debugging purposes. If CC_LOG is set, then all output is - * written, if ADDR_TRACING is set, then only operations which affect + * written. If ADDR_TRACING is set, then only operations which affect * entries with either entry_number TRACE_ENTRY or address TRACE_ADDR are * written. * @@ -1761,14 +1761,13 @@ clockcache_config_init(clockcache_config *cache_cfg, } platform_status -clockcache_init(clockcache *cc, // OUT - clockcache_config *cfg, // IN - io_handle *io, // IN - allocator *al, // IN - char *name, // IN - platform_heap_handle hh, // IN - platform_heap_id hid, // IN - platform_module_id mid) // IN +clockcache_init(clockcache *cc, // OUT + clockcache_config *cfg, // IN + io_handle *io, // IN + allocator *al, // IN + char *name, // IN + platform_heap_id hid, // IN + platform_module_id mid) // IN { int i; threadid thr_i; @@ -1803,10 +1802,9 @@ clockcache_init(clockcache *cc, // OUT clockcache_log( 0, 0, "init: capacity %lu name %s\n", cc->cfg->capacity, name); - cc->al = al; - cc->io = io; - cc->heap_handle = hh; - cc->heap_id = hid; + cc->al = al; + cc->io = io; + cc->heap_id = hid; /* lookup maps addrs to entries, entry contains the entries themselves */ cc->lookup = @@ -1824,12 +1822,14 @@ clockcache_init(clockcache *cc, // OUT goto alloc_error; } + platform_status rc = STATUS_NO_MEMORY; + /* data must be aligned because of O_DIRECT */ - cc->bh = platform_buffer_create(cc->cfg->capacity, cc->heap_handle, mid); - if (!cc->bh) { + rc = platform_buffer_init(&cc->bh, cc->cfg->capacity); + if (!SUCCESS(rc)) { goto alloc_error; } - cc->data = platform_buffer_getaddr(cc->bh); + cc->data = platform_buffer_getaddr(&cc->bh); /* Set up the entries */ for (i = 0; i < cc->cfg->page_capacity; i++) { @@ -1841,14 +1841,19 @@ clockcache_init(clockcache *cc, // OUT /* Entry per-thread ref counts */ size_t refcount_size = cc->cfg->page_capacity * CC_RC_WIDTH * sizeof(uint8); - cc->rc_bh = platform_buffer_create(refcount_size, cc->heap_handle, mid); - if (!cc->rc_bh) { + + rc = platform_buffer_init(&cc->rc_bh, refcount_size); + if (!SUCCESS(rc)) { goto alloc_error; } - cc->refcount = platform_buffer_getaddr(cc->rc_bh); + cc->refcount = platform_buffer_getaddr(&cc->rc_bh); + /* Separate ref counts for pins */ cc->pincount = TYPED_ARRAY_ZALLOC(cc->heap_id, cc->pincount, cc->cfg->page_capacity); + if (!cc->pincount) { + goto alloc_error; + } /* The hands and associated page */ cc->free_hand = 0; @@ -1872,15 +1877,17 @@ clockcache_init(clockcache *cc, // OUT return STATUS_NO_MEMORY; } +/* + * De-init the resources allocated to initialize a clockcache. + * This function may be called to deal with error situations, or a failed + * clockcache_init(). So check for non-NULL handles before trying to release + * resources. + */ void clockcache_deinit(clockcache *cc) // IN/OUT { platform_assert(cc != NULL); - /* - * Check for non-null cause this is also used to clean up a failed - * clockcache_init - */ if (cc->logfile) { clockcache_log(0, 0, "deinit %s\n", ""); #if defined(CC_LOG) || defined(ADDR_TRACING) @@ -1888,20 +1895,35 @@ clockcache_deinit(clockcache *cc) // IN/OUT #endif } - if (cc->rc_bh) { - platform_buffer_destroy(cc->rc_bh); + if (cc->lookup) { + platform_free(cc->heap_id, cc->lookup); } + if (cc->entry) { + platform_free(cc->heap_id, cc->entry); + } + + debug_only platform_status rc = STATUS_TEST_FAILED; + if (cc->data) { + rc = platform_buffer_deinit(&cc->bh); - platform_free(cc->heap_id, cc->entry); - platform_free(cc->heap_id, cc->lookup); - if (cc->bh) { - platform_buffer_destroy(cc->bh); + // We expect above to succeed. Anyway, we are in the process of + // dismantling the clockcache, hence, for now, can't do much by way + // of reporting errors further upstream. + debug_assert(SUCCESS(rc), "rc=%s", platform_status_to_string(rc)); + cc->data = NULL; } - cc->data = NULL; - platform_free_volatile(cc->heap_id, cc->batch_busy); + if (cc->refcount) { + rc = platform_buffer_deinit(&cc->rc_bh); + debug_assert(SUCCESS(rc), "rc=%s", platform_status_to_string(rc)); + cc->refcount = NULL; + } + if (cc->pincount) { platform_free_volatile(cc->heap_id, cc->pincount); } + if (cc->batch_busy) { + platform_free_volatile(cc->heap_id, cc->batch_busy); + } } /* diff --git a/src/clockcache.h b/src/clockcache.h index f442aa11f..09e092717 100644 --- a/src/clockcache.h +++ b/src/clockcache.h @@ -116,14 +116,13 @@ struct clockcache { uint32 *lookup; clockcache_entry *entry; - buffer_handle *bh; // actual memory for pages + buffer_handle bh; // actual memory for pages char *data; // convenience pointer for bh platform_log_handle *logfile; - platform_heap_handle heap_handle; platform_heap_id heap_id; // Distributed locks (the write bit is in the status uint32 of the entry) - buffer_handle *rc_bh; + buffer_handle rc_bh; volatile uint8 *refcount; volatile uint8 *pincount; @@ -157,14 +156,13 @@ clockcache_config_init(clockcache_config *cache_config, uint64 use_stats); platform_status -clockcache_init(clockcache *cc, // OUT - clockcache_config *cfg, // IN - io_handle *io, // IN - allocator *al, // IN - char *name, // IN - platform_heap_handle hh, // IN - platform_heap_id hid, // IN - platform_module_id mid); // IN +clockcache_init(clockcache *cc, // OUT + clockcache_config *cfg, // IN + io_handle *io, // IN + allocator *al, // IN + char *name, // IN + platform_heap_id hid, // IN + platform_module_id mid); // IN void clockcache_deinit(clockcache *cc); // IN diff --git a/src/platform_linux/platform.c b/src/platform_linux/platform.c index 378a9e3ba..f63bc2856 100644 --- a/src/platform_linux/platform.c +++ b/src/platform_linux/platform.c @@ -56,45 +56,49 @@ void platform_heap_destroy(platform_heap_handle UNUSED_PARAM(*heap_handle)) {} -buffer_handle * -platform_buffer_create(size_t length, - platform_heap_handle UNUSED_PARAM(heap_handle), - platform_module_id UNUSED_PARAM(module_id)) +/* + * platform_buffer_init() - Initialize an input buffer_handle, bh. + * + * Certain modules, e.g. the buffer cache, need a very large buffer which + * may not be serviceable by the heap. Create the requested buffer using + * mmap() and initialize the input 'bh' to track this memory allocation. + */ +platform_status +platform_buffer_init(buffer_handle *bh, size_t length) { - buffer_handle *bh = TYPED_MALLOC(platform_get_heap_id(), bh); + platform_status rc = STATUS_NO_MEMORY; - if (bh != NULL) { - int prot = PROT_READ | PROT_WRITE; - int flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE; - if (platform_use_hugetlb) { - flags |= MAP_HUGETLB; - } + int prot = PROT_READ | PROT_WRITE; + int flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE; + if (platform_use_hugetlb) { + flags |= MAP_HUGETLB; + } - bh->addr = mmap(NULL, length, prot, flags, -1, 0); - if (bh->addr == MAP_FAILED) { - platform_error_log( - "mmap (%lu) failed with error: %s\n", length, strerror(errno)); - goto error; - } + bh->addr = mmap(NULL, length, prot, flags, -1, 0); + if (bh->addr == MAP_FAILED) { + platform_error_log( + "mmap (%lu bytes) failed with error: %s\n", length, strerror(errno)); + goto error; + } - if (platform_use_mlock) { - int rc = mlock(bh->addr, length); - if (rc != 0) { - platform_error_log( - "mlock (%lu) failed with error: %s\n", length, strerror(errno)); - munmap(bh->addr, length); - goto error; - } + if (platform_use_mlock) { + int mlock_rv = mlock(bh->addr, length); + if (mlock_rv != 0) { + platform_error_log("mlock (%lu bytes) failed with error: %s\n", + length, + strerror(errno)); + munmap(bh->addr, length); + rc = CONST_STATUS(errno); + goto error; } } - bh->length = length; - return bh; + return STATUS_OK; error: - platform_free(platform_get_heap_id(), bh); - bh = NULL; - return bh; + // Reset, in case mmap() or mlock() failed. + bh->addr = NULL; + return rc; } void * @@ -103,8 +107,14 @@ platform_buffer_getaddr(const buffer_handle *bh) return bh->addr; } +/* + * platform_buffer_deinit() - Deinit the buffer handle, which involves + * unmapping the memory for the large buffer created using mmap(). + * This is expected to be called on a 'bh' that has been successfully + * initialized, thru a prior platform_buffer_init() call. + */ platform_status -platform_buffer_destroy(buffer_handle *bh) +platform_buffer_deinit(buffer_handle *bh) { int ret; ret = munmap(bh->addr, bh->length); @@ -112,8 +122,8 @@ platform_buffer_destroy(buffer_handle *bh) return CONST_STATUS(errno); } - platform_free(platform_get_heap_id(), bh); - + bh->addr = NULL; + bh->length = 0; return STATUS_OK; } diff --git a/src/platform_linux/platform.h b/src/platform_linux/platform.h index 3c9597e6c..54950caed 100644 --- a/src/platform_linux/platform.h +++ b/src/platform_linux/platform.h @@ -626,16 +626,14 @@ platform_heap_create(platform_module_id module_id, void platform_heap_destroy(platform_heap_handle *heap_handle); -buffer_handle * -platform_buffer_create(size_t length, - platform_heap_handle heap_handle, - platform_module_id module_id); +platform_status +platform_buffer_init(buffer_handle *bh, size_t length); void * platform_buffer_getaddr(const buffer_handle *bh); platform_status -platform_buffer_destroy(buffer_handle *bh); +platform_buffer_deinit(buffer_handle *bh); platform_status platform_mutex_init(platform_mutex *mu, diff --git a/src/rc_allocator.c b/src/rc_allocator.c index 5d46baae1..88cc5a1fc 100644 --- a/src/rc_allocator.c +++ b/src/rc_allocator.c @@ -318,23 +318,21 @@ rc_allocator_valid_config(allocator_config *cfg) *---------------------------------------------------------------------- */ platform_status -rc_allocator_init(rc_allocator *al, - allocator_config *cfg, - io_handle *io, - platform_heap_handle hh, - platform_heap_id hid, - platform_module_id mid) +rc_allocator_init(rc_allocator *al, + allocator_config *cfg, + io_handle *io, + platform_heap_id hid, + platform_module_id mid) { uint64 rc_extent_count; uint64 addr; platform_status rc; platform_assert(al != NULL); ZERO_CONTENTS(al); - al->super.ops = &rc_allocator_ops; - al->cfg = cfg; - al->io = io; - al->heap_handle = hh; - al->heap_id = hid; + al->super.ops = &rc_allocator_ops; + al->cfg = cfg; + al->io = io; + al->heap_id = hid; rc = rc_allocator_valid_config(cfg); if (!SUCCESS(rc)) { @@ -353,17 +351,16 @@ rc_allocator_init(rc_allocator *al, return rc; } // To ensure alignment always allocate in multiples of page size. - uint32 buffer_size = cfg->extent_capacity * sizeof(uint8); + uint64 buffer_size = cfg->extent_capacity * sizeof(uint8); buffer_size = ROUNDUP(buffer_size, cfg->io_cfg->page_size); - al->bh = platform_buffer_create(buffer_size, al->heap_handle, mid); - if (al->bh == NULL) { - platform_error_log("Failed to create buffer for ref counts\n"); + rc = platform_buffer_init(&al->bh, buffer_size); + if (!SUCCESS(rc)) { platform_mutex_destroy(&al->lock); platform_free(al->heap_id, al->meta_page); + platform_error_log("Failed to create buffer for ref counts\n"); return STATUS_NO_MEMORY; } - - al->ref_count = platform_buffer_getaddr(al->bh); + al->ref_count = platform_buffer_getaddr(&al->bh); memset(al->ref_count, 0, buffer_size); // allocate the super block @@ -388,7 +385,7 @@ rc_allocator_init(rc_allocator *al, void rc_allocator_deinit(rc_allocator *al) { - platform_buffer_destroy(al->bh); + platform_buffer_deinit(&al->bh); al->ref_count = NULL; platform_mutex_destroy(&al->lock); platform_free(al->heap_id, al->meta_page); @@ -403,22 +400,20 @@ rc_allocator_deinit(rc_allocator *al) *---------------------------------------------------------------------- */ platform_status -rc_allocator_mount(rc_allocator *al, - allocator_config *cfg, - io_handle *io, - platform_heap_handle hh, - platform_heap_id hid, - platform_module_id mid) +rc_allocator_mount(rc_allocator *al, + allocator_config *cfg, + io_handle *io, + platform_heap_id hid, + platform_module_id mid) { platform_status status; platform_assert(al != NULL); ZERO_CONTENTS(al); - al->super.ops = &rc_allocator_ops; - al->cfg = cfg; - al->io = io; - al->heap_handle = hh; - al->heap_id = hid; + al->super.ops = &rc_allocator_ops; + al->cfg = cfg; + al->io = io; + al->heap_id = hid; status = platform_mutex_init(&al->lock, mid, al->heap_id); if (!SUCCESS(status)) { @@ -439,11 +434,16 @@ rc_allocator_mount(rc_allocator *al, platform_assert(cfg->capacity == cfg->io_cfg->page_size * cfg->page_capacity); - uint32 buffer_size = cfg->extent_capacity * sizeof(uint8); + uint64 buffer_size = cfg->extent_capacity * sizeof(uint8); buffer_size = ROUNDUP(buffer_size, cfg->io_cfg->page_size); - al->bh = platform_buffer_create(buffer_size, al->heap_handle, mid); - platform_assert(al->bh != NULL); - al->ref_count = platform_buffer_getaddr(al->bh); + status = platform_buffer_init(&al->bh, buffer_size); + if (!SUCCESS(status)) { + platform_free(al->heap_id, al->meta_page); + platform_mutex_destroy(&al->lock); + platform_error_log("Failed to create buffer to load ref counts\n"); + return STATUS_NO_MEMORY; + } + al->ref_count = platform_buffer_getaddr(&al->bh); // load the meta page from disk. status = io_read( diff --git a/src/rc_allocator.h b/src/rc_allocator.h index 38f8ef52f..87e0959ef 100644 --- a/src/rc_allocator.h +++ b/src/rc_allocator.h @@ -58,7 +58,7 @@ typedef struct rc_allocator_stats { typedef struct rc_allocator { allocator super; allocator_config *cfg; - buffer_handle *bh; + buffer_handle bh; uint8 *ref_count; uint64 hand; io_handle *io; @@ -68,32 +68,29 @@ typedef struct rc_allocator { * mutex to synchronize updates to super block addresses of the splinter * tables in the meta page. */ - platform_mutex lock; - platform_heap_handle heap_handle; - platform_heap_id heap_id; + platform_mutex lock; + platform_heap_id heap_id; // Stats -- not distributed for now rc_allocator_stats stats; } rc_allocator; platform_status -rc_allocator_init(rc_allocator *al, - allocator_config *cfg, - io_handle *io, - platform_heap_handle hh, - platform_heap_id hid, - platform_module_id mid); +rc_allocator_init(rc_allocator *al, + allocator_config *cfg, + io_handle *io, + platform_heap_id hid, + platform_module_id mid); void rc_allocator_deinit(rc_allocator *al); platform_status -rc_allocator_mount(rc_allocator *al, - allocator_config *cfg, - io_handle *io, - platform_heap_handle hh, - platform_heap_id hid, - platform_module_id mid); +rc_allocator_mount(rc_allocator *al, + allocator_config *cfg, + io_handle *io, + platform_heap_id hid, + platform_module_id mid); void rc_allocator_unmount(rc_allocator *al); diff --git a/src/splinterdb.c b/src/splinterdb.c index ff67f282f..02d458f05 100644 --- a/src/splinterdb.c +++ b/src/splinterdb.c @@ -274,14 +274,12 @@ splinterdb_create_or_open(const splinterdb_config *kvs_cfg, // IN status = rc_allocator_mount(&kvs->allocator_handle, &kvs->allocator_cfg, (io_handle *)&kvs->io_handle, - kvs->heap_handle, kvs->heap_id, platform_get_module_id()); } else { status = rc_allocator_init(&kvs->allocator_handle, &kvs->allocator_cfg, (io_handle *)&kvs->io_handle, - kvs->heap_handle, kvs->heap_id, platform_get_module_id()); } @@ -296,7 +294,6 @@ splinterdb_create_or_open(const splinterdb_config *kvs_cfg, // IN (io_handle *)&kvs->io_handle, (allocator *)&kvs->allocator_handle, "splinterdb", - kvs->heap_handle, kvs->heap_id, platform_get_module_id()); if (!SUCCESS(status)) { diff --git a/tests/functional/btree_test.c b/tests/functional/btree_test.c index 41e6e83fe..6c5abd7c3 100644 --- a/tests/functional/btree_test.c +++ b/tests/functional/btree_test.c @@ -1571,7 +1571,7 @@ btree_test(int argc, char *argv[]) rc_allocator al; rc_allocator_init( - &al, &al_cfg, (io_handle *)io, hh, hid, platform_get_module_id()); + &al, &al_cfg, (io_handle *)io, hid, platform_get_module_id()); clockcache *cc = TYPED_MALLOC(hid, cc); rc = clockcache_init(cc, @@ -1579,7 +1579,6 @@ btree_test(int argc, char *argv[]) (io_handle *)io, (allocator *)&al, "test", - hh, hid, platform_get_module_id()); platform_assert_status_ok(rc); diff --git a/tests/functional/cache_test.c b/tests/functional/cache_test.c index 58a985ab5..eb4df93d9 100644 --- a/tests/functional/cache_test.c +++ b/tests/functional/cache_test.c @@ -1047,7 +1047,7 @@ cache_test(int argc, char *argv[]) rc_allocator al; rc_allocator_init( - &al, &al_cfg, (io_handle *)io, hh, hid, platform_get_module_id()); + &al, &al_cfg, (io_handle *)io, hid, platform_get_module_id()); clockcache *cc = TYPED_MALLOC(hid, cc); rc = clockcache_init(cc, @@ -1055,7 +1055,6 @@ cache_test(int argc, char *argv[]) (io_handle *)io, (allocator *)&al, "test", - hh, hid, platform_get_module_id()); platform_assert_status_ok(rc); diff --git a/tests/functional/filter_test.c b/tests/functional/filter_test.c index 1c39d0633..3fff94648 100644 --- a/tests/functional/filter_test.c +++ b/tests/functional/filter_test.c @@ -360,7 +360,7 @@ filter_test(int argc, char *argv[]) platform_assert_status_ok(rc); rc = rc_allocator_init( - &al, &allocator_cfg, (io_handle *)io, hh, hid, platform_get_module_id()); + &al, &allocator_cfg, (io_handle *)io, hid, platform_get_module_id()); platform_assert_status_ok(rc); cc = TYPED_MALLOC(hid, cc); @@ -370,7 +370,6 @@ filter_test(int argc, char *argv[]) (io_handle *)io, (allocator *)&al, "test", - hh, hid, platform_get_module_id()); platform_assert_status_ok(rc); diff --git a/tests/functional/log_test.c b/tests/functional/log_test.c index 4e16f869d..6e5dea24c 100644 --- a/tests/functional/log_test.c +++ b/tests/functional/log_test.c @@ -28,7 +28,6 @@ test_log_crash(clockcache *cc, shard_log_config *cfg, shard_log *log, task_system *ts, - platform_heap_handle hh, platform_heap_id hid, test_message_generator *gen, uint64 num_entries, @@ -75,7 +74,7 @@ test_log_crash(clockcache *cc, if (crash) { clockcache_deinit(cc); rc = clockcache_init( - cc, cache_cfg, io, al, "crashed", hh, hid, platform_get_module_id()); + cc, cache_cfg, io, al, "crashed", hid, platform_get_module_id()); platform_assert_status_ok(rc); } @@ -318,7 +317,7 @@ log_test(int argc, char *argv[]) } status = rc_allocator_init( - &al, &al_cfg, (io_handle *)io, hh, hid, platform_get_module_id()); + &al, &al_cfg, (io_handle *)io, hid, platform_get_module_id()); platform_assert_status_ok(status); clockcache *cc = TYPED_MALLOC(hid, cc); @@ -328,7 +327,6 @@ log_test(int argc, char *argv[]) (io_handle *)io, (allocator *)&al, "test", - hh, hid, platform_get_module_id()); platform_assert_status_ok(status); @@ -348,7 +346,6 @@ log_test(int argc, char *argv[]) &log_cfg, log, ts, - hh, hid, &gen, 500000, @@ -362,11 +359,10 @@ log_test(int argc, char *argv[]) &log_cfg, log, ts, - hh, hid, &gen, 500000, - FALSE /* don't cash */); + FALSE /* don't crash */); platform_assert(rc == 0); } diff --git a/tests/functional/splinter_test.c b/tests/functional/splinter_test.c index 24f8ec917..13ffbbee5 100644 --- a/tests/functional/splinter_test.c +++ b/tests/functional/splinter_test.c @@ -2762,7 +2762,7 @@ splinter_test(int argc, char *argv[]) rc_allocator al; rc_allocator_init( - &al, &al_cfg, (io_handle *)io, hh, hid, platform_get_module_id()); + &al, &al_cfg, (io_handle *)io, hid, platform_get_module_id()); platform_error_log("Running splinter_test with %d caches\n", num_caches); clockcache *cc = TYPED_ARRAY_MALLOC(hid, cc, num_caches); @@ -2773,7 +2773,6 @@ splinter_test(int argc, char *argv[]) (io_handle *)io, (allocator *)&al, "test", - hh, hid, platform_get_module_id()); platform_assert_status_ok(rc); @@ -2901,7 +2900,6 @@ splinter_test(int argc, char *argv[]) test_ops, correctness_check_frequency, ts, - hh, hid, num_tables, num_caches, diff --git a/tests/functional/test_functionality.c b/tests/functional/test_functionality.c index ff948d025..553d626eb 100644 --- a/tests/functional/test_functionality.c +++ b/tests/functional/test_functionality.c @@ -20,7 +20,7 @@ void destroy_test_splinter_shadow_array(test_splinter_shadow_array *sharr) { - platform_buffer_destroy(sharr->buffer); + platform_buffer_deinit(&sharr->buffer); sharr->nkeys = 0; } @@ -469,7 +469,6 @@ static platform_status validate_tree_against_shadow(trunk_handle *spl, random_state *prg, test_splinter_shadow_tree *shadow, - platform_heap_handle hh, platform_heap_id hid, bool do_it, test_async_lookup *async_lookup) @@ -485,7 +484,7 @@ validate_tree_against_shadow(trunk_handle *spl, memset(&sharr, 0, sizeof(sharr)); if (do_it) { - rc = test_splinter_build_shadow_array(shadow, &sharr, hh); + rc = test_splinter_build_shadow_array(shadow, &sharr); if (!SUCCESS(rc)) { // might need to cleanup a partially allocated shadow array. platform_error_log("Failed to build shadow array: %s\n", @@ -631,19 +630,18 @@ cmp_ptrs(const void *a, const void *b) *----------------------------------------------------------------------------- */ platform_status -test_functionality(allocator *al, - io_handle *io, - cache *cc[], - trunk_config *cfg, - uint64 seed, - uint64 num_inserts, - uint64 correctness_check_frequency, - task_system *state, - platform_heap_handle hh, - platform_heap_id hid, - uint8 num_tables, - uint8 num_caches, - uint32 max_async_inflight) +test_functionality(allocator *al, + io_handle *io, + cache *cc[], + trunk_config *cfg, + uint64 seed, + uint64 num_inserts, + uint64 correctness_check_frequency, + task_system *state, + platform_heap_id hid, + uint8 num_tables, + uint8 num_caches, + uint32 max_async_inflight) { platform_error_log("Functional test started with %d tables\n", num_tables); platform_assert(cc != NULL); @@ -675,7 +673,7 @@ test_functionality(allocator *al, // Initialize the splinter/shadow for each splinter table. for (uint8 idx = 0; idx < num_tables; idx++) { cache *cache_to_use = num_caches > 1 ? cc[idx] : *cc; - status = test_splinter_shadow_create(&shadows[idx], hh, hid, num_inserts); + status = test_splinter_shadow_create(&shadows[idx], hid, num_inserts); if (!SUCCESS(status)) { platform_error_log("Failed to init shadow for splinter: %s\n", platform_status_to_string(status)); @@ -697,7 +695,7 @@ test_functionality(allocator *al, trunk_handle *spl = spl_tables[idx]; test_splinter_shadow_tree *shadow = shadows[idx]; status = validate_tree_against_shadow( - spl, &prg, shadow, hh, hid, TRUE, async_lookup); + spl, &prg, shadow, hid, TRUE, async_lookup); if (!SUCCESS(status)) { platform_error_log("Failed to validate empty tree against shadow: \ %s\n", @@ -793,7 +791,6 @@ test_functionality(allocator *al, spl, &prg, shadow, - hh, hid, correctness_check_frequency && (i % correctness_check_frequency) == 0, @@ -840,7 +837,6 @@ test_functionality(allocator *al, spl, &prg, shadow, - hh, hid, correctness_check_frequency && ((i - 1) % correctness_check_frequency) != 0, diff --git a/tests/functional/test_functionality.h b/tests/functional/test_functionality.h index 83fc9c7cf..fc90b0e20 100644 --- a/tests/functional/test_functionality.h +++ b/tests/functional/test_functionality.h @@ -7,16 +7,15 @@ #include "platform.h" platform_status -test_functionality(allocator *al, - io_handle *io, - cache *cc[], - trunk_config *cfg, - uint64 seed, - uint64 num_inserts, - uint64 correctness_check_frequency, - task_system *ts, - platform_heap_handle hh, - platform_heap_id hid, - uint8 num_tables, - uint8 num_caches, - uint32 max_async_inflight); +test_functionality(allocator *al, + io_handle *io, + cache *cc[], + trunk_config *cfg, + uint64 seed, + uint64 num_inserts, + uint64 correctness_check_frequency, + task_system *ts, + platform_heap_id hid, + uint8 num_tables, + uint8 num_caches, + uint32 max_async_inflight); diff --git a/tests/functional/test_splinter_shadow.c b/tests/functional/test_splinter_shadow.c index 290456403..a1c8144ea 100644 --- a/tests/functional/test_splinter_shadow.c +++ b/tests/functional/test_splinter_shadow.c @@ -81,9 +81,7 @@ test_splinter_shadow_cmp_keys(const AvlTreeLinks *k1, const AvlTreeLinks *k2) /* *----------------------------------------------------------------------------- - * * test_splinter_shadow_init -- - * * Function to initialize memory and avlTree for the shadow tree. * * Results: @@ -93,18 +91,17 @@ test_splinter_shadow_cmp_keys(const AvlTreeLinks *k1, const AvlTreeLinks *k2) * Will allocate memory for the shadow structure. *----------------------------------------------------------------------------- */ - platform_status test_splinter_shadow_create(test_splinter_shadow_tree **tree, - platform_heap_handle hh, platform_heap_id hid, uint64 max_operations) { - test_splinter_shadow_tree *shadow = TYPED_ZALLOC(hid, shadow); + platform_status rc = STATUS_NO_MEMORY; + test_splinter_shadow_tree *shadow = TYPED_ZALLOC(hid, shadow); if (shadow == NULL) { platform_default_log("Failed to allocate memory for shadow init"); - return STATUS_NO_MEMORY; + return rc; } /* @@ -113,16 +110,15 @@ test_splinter_shadow_create(test_splinter_shadow_tree **tree, * insert operations, since only those operations will require new node * allocations. */ - shadow->nodes_buffer = - platform_buffer_create(sizeof(test_splinter_shadow_node) * max_operations, - hh, - platform_get_module_id()); - if (shadow->nodes_buffer == NULL) { + rc = + platform_buffer_init(&shadow->nodes_buffer, + sizeof(test_splinter_shadow_node) * max_operations); + if (!SUCCESS(rc)) { platform_default_log("Failed to pre allocate nodes for shadow tree\n"); platform_free(hid, shadow); - return STATUS_NO_MEMORY; + return rc; } - shadow->nodes = platform_buffer_getaddr(shadow->nodes_buffer); + shadow->nodes = platform_buffer_getaddr(&shadow->nodes_buffer); shadow->numPreAllocatedNodes = max_operations; AvlTree_Init(&shadow->tree, @@ -133,7 +129,6 @@ test_splinter_shadow_create(test_splinter_shadow_tree **tree, return STATUS_OK; } - /* *----------------------------------------------------------------------------- * @@ -251,7 +246,6 @@ test_splinter_shadow_add(test_splinter_shadow_tree *tree, /* *----------------------------------------------------------------------------- - * * test_splinter_shadow_destroy_tree -- * * Function to cleanup the shadow tree structure. @@ -268,12 +262,11 @@ void test_splinter_shadow_destroy(platform_heap_id hid, test_splinter_shadow_tree *tree) { - platform_buffer_destroy(tree->nodes_buffer); + platform_buffer_deinit(&tree->nodes_buffer); tree->numKeys = 0; platform_free(hid, tree); } - /* *----------------------------------------------------------------------------- * @@ -310,7 +303,6 @@ test_splinter_shadow_iterate_tree(AvlTreeLinks *root, /* *----------------------------------------------------------------------------- - * * test_splinter_build_shadow_array -- * * Function to recurse over the tree and copy the key.value into an @@ -323,11 +315,9 @@ test_splinter_shadow_iterate_tree(AvlTreeLinks *root, * Will allocate memory for the key and value arrays. *----------------------------------------------------------------------------- */ - platform_status test_splinter_build_shadow_array(test_splinter_shadow_tree *tree, - test_splinter_shadow_array *shadow_array, - platform_heap_handle hh) + test_splinter_shadow_array *shadow_array) { uint64 idx = 0; shadow_array->nkeys = tree->numKeys; @@ -335,13 +325,14 @@ test_splinter_build_shadow_array(test_splinter_shadow_tree *tree, (sizeof(*shadow_array->keys) + sizeof(*shadow_array->ref_counts)) * tree->numKeys; - shadow_array->buffer = - platform_buffer_create(totalBufferSize, hh, platform_get_module_id()); - if (shadow_array->buffer == NULL) { + platform_status rc = STATUS_NO_MEMORY; + + rc = platform_buffer_init(&shadow_array->buffer, totalBufferSize); + if (!SUCCESS(rc)) { platform_default_log("Failed to allocate memory for shadow array\n"); - return STATUS_NO_MEMORY; + return rc; } - shadow_array->keys = platform_buffer_getaddr(shadow_array->buffer); + shadow_array->keys = platform_buffer_getaddr(&shadow_array->buffer); // Memory for ref count array starts at base memory + memory for keys array. shadow_array->ref_counts = (void *)((uint64)shadow_array->keys diff --git a/tests/functional/test_splinter_shadow.h b/tests/functional/test_splinter_shadow.h index a13c80553..a80e0e51b 100644 --- a/tests/functional/test_splinter_shadow.h +++ b/tests/functional/test_splinter_shadow.h @@ -25,22 +25,21 @@ typedef struct test_splinter_shadow_tree { AvlTree tree; uint64 numPreAllocatedNodes; uint64 currentAllocIdx; - buffer_handle *nodes_buffer; + buffer_handle nodes_buffer; test_splinter_shadow_node *nodes; } test_splinter_shadow_tree; typedef struct test_splinter_shadow_array { - uint64 nkeys; - buffer_handle *buffer; - uint64 *keys; - int8 *ref_counts; + uint64 nkeys; + buffer_handle buffer; + uint64 *keys; + int8 *ref_counts; } test_splinter_shadow_array; platform_status test_splinter_shadow_create(test_splinter_shadow_tree **tree, - platform_heap_handle hh, platform_heap_id hid, uint64 max_operations); @@ -81,7 +80,6 @@ test_splinter_shadow_destroy(platform_heap_id hid, platform_status test_splinter_build_shadow_array(test_splinter_shadow_tree *tree, - test_splinter_shadow_array *shadow_array, - platform_heap_handle hh); + test_splinter_shadow_array *shadow_array); #endif diff --git a/tests/functional/ycsb_test.c b/tests/functional/ycsb_test.c index 0e06b8438..cefdca145 100644 --- a/tests/functional/ycsb_test.c +++ b/tests/functional/ycsb_test.c @@ -1291,18 +1291,13 @@ ycsb_test(int argc, char *argv[]) trunk_handle *spl; if (use_existing) { - rc_allocator_mount(&al, - &allocator_cfg, - (io_handle *)io, - hh, - hid, - platform_get_module_id()); + rc_allocator_mount( + &al, &allocator_cfg, (io_handle *)io, hid, platform_get_module_id()); rc = clockcache_init(cc, &cache_cfg, (io_handle *)io, (allocator *)&al, "test", - hh, hid, platform_get_module_id()); platform_assert_status_ok(rc); @@ -1314,18 +1309,13 @@ ycsb_test(int argc, char *argv[]) hid); platform_assert(spl); } else { - rc_allocator_init(&al, - &allocator_cfg, - (io_handle *)io, - hh, - hid, - platform_get_module_id()); + rc_allocator_init( + &al, &allocator_cfg, (io_handle *)io, hid, platform_get_module_id()); rc = clockcache_init(cc, &cache_cfg, (io_handle *)io, (allocator *)&al, "test", - hh, hid, platform_get_module_id()); platform_assert_status_ok(rc); diff --git a/tests/unit/btree_stress_test.c b/tests/unit/btree_stress_test.c index e47a04919..0408923a6 100644 --- a/tests/unit/btree_stress_test.c +++ b/tests/unit/btree_stress_test.c @@ -150,7 +150,6 @@ CTEST_SETUP(btree_stress) || !SUCCESS(rc_allocator_init(&data->al, &data->allocator_cfg, (io_handle *)&data->io, - data->hh, data->hid, platform_get_module_id())) || !SUCCESS(clockcache_init(&data->cc, @@ -158,7 +157,6 @@ CTEST_SETUP(btree_stress) (io_handle *)&data->io, (allocator *)&data->al, "test", - data->hh, data->hid, platform_get_module_id()))) { @@ -174,6 +172,7 @@ CTEST_TEARDOWN(btree_stress) clockcache_deinit(&data->cc); rc_allocator_deinit(&data->al); task_system_destroy(data->hid, &data->ts); + platform_heap_destroy(&data->hh); } /* diff --git a/tests/unit/platform_apis_test.c b/tests/unit/platform_apis_test.c new file mode 100644 index 000000000..69e074882 --- /dev/null +++ b/tests/unit/platform_apis_test.c @@ -0,0 +1,104 @@ +// Copyright 2023 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/* + * ----------------------------------------------------------------------------- + * platform_apis_test.c + * + * Exercise some of the interfaces in platform.c + * ----------------------------------------------------------------------------- + */ +#include + +#include "ctest.h" // This is required for all test-case files. +#include "platform.h" + +/* + * Global data declaration macro: + */ +CTEST_DATA(platform_api) +{ + // Declare heap handles for platform heap memory. + platform_heap_handle hh; + platform_heap_id hid; + platform_module_id mid; +}; + +CTEST_SETUP(platform_api) +{ + // This test exercises error cases, so even when everything succeeds + // it generates lots of "error" messages. + // By default, that would go to stderr, which would pollute test output. + // Here we ensure those expected error messages are only printed + // when the caller sets the VERBOSE env var to opt-in. + if (Ctest_verbose) { + platform_set_log_streams(stdout, stderr); + CTEST_LOG_INFO("\nVerbose mode on. This test exercises an error case, " + "so on sucess it " + "will print a message that appears to be an error.\n"); + } else { + FILE *dev_null = fopen("/dev/null", "w"); + ASSERT_NOT_NULL(dev_null); + platform_set_log_streams(dev_null, dev_null); + } + + platform_status rc = STATUS_OK; + + uint64 heap_capacity = (256 * MiB); // small heap is sufficient. + data->mid = platform_get_module_id(); + rc = platform_heap_create(data->mid, heap_capacity, &data->hh, &data->hid); + platform_assert_status_ok(rc); +} + +CTEST_TEARDOWN(platform_api) +{ + platform_heap_destroy(&data->hh); +} + +/* + * Test platform_buffer_init() and platform_buffer_deinit(). + */ +CTEST2(platform_api, test_platform_buffer_init) +{ + platform_status rc = STATUS_NO_MEMORY; + + buffer_handle bh; + ZERO_CONTENTS(&bh); + + rc = platform_buffer_init(&bh, KiB); + ASSERT_TRUE(SUCCESS(rc)); + ASSERT_TRUE(bh.addr != NULL); + ASSERT_TRUE(bh.addr == platform_buffer_getaddr(&bh)); + ASSERT_TRUE(bh.length == KiB); + + rc = platform_buffer_deinit(&bh); + ASSERT_TRUE(SUCCESS(rc)); + ASSERT_TRUE(bh.addr == NULL); + ASSERT_TRUE(bh.length == 0); +} + +/* + * Test failure to mmap() a very large buffer_init(). + */ +CTEST2(platform_api, test_platform_buffer_init_fails_for_very_large_length) +{ + platform_status rc = STATUS_NO_MEMORY; + + buffer_handle bh; + ZERO_CONTENTS(&bh); + + size_t length = (1024 * KiB * GiB); + + // On most test machines we use, this is expected to fail as mmap() under + // here will fail for very large lengths. (If this test case ever fails, + // check the 'length' here and the machine's configuration to see why + // mmap() unexpectedly succeeded.) + rc = platform_buffer_init(&bh, length); + ASSERT_FALSE(SUCCESS(rc)); + ASSERT_TRUE(bh.addr == NULL); + ASSERT_TRUE(bh.length == 0); + + // deinit() would fail horribly when nothing was successfully mmap()'ed + rc = platform_buffer_deinit(&bh); + ASSERT_FALSE(SUCCESS(rc)); +} diff --git a/tests/unit/splinter_test.c b/tests/unit/splinter_test.c index d7e1a84e8..9d66168c7 100644 --- a/tests/unit/splinter_test.c +++ b/tests/unit/splinter_test.c @@ -189,7 +189,7 @@ CTEST_SETUP(splinter) "Failed to init splinter state: %s\n", platform_status_to_string(rc)); - rc_allocator_init(&data->al, &data->al_cfg, (io_handle *)data->io, data->hh, data->hid, + rc_allocator_init(&data->al, &data->al_cfg, (io_handle *)data->io, data->hid, platform_get_module_id()); data->clock_cache = TYPED_ARRAY_MALLOC(data->hid, data->clock_cache, num_caches); @@ -201,7 +201,6 @@ CTEST_SETUP(splinter) (io_handle *)data->io, (allocator *)&data->al, "test", - data->hh, data->hid, platform_get_module_id());