diff options
author | thiagoftsm <thiagoftsm@gmail.com> | 2019-12-18 16:37:02 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-12-18 16:37:02 +0000 |
commit | c8ded37b25237ec4aed8604888bd4281c33087a6 (patch) | |
tree | 087e14bd3c5eefb170b91c5fe100c470ac626239 | |
parent | 4d9ed56f1b04b88ee73e4d359f478cac25c194c7 (diff) |
Fix race condition in dbengine (#7565)
* fix_db_race_condition: unit test
Adjust unit test for dbengine
* fix_db_race_condition: page cache
Fix database
* fix_db_race_condition: Missing function call
This commit brings the correct function call inside rrdengine.c
-rw-r--r-- | daemon/main.c | 13 | ||||
-rw-r--r-- | daemon/unit_test.c | 83 | ||||
-rw-r--r-- | daemon/unit_test.h | 2 | ||||
-rw-r--r-- | database/engine/pagecache.c | 38 | ||||
-rw-r--r-- | database/engine/pagecache.h | 1 | ||||
-rw-r--r-- | database/engine/rrdengine.c | 10 | ||||
-rwxr-xr-x | database/engine/rrdengineapi.c | 11 | ||||
-rw-r--r-- | database/engine/rrdenglocking.c | 36 |
8 files changed, 121 insertions, 73 deletions
diff --git a/daemon/main.c b/daemon/main.c index ece8feb447..b00de00628 100644 --- a/daemon/main.c +++ b/daemon/main.c @@ -340,10 +340,12 @@ int help(int exitcode) { " -W unittest Run internal unittests and exit.\n\n" #ifdef ENABLE_DBENGINE " -W createdataset=N Create a DB engine dataset of N seconds and exit.\n\n" - " -W stresstest=A,B,C,D,E Run a DB engine stress test for A seconds,\n" + " -W stresstest=A,B,C,D,E,F\n" + " Run a DB engine stress test for A seconds,\n" " with B writers and C readers, with a ramp up\n" " time of D seconds for writers, a page cache\n" - " size of E MiB, and exit.\n\n" + " size of E MiB, an optional disk space limit" + " of F MiB and exit.\n\n" #endif " -W set section option value\n" " set netdata.conf option from the command line.\n\n" @@ -956,7 +958,7 @@ int main(int argc, char **argv) { else if(strncmp(optarg, stresstest_string, strlen(stresstest_string)) == 0) { char *endptr; unsigned test_duration_sec = 0, dset_charts = 0, query_threads = 0, ramp_up_seconds = 0, - page_cache_mb = 0; + page_cache_mb = 0, disk_space_mb = 0; optarg += strlen(stresstest_string); test_duration_sec = (unsigned)strtoul(optarg, &endptr, 0); @@ -968,8 +970,11 @@ int main(int argc, char **argv) { ramp_up_seconds = (unsigned)strtoul(endptr + 1, &endptr, 0); if (',' == *endptr) page_cache_mb = (unsigned)strtoul(endptr + 1, &endptr, 0); + if (',' == *endptr) + disk_space_mb = (unsigned)strtoul(endptr + 1, &endptr, 0); + dbengine_stress_test(test_duration_sec, dset_charts, query_threads, ramp_up_seconds, - page_cache_mb); + page_cache_mb, disk_space_mb); return 0; } #endif diff --git a/daemon/unit_test.c b/daemon/unit_test.c index 2e59273214..87d281d766 100644 --- a/daemon/unit_test.c +++ b/daemon/unit_test.c @@ -1723,7 +1723,7 @@ int test_dbengine(void) default_rrd_memory_mode = RRD_MEMORY_MODE_DBENGINE; - debug(D_RRDHOST, "Initializing localhost with hostname 'unittest-dbengine'"); + fprintf(stderr, "Initializing localhost with hostname 'unittest-dbengine'"); host = dbengine_rrdhost_find_or_create("unittest-dbengine"); if (NULL == host) return 1; @@ -1915,6 +1915,9 @@ static void generate_dbengine_chart(void *arg) rrdset_done(st); thread_info->time_max = time_current; } + for (j = 0; j < DSET_DIMS; ++j) { + rrdeng_store_metric_finalize(rd[j]); + } } void generate_dbengine_dataset(unsigned history_seconds) @@ -1935,7 +1938,7 @@ void generate_dbengine_dataset(unsigned history_seconds) default_rrdeng_disk_quota_mb -= default_rrdeng_disk_quota_mb * EXPECTED_COMPRESSION_RATIO / 100; error_log_limit_unlimited(); - debug(D_RRDHOST, "Initializing localhost with hostname 'dbengine-dataset'"); + fprintf(stderr, "Initializing localhost with hostname 'dbengine-dataset'"); host = dbengine_rrdhost_find_or_create("dbengine-dataset"); if (NULL == host) @@ -1986,6 +1989,7 @@ struct dbengine_query_thread { unsigned history_seconds; /* how far back in the past to go */ volatile long done; /* initialize to 0, set to 1 to stop thread */ unsigned long errors, queries_nr, queried_metrics_nr; /* statistics */ + uint8_t delete_old_data; /* if non zero then data are deleted when disk space is exhausted */ struct dbengine_chart_thread *chart_threads[]; /* dset_charts elements */ }; @@ -1995,7 +1999,7 @@ static void query_dbengine_chart(void *arg) struct dbengine_query_thread *thread_info = (struct dbengine_query_thread *)arg; const int DSET_CHARTS = thread_info->dset_charts; const int DSET_DIMS = thread_info->dset_dims; - time_t time_after, time_before, time_min, time_max, duration; + time_t time_after, time_before, time_min, time_approx_min, time_max, duration; int i, j, update_every = 1; RRDSET *st; RRDDIM *rd; @@ -2015,6 +2019,13 @@ static void query_dbengine_chart(void *arg) time_min = thread_info->time_present - thread_info->history_seconds + 1; time_max = thread_info->chart_threads[i]->time_max; + + if (thread_info->delete_old_data) { + /* A time window of twice the disk space is sufficient for compression space savings of up to 50% */ + time_approx_min = time_max - (default_rrdeng_disk_quota_mb * 2 * 1024 * 1024) / + (((uint64_t) DSET_DIMS * DSET_CHARTS) * sizeof(storage_number)); + time_min = MAX(time_min, time_approx_min); + } if (!time_max) { time_before = time_after = time_min; } else { @@ -2030,18 +2041,22 @@ static void query_dbengine_chart(void *arg) expected = unpack_storage_number(pack_storage_number((calculated_number) generatedv, SN_EXISTS)); if (unlikely(rd->state->query_ops.is_finished(&handle))) { - fprintf(stderr, " DB-engine stresstest %s/%s: at %lu secs, expecting value " - CALCULATED_NUMBER_FORMAT ", found data gap, ### E R R O R ###\n", - st->name, rd->name, (unsigned long) time_now, expected); - ++thread_info->errors; + if (!thread_info->delete_old_data) { /* data validation only when we don't delete */ + fprintf(stderr, " DB-engine stresstest %s/%s: at %lu secs, expecting value " + CALCULATED_NUMBER_FORMAT ", found data gap, ### E R R O R ###\n", + st->name, rd->name, (unsigned long) time_now, expected); + ++thread_info->errors; + } break; } n = rd->state->query_ops.next_metric(&handle, &time_retrieved); if (SN_EMPTY_SLOT == n) { - fprintf(stderr, " DB-engine stresstest %s/%s: at %lu secs, expecting value " - CALCULATED_NUMBER_FORMAT ", found data gap, ### E R R O R ###\n", - st->name, rd->name, (unsigned long) time_now, expected); - ++thread_info->errors; + if (!thread_info->delete_old_data) { /* data validation only when we don't delete */ + fprintf(stderr, " DB-engine stresstest %s/%s: at %lu secs, expecting value " + CALCULATED_NUMBER_FORMAT ", found data gap, ### E R R O R ###\n", + st->name, rd->name, (unsigned long) time_now, expected); + ++thread_info->errors; + } break; } ++thread_info->queried_metrics_nr; @@ -2049,15 +2064,21 @@ static void query_dbengine_chart(void *arg) same = (calculated_number_round(value) == calculated_number_round(expected)) ? 1 : 0; if (!same) { - fprintf(stderr, " DB-engine stresstest %s/%s: at %lu secs, expecting value " - CALCULATED_NUMBER_FORMAT ", found " CALCULATED_NUMBER_FORMAT ", ### E R R O R ###\n", - st->name, rd->name, (unsigned long) time_now, expected, value); - ++thread_info->errors; + if (!thread_info->delete_old_data) { /* data validation only when we don't delete */ + fprintf(stderr, " DB-engine stresstest %s/%s: at %lu secs, expecting value " + CALCULATED_NUMBER_FORMAT ", found " CALCULATED_NUMBER_FORMAT + ", ### E R R O R ###\n", + st->name, rd->name, (unsigned long) time_now, expected, value); + ++thread_info->errors; + } } if (time_retrieved != time_now) { - fprintf(stderr, " DB-engine stresstest %s/%s: at %lu secs, found timestamp %lu ### E R R O R ###\n", - st->name, rd->name, (unsigned long) time_now, (unsigned long) time_retrieved); - ++thread_info->errors; + if (!thread_info->delete_old_data) { /* data validation only when we don't delete */ + fprintf(stderr, + " DB-engine stresstest %s/%s: at %lu secs, found timestamp %lu ### E R R O R ###\n", + st->name, rd->name, (unsigned long) time_now, (unsigned long) time_retrieved); + ++thread_info->errors; + } } } rd->state->query_ops.finalize(&handle); @@ -2065,17 +2086,19 @@ static void query_dbengine_chart(void *arg) } void dbengine_stress_test(unsigned TEST_DURATION_SEC, unsigned DSET_CHARTS, unsigned QUERY_THREADS, - unsigned RAMP_UP_SECONDS, unsigned PAGE_CACHE_MB) + unsigned RAMP_UP_SECONDS, unsigned PAGE_CACHE_MB, unsigned DISK_SPACE_MB) { const unsigned DSET_DIMS = 128; const uint64_t EXPECTED_COMPRESSION_RATIO = 20; - const unsigned HISTORY_SECONDS = 3600 * 24 * 365; /* 1 year of history */ + const unsigned HISTORY_SECONDS = 3600 * 24 * 365 * 50; /* 50 year of history */ RRDHOST *host = NULL; struct dbengine_chart_thread **chart_threads; struct dbengine_query_thread **query_threads; unsigned i, j; time_t time_start, time_end; + error_log_limit_unlimited(); + if (!TEST_DURATION_SEC) TEST_DURATION_SEC = 10; if (!DSET_CHARTS) @@ -2087,13 +2110,18 @@ void dbengine_stress_test(unsigned TEST_DURATION_SEC, unsigned DSET_CHARTS, unsi default_rrd_memory_mode = RRD_MEMORY_MODE_DBENGINE; default_rrdeng_page_cache_mb = PAGE_CACHE_MB; - // Worst case for uncompressible data - default_rrdeng_disk_quota_mb = (((uint64_t)DSET_DIMS * DSET_CHARTS) * sizeof(storage_number) * HISTORY_SECONDS) / - (1024 * 1024); - default_rrdeng_disk_quota_mb -= default_rrdeng_disk_quota_mb * EXPECTED_COMPRESSION_RATIO / 100; + if (DISK_SPACE_MB) { + fprintf(stderr, "By setting disk space limit data are allowed to be deleted. " + "Data validation is turned off for this run.\n"); + default_rrdeng_disk_quota_mb = DISK_SPACE_MB; + } else { + // Worst case for uncompressible data + default_rrdeng_disk_quota_mb = + (((uint64_t) DSET_DIMS * DSET_CHARTS) * sizeof(storage_number) * HISTORY_SECONDS) / (1024 * 1024); + default_rrdeng_disk_quota_mb -= default_rrdeng_disk_quota_mb * EXPECTED_COMPRESSION_RATIO / 100; + } - error_log_limit_unlimited(); - debug(D_RRDHOST, "Initializing localhost with hostname 'dbengine-stress-test'"); + fprintf(stderr, "Initializing localhost with hostname 'dbengine-stress-test'\n"); host = dbengine_rrdhost_find_or_create("dbengine-stress-test"); if (NULL == host) @@ -2112,7 +2140,7 @@ void dbengine_stress_test(unsigned TEST_DURATION_SEC, unsigned DSET_CHARTS, unsi "%u MiB of page cache.\n", RAMP_UP_SECONDS, TEST_DURATION_SEC, DSET_CHARTS, QUERY_THREADS, PAGE_CACHE_MB); - time_start = now_realtime_sec(); + time_start = now_realtime_sec() + HISTORY_SECONDS; /* move history to the future */ for (i = 0 ; i < DSET_CHARTS ; ++i) { chart_threads[i]->host = host; chart_threads[i]->chartname = "random"; @@ -2146,6 +2174,7 @@ void dbengine_stress_test(unsigned TEST_DURATION_SEC, unsigned DSET_CHARTS, unsi for (j = 0 ; j < DSET_CHARTS ; ++j) { query_threads[i]->chart_threads[j] = chart_threads[j]; } + query_threads[i]->delete_old_data = DISK_SPACE_MB ? 1 : 0; assert(0 == uv_thread_create(&query_threads[i]->thread, query_dbengine_chart, query_threads[i])); } sleep(TEST_DURATION_SEC); diff --git a/daemon/unit_test.h b/daemon/unit_test.h index 230a700858..79d415be04 100644 --- a/daemon/unit_test.h +++ b/daemon/unit_test.h @@ -12,7 +12,7 @@ extern int unit_test_buffer(void); extern int test_dbengine(void); extern void generate_dbengine_dataset(unsigned history_seconds); extern void dbengine_stress_test(unsigned TEST_DURATION_SEC, unsigned DSET_CHARTS, unsigned QUERY_THREADS, - unsigned RAMP_UP_SECONDS, unsigned PAGE_CACHE_MB); + unsigned RAMP_UP_SECONDS, unsigned PAGE_CACHE_MB, unsigned DISK_SPACE_MB); #endif diff --git a/database/engine/pagecache.c b/database/engine/pagecache.c index 7ac4512d69..60fd7a5f89 100644 --- a/database/engine/pagecache.c +++ b/database/engine/pagecache.c @@ -101,6 +101,13 @@ void pg_cache_wake_up_waiters_unsafe(struct rrdeng_page_descr *descr) uv_cond_broadcast(&pg_cache_descr->cond); } +void pg_cache_wake_up_waiters(struct rrdengine_instance *ctx, struct rrdeng_page_descr *descr) +{ + rrdeng_page_descr_mutex_lock(ctx, descr); + pg_cache_wake_up_waiters_unsafe(descr); + rrdeng_page_descr_mutex_unlock(ctx, descr); +} + /* * The caller must hold page descriptor lock. * The lock will be released and re-acquired. The descriptor is not guaranteed @@ -135,10 +142,8 @@ unsigned long pg_cache_wait_event(struct rrdengine_instance *ctx, struct rrdeng_ /* * The caller must hold page descriptor lock. - * Gets a reference to the page descriptor. - * Returns 1 on success and 0 on failure. */ -int pg_cache_try_get_unsafe(struct rrdeng_page_descr *descr, int exclusive_access) +int pg_cache_can_get_unsafe(struct rrdeng_page_descr *descr, int exclusive_access) { struct page_cache_descr *pg_cache_descr = descr->pg_cache_descr; @@ -146,25 +151,25 @@ int pg_cache_try_get_unsafe(struct rrdeng_page_descr *descr, int exclusive_acces (exclusive_access && pg_cache_descr->refcnt)) { return 0; } - if (exclusive_access) - pg_cache_descr->flags |= RRD_PAGE_LOCKED; - ++pg_cache_descr->refcnt; return 1; } /* * The caller must hold page descriptor lock. - * Same return values as pg_cache_try_get_unsafe() without doing anything. + * Gets a reference to the page descriptor. + * Returns 1 on success and 0 on failure. */ -int pg_cache_can_get_unsafe(struct rrdeng_page_descr *descr, int exclusive_access) +int pg_cache_try_get_unsafe(struct rrdeng_page_descr *descr, int exclusive_access) { struct page_cache_descr *pg_cache_descr = descr->pg_cache_descr; - if ((pg_cache_descr->flags & (RRD_PAGE_LOCKED | RRD_PAGE_READ_PENDING)) || - (exclusive_access && pg_cache_descr->refcnt)) { + if (!pg_cache_can_get_unsafe(descr, exclusive_access)) return 0; - } + + if (exclusive_access) + pg_cache_descr->flags |= RRD_PAGE_LOCKED; + ++pg_cache_descr->refcnt; return 1; } @@ -409,7 +414,9 @@ void pg_cache_punch_hole(struct rrdengine_instance *ctx, struct rrdeng_page_desc print_page_cache_descr(descr); pg_cache_wait_event_unsafe(descr); } - if (!remove_dirty) { + if (remove_dirty) { + pg_cache_descr->flags &= ~RRD_PAGE_DIRTY; + } else { /* even a locked page could be dirty */ while (unlikely(pg_cache_descr->flags & RRD_PAGE_DIRTY)) { debug(D_RRDENGINE, "%s: Found dirty page, waiting for it to be flushed:", __func__); @@ -429,8 +436,11 @@ void pg_cache_punch_hole(struct rrdengine_instance *ctx, struct rrdeng_page_desc uv_rwlock_wrunlock(&pg_cache->pg_cache_rwlock); } pg_cache_put(ctx, descr); - if (descr->pg_cache_descr_state & PG_CACHE_DESCR_ALLOCATED) - rrdeng_try_deallocate_pg_cache_descr(ctx, descr); + rrdeng_try_deallocate_pg_cache_descr(ctx, descr); + while (descr->pg_cache_descr_state & PG_CACHE_DESCR_ALLOCATED) { + rrdeng_try_deallocate_pg_cache_descr(ctx, descr); /* spin */ + (void)sleep_usec(1000); /* 1 msec */ + } destroy: freez(descr); pg_cache_update_metric_times(page_index); diff --git a/database/engine/pagecache.h b/database/engine/pagecache.h index 7d8fa2a1df..9fd9991b5e 100644 --- a/database/engine/pagecache.h +++ b/database/engine/pagecache.h @@ -148,6 +148,7 @@ struct page_cache { /* TODO: add statistics */ }; extern void pg_cache_wake_up_waiters_unsafe(struct rrdeng_page_descr *descr); +extern void pg_cache_wake_up_waiters(struct rrdengine_instance *ctx, struct rrdeng_page_descr *descr); extern void pg_cache_wait_event_unsafe(struct rrdeng_page_descr *descr); extern unsigned long pg_cache_wait_event(struct rrdengine_instance *ctx, struct rrdeng_page_descr *descr); extern void pg_cache_replaceQ_insert(struct rrdengine_instance *ctx, diff --git a/database/engine/rrdengine.c b/database/engine/rrdengine.c index 9615665f85..3d49379048 100644 --- a/database/engine/rrdengine.c +++ b/database/engine/rrdengine.c @@ -121,19 +121,19 @@ after_crc_check: } else { (void) memcpy(page, uncompressed_buf + page_offset, descr->page_length); } - pg_cache_replaceQ_insert(ctx, descr); rrdeng_page_descr_mutex_lock(ctx, descr); pg_cache_descr = descr->pg_cache_descr; pg_cache_descr->page = page; pg_cache_descr->flags |= RRD_PAGE_POPULATED; pg_cache_descr->flags &= ~RRD_PAGE_READ_PENDING; - debug(D_RRDENGINE, "%s: Waking up waiters.", __func__); + rrdeng_page_descr_mutex_unlock(ctx, descr); + pg_cache_replaceQ_insert(ctx, descr); if (xt_io_descr->release_descr) { - pg_cache_put_unsafe(descr); + pg_cache_put(ctx, descr); } else { - pg_cache_wake_up_waiters_unsafe(descr); + debug(D_RRDENGINE, "%s: Waking up waiters.", __func__); + pg_cache_wake_up_waiters(ctx, descr); } - rrdeng_page_descr_mutex_unlock(ctx, descr); } if (!have_read_error && RRD_NO_COMPRESSION != header->compression_algorithm) { freez(uncompressed_buf); diff --git a/database/engine/rrdengineapi.c b/database/engine/rrdengineapi.c index f824728798..fd41a0cd89 100755 --- a/database/engine/rrdengineapi.c +++ b/database/engine/rrdengineapi.c @@ -473,17 +473,18 @@ storage_number rrdeng_load_metric_next(struct rrddim_query_handle *rrdimm_handle /* We need to get a new page */ if (descr) { /* Drop old page's reference */ - handle->next_page_time = (page_end_time / USEC_PER_SEC) + 1; - if (unlikely(handle->next_page_time > rrdimm_handle->end_time)) { - goto no_more_metrics; - } - next_page_time = handle->next_page_time * USEC_PER_SEC; #ifdef NETDATA_INTERNAL_CHECKS rrd_stat_atomic_add(&ctx->stats.metric_API_consumers, -1); #endif pg_cache_put(ctx, descr); handle->descr = NULL; + handle->next_page_time = (page_end_time / USEC_PER_SEC) + 1; + if (unlikely(handle->next_page_time > rrdimm_handle->end_time)) { + goto no_more_metrics; + } + next_page_time = handle->next_page_time * USEC_PER_SEC; } + descr = pg_cache_lookup_next(ctx, handle->page_index, &handle->page_index->id, next_page_time, rrdimm_handle->end_time * USEC_PER_SEC); if (NULL == descr) { diff --git a/database/engine/rrdenglocking.c b/database/engine/rrdenglocking.c index 754d802a23..0c1d26f25a 100644 --- a/database/engine/rrdenglocking.c +++ b/database/engine/rrdenglocking.c @@ -20,12 +20,7 @@ struct page_cache_descr *rrdeng_create_pg_cache_descr(struct rrdengine_instance void rrdeng_destroy_pg_cache_descr(struct rrdengine_instance *ctx, struct page_cache_descr *pg_cache_descr) { - /* Flush any lock and condition variable users */ - uv_mutex_lock(&pg_cache_descr->mutex); - uv_mutex_unlock(&pg_cache_descr->mutex); - uv_cond_destroy(&pg_cache_descr->cond); - uv_mutex_destroy(&pg_cache_descr->mutex); freez(pg_cache_descr); rrd_stat_atomic_add(&ctx->stats.page_cache_descriptors, -1); @@ -100,7 +95,7 @@ void rrdeng_page_descr_mutex_lock(struct rrdengine_instance *ctx, struct rrdeng_ void rrdeng_page_descr_mutex_unlock(struct rrdengine_instance *ctx, struct rrdeng_page_descr *descr) { unsigned long old_state, new_state, ret_state, old_users; - struct page_cache_descr *pg_cache_descr; + struct page_cache_descr *pg_cache_descr, *delete_pg_cache_descr = NULL; uint8_t we_locked; uv_mutex_unlock(&descr->pg_cache_descr->mutex); @@ -116,7 +111,8 @@ void rrdeng_page_descr_mutex_unlock(struct rrdengine_instance *ctx, struct rrden ret_state = ulong_compare_and_swap(&descr->pg_cache_descr_state, old_state, 0); if (old_state == ret_state) { /* success */ - break; + rrdeng_destroy_pg_cache_descr(ctx, delete_pg_cache_descr); + return; } continue; /* spin */ } @@ -128,12 +124,15 @@ void rrdeng_page_descr_mutex_unlock(struct rrdengine_instance *ctx, struct rrden pg_cache_descr = descr->pg_cache_descr; /* caller is the only page cache descriptor user and there are no pending references on the page */ if ((old_state & PG_CACHE_DESCR_DESTROY) && (1 == old_users) && - !pg_cache_descr->flags && !pg_cache_descr->refcnt && !pg_cache_descr->waiters) { + !pg_cache_descr->flags && !pg_cache_descr->refcnt) { + assert(!pg_cache_descr->waiters); + new_state = PG_CACHE_DESCR_LOCKED; ret_state = ulong_compare_and_swap(&descr->pg_cache_descr_state, old_state, new_state); if (old_state == ret_state) { we_locked = 1; - rrdeng_destroy_pg_cache_descr(ctx, pg_cache_descr); + delete_pg_cache_descr = pg_cache_descr; + descr->pg_cache_descr = NULL; /* retry */ continue; } @@ -150,7 +149,6 @@ void rrdeng_page_descr_mutex_unlock(struct rrdengine_instance *ctx, struct rrden } /* spin */ } - } /* @@ -161,11 +159,11 @@ void rrdeng_page_descr_mutex_unlock(struct rrdengine_instance *ctx, struct rrden void rrdeng_try_deallocate_pg_cache_descr(struct rrdengine_instance *ctx, struct rrdeng_page_descr *descr) { unsigned long old_state, new_state, ret_state, old_users; - struct page_cache_descr *pg_cache_descr; - uint8_t just_locked, we_freed, must_unlock; + struct page_cache_descr *pg_cache_descr = NULL; + uint8_t just_locked, can_free, must_unlock; just_locked = 0; - we_freed = 0; + can_free = 0; must_unlock = 0; while (1) { /* spin */ old_state = descr->pg_cache_descr_state; @@ -177,9 +175,11 @@ void rrdeng_try_deallocate_pg_cache_descr(struct rrdengine_instance *ctx, struct must_unlock = 1; just_locked = 0; /* Try deallocate if there are no pending references on the page */ - if (!pg_cache_descr->flags && !pg_cache_descr->refcnt && !pg_cache_descr->waiters) { - rrdeng_destroy_pg_cache_descr(ctx, pg_cache_descr); - we_freed = 1; + if (!pg_cache_descr->flags && !pg_cache_descr->refcnt) { + assert(!pg_cache_descr->waiters); + + descr->pg_cache_descr = NULL; + can_free = 1; /* success */ continue; } @@ -188,7 +188,7 @@ void rrdeng_try_deallocate_pg_cache_descr(struct rrdengine_instance *ctx, struct if (unlikely(must_unlock)) { assert(0 == old_users); - if (we_freed) { + if (can_free) { /* success */ new_state = 0; } else { @@ -198,6 +198,8 @@ void rrdeng_try_deallocate_pg_cache_descr(struct rrdengine_instance *ctx, struct ret_state = ulong_compare_and_swap(&descr->pg_cache_descr_state, old_state, new_state); if (old_state == ret_state) { /* unlocked */ + if (can_free) + rrdeng_destroy_pg_cache_descr(ctx, pg_cache_descr); return; } continue; /* spin */ |