Skip to content

Commit

Permalink
(#507) Rework of platform_buffer_create()/destroy() to init/deinit() …
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@gapisback
gapisback committed Jan 11, 2023
1 parent 4fef244 commit bbe8063
Show file tree
Hide file tree
Showing 21 changed files with 325 additions and 233 deletions.
3 changes: 3 additions & 0 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
84 changes: 53 additions & 31 deletions src/clockcache.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 =
Expand All @@ -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++) {
Expand All @@ -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;
Expand All @@ -1872,36 +1877,53 @@ 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)
platform_close_log_file(cc->logfile);
#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);
}
}

/*
Expand Down
20 changes: 9 additions & 11 deletions src/clockcache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
76 changes: 43 additions & 33 deletions src/platform_linux/platform.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 *
Expand All @@ -103,17 +107,23 @@ 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);
if (ret) {
return CONST_STATUS(errno);
}

platform_free(platform_get_heap_id(), bh);

bh->addr = NULL;
bh->length = 0;
return STATUS_OK;
}

Expand Down
8 changes: 3 additions & 5 deletions src/platform_linux/platform.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit bbe8063

Please sign in to comment.