diff --git a/src/libpmemobj/memops.c b/src/libpmemobj/memops.c index 04fa41f22584490baf22a64138da3cfb35c1e9f2..48c5940334e2c9733318d3144699c6778a96b211 100644 --- a/src/libpmemobj/memops.c +++ b/src/libpmemobj/memops.c @@ -397,6 +397,25 @@ operation_add_buffer(struct operation_context *ctx, size_t curr_size = MIN(real_size, ctx->ulog_curr_capacity); size_t data_size = curr_size - sizeof(struct ulog_entry_buf); + size_t entry_size = ALIGN_UP(curr_size, CACHELINE_SIZE); + + /* + * To make sure that the log is consistent and contiguous, we need + * make sure that the header of the entry that would be located + * immediately after this one is zeroed. + */ + struct ulog_entry_base *next_entry = NULL; + if (entry_size == ctx->ulog_curr_capacity) { + struct ulog *u = ulog_next(ctx->ulog_curr, ctx->p_ops); + if (u != NULL) + next_entry = (struct ulog_entry_base *)u->data; + } else { + size_t next_entry_offset = ctx->ulog_curr_offset + entry_size; + next_entry = (struct ulog_entry_base *)(ctx->ulog_curr->data + + next_entry_offset); + } + if (next_entry != NULL) + ulog_clobber_entry(next_entry, ctx->p_ops); /* create a persistent log entry */ struct ulog_entry_buf *e = ulog_entry_buf_create(ctx->ulog_curr, @@ -404,7 +423,6 @@ operation_add_buffer(struct operation_context *ctx, ctx->ulog_curr_gen_num, dest, src, data_size, type, ctx->p_ops); - size_t entry_size = ALIGN_UP(curr_size, CACHELINE_SIZE); ASSERT(entry_size == ulog_entry_size(&e->base)); ASSERT(entry_size <= ctx->ulog_curr_capacity); diff --git a/src/libpmemobj/ulog.c b/src/libpmemobj/ulog.c index 2bbe09b5d32567c67fd1b25f21c1b04f7d44551e..5327f27e4bf4965c6b69c759b33889e6256dc953 100644 --- a/src/libpmemobj/ulog.c +++ b/src/libpmemobj/ulog.c @@ -174,10 +174,13 @@ ulog_construct(uint64_t offset, size_t capacity, uint64_t gen_num, ulog->gen_num = gen_num; memset(ulog->unused, 0, sizeof(ulog->unused)); + /* we only need to zero out the header of ulog's first entry */ + size_t zeroed_data = CACHELINE_ALIGN(sizeof(struct ulog_entry_base)); + if (flush) { pmemops_xflush(p_ops, ulog, sizeof(*ulog), PMEMOBJ_F_RELAXED); - pmemops_memset(p_ops, ulog->data, 0, capacity, + pmemops_memset(p_ops, ulog->data, 0, zeroed_data, PMEMOBJ_F_MEM_NONTEMPORAL | PMEMOBJ_F_MEM_NODRAIN | PMEMOBJ_F_RELAXED); @@ -186,7 +189,7 @@ ulog_construct(uint64_t offset, size_t capacity, uint64_t gen_num, * We want to avoid replicating zeroes for every ulog of every * lane, to do that, we need to use plain old memset. */ - memset(ulog->data, 0, capacity); + memset(ulog->data, 0, zeroed_data); } VALGRIND_REMOVE_FROM_TX(ulog, SIZEOF_ULOG(capacity)); @@ -400,6 +403,22 @@ ulog_entry_val_create(struct ulog *ulog, size_t offset, uint64_t *dest, return e; } +/* + * ulog_clobber_entry -- zeroes out a single log entry header + */ +void +ulog_clobber_entry(const struct ulog_entry_base *e, + const struct pmem_ops *p_ops) +{ + static const size_t aligned_entry_size = + CACHELINE_ALIGN(sizeof(struct ulog_entry_base)); + + VALGRIND_ADD_TO_TX(e, aligned_entry_size); + pmemops_memset(p_ops, (char *)e, 0, aligned_entry_size, + PMEMOBJ_F_MEM_NONTEMPORAL); + VALGRIND_REMOVE_FROM_TX(e, aligned_entry_size); +} + /* * ulog_entry_buf_create -- atomically creates a buffer entry in the log */ @@ -630,35 +649,6 @@ ulog_clobber_data(struct ulog *ulog_first, { ASSERTne(ulog_first, NULL); - /* --------------- */ - /* - * This is a quick workaround for issue injected in commit: - * 2ab13304664b353b82730f49b78fc67eea33b25b (ulog-invalidation). - * This patrt of the code for sure impact performance, - * and needs to be removed when proper fix will be implemented. - */ - size_t rcapacity = ulog_base_nbytes; - size_t numlog = 0; - - for (struct ulog *r = ulog_first; r != NULL; ) { - size_t nzero = MIN(nbytes, rcapacity); - VALGRIND_ADD_TO_TX(r->data, nzero); - pmemops_memset(p_ops, r->data, 0, nzero, PMEMOBJ_F_MEM_WC); - VALGRIND_ADD_TO_TX(r->data, nzero); - nbytes -= nzero; - - if (nbytes == 0) - break; - - r = ulog_by_offset(VEC_ARR(next)[numlog++], p_ops); - if (numlog > 1) - break; - - ASSERTne(r, NULL); - rcapacity = r->capacity; - } - /* -------------- */ - /* In case of abort we need to increment counter in the first ulog. */ if (flags & ULOG_INC_FIRST_GEN_NUM) ulog_inc_gen_num(ulog_first, p_ops); diff --git a/src/libpmemobj/ulog.h b/src/libpmemobj/ulog.h index 982a99a01ced890723cd3f2d8f82bca9cec2af4e..c1b3a075c7b12bc70df2db397f6ea009cc1935b5 100644 --- a/src/libpmemobj/ulog.h +++ b/src/libpmemobj/ulog.h @@ -140,6 +140,8 @@ int ulog_clobber_data(struct ulog *dest, size_t nbytes, size_t ulog_base_nbytes, struct ulog_next *next, ulog_free_fn ulog_free, const struct pmem_ops *p_ops, unsigned flags); +void ulog_clobber_entry(const struct ulog_entry_base *e, + const struct pmem_ops *p_ops); void ulog_process(struct ulog *ulog, ulog_check_offset_fn check, const struct pmem_ops *p_ops); diff --git a/src/test/obj_memops/obj_memops.c b/src/test/obj_memops/obj_memops.c index 9be227c441c4970d9514a82a8b36a1a218ca4584..43199d669be9dc5314db7df61d6f8645d93a3f1d 100644 --- a/src/test/obj_memops/obj_memops.c +++ b/src/test/obj_memops/obj_memops.c @@ -273,6 +273,32 @@ test_undo_small_single_set(struct operation_context *ctx, operation_finish(ctx, ULOG_INC_FIRST_GEN_NUM); } +static void +test_undo_small_multiple_set(struct operation_context *ctx, + struct test_object *object) +{ + operation_start(ctx); + + object->values[0] = 1; + object->values[1] = 2; + + int c = 0; + + operation_add_buffer(ctx, + &object->values[0], &c, sizeof(*object->values), + ULOG_OPERATION_BUF_SET); + operation_add_buffer(ctx, + &object->values[1], &c, sizeof(*object->values), + ULOG_OPERATION_BUF_SET); + + operation_process(ctx); + + UT_ASSERTeq(object->values[0], 0); + UT_ASSERTeq(object->values[1], 0); + + operation_finish(ctx, ULOG_INC_FIRST_GEN_NUM); +} + static void test_undo_large_single_copy(struct operation_context *ctx, struct test_object *object) @@ -316,6 +342,7 @@ test_undo_checksum_mismatch(PMEMobjpool *pop, struct operation_context *ctx, pmemobj_persist(pop, &object->values, sizeof(*object->values) * 20); log->data[100] += 1; /* corrupt the log somewhere */ + pmemobj_persist(pop, &log->data[100], sizeof(log->data[100])); operation_process(ctx); @@ -375,6 +402,135 @@ test_undo_large_copy(PMEMobjpool *pop, struct operation_context *ctx, operation_finish(ctx, ULOG_INC_FIRST_GEN_NUM); } +static int +test_undo_foreach(struct ulog_entry_base *e, void *arg, + const struct pmem_ops *p_ops) +{ + size_t *nentries = arg; + ++(*nentries); + + return 0; +} + +/* + * drain_empty -- drain for pmem_ops + */ +static void +drain_empty(void *ctx) +{ + /* do nothing */ +} + +/* + * persist_empty -- persist for pmem_ops + */ +static int +persist_empty(void *ctx, const void *addr, size_t len, unsigned flags) +{ + return 0; +} + +/* + * flush_empty -- flush for pmem_ops + */ +static int +flush_empty(void *ctx, const void *addr, size_t len, unsigned flags) +{ + return 0; +} + +/* + * memcpy_libc -- memcpy for pmem_ops + */ +static void * +memcpy_libc(void *ctx, void *dest, const void *src, size_t len, unsigned flags) +{ + return memcpy(dest, src, len); +} + +/* + * memset_libc -- memset for pmem_ops + */ +static void * +memset_libc(void *ctx, void *ptr, int c, size_t sz, unsigned flags) +{ + return memset(ptr, c, sz); +} + +/* + * test_undo_log_reuse -- test for correct reuse of log space + */ +static void +test_undo_log_reuse() +{ +#define ULOG_SIZE 1024 + struct pmem_ops ops = { + .persist = persist_empty, + .flush = flush_empty, + .drain = drain_empty, + .memcpy = memcpy_libc, + .memmove = NULL, + .memset = memset_libc, + .base = NULL, + }; + struct ULOG(ULOG_SIZE) *first = util_aligned_malloc(64, + SIZEOF_ULOG(ULOG_SIZE)); + struct ULOG(ULOG_SIZE) *second = util_aligned_malloc(64, + SIZEOF_ULOG(ULOG_SIZE)); + ulog_construct((uint64_t)(first), ULOG_SIZE, 0, 0, &ops); + ulog_construct((uint64_t)(second), ULOG_SIZE, 0, 0, &ops); + + first->next = (uint64_t)(second); + + struct operation_context *ctx = operation_new( + (struct ulog *)first, ULOG_SIZE, + NULL, NULL, + &ops, LOG_TYPE_UNDO); + + size_t nentries = 0; + ulog_foreach_entry((struct ulog *)first, + test_undo_foreach, &nentries, &ops); + UT_ASSERTeq(nentries, 0); + + /* first, let's populate the log with some valid entries */ + + size_t entry_size = (ULOG_SIZE / 2) - sizeof(struct ulog_entry_buf); + size_t total_entries = ((ULOG_SIZE * 2) / entry_size); + char *data = MALLOC(entry_size); + memset(data, 0xc, entry_size); /* fill it with something */ + + for (size_t i = 0; i < total_entries; ++i) { + operation_add_buffer(ctx, (void *)0x123, data, + entry_size, + ULOG_OPERATION_BUF_CPY); + + nentries = 0; + ulog_foreach_entry((struct ulog *)first, + test_undo_foreach, &nentries, &ops); + UT_ASSERTeq(nentries, i + 1); + } + + operation_init(ctx); /* initialize a new operation */ + + /* let's overwrite old entries and see if they are no longer visible */ + for (size_t i = 0; i < total_entries; ++i) { + operation_add_buffer(ctx, (void *)0x123, data, + entry_size, + ULOG_OPERATION_BUF_CPY); + + nentries = 0; + ulog_foreach_entry((struct ulog *)first, + test_undo_foreach, &nentries, &ops); + UT_ASSERTeq(nentries, i + 1); + } + + FREE(data); + operation_delete(ctx); + util_aligned_free(first); + util_aligned_free(second); +#undef ULOG_SIZE +} + static void test_undo(PMEMobjpool *pop, struct test_object *object) { @@ -385,6 +541,7 @@ test_undo(PMEMobjpool *pop, struct test_object *object) test_undo_small_single_copy(ctx, object); test_undo_small_single_set(ctx, object); + test_undo_small_multiple_set(ctx, object); test_undo_large_single_copy(ctx, object); test_undo_large_copy(pop, ctx, object); test_undo_checksum_mismatch(pop, ctx, object, @@ -424,6 +581,7 @@ main(int argc, char *argv[]) test_redo(pop, object); test_undo(pop, object); + test_undo_log_reuse(); pmemobj_close(pop); diff --git a/src/test/obj_persist_count/out0.log.match b/src/test/obj_persist_count/out0.log.match index 83dfe7657df9d408d67d607c590fdd63a443b6eb..6ae0bf0546f4301bc9b6cd5a564d291deabbb3c0 100644 --- a/src/test/obj_persist_count/out0.log.match +++ b/src/test/obj_persist_count/out0.log.match @@ -13,8 +13,8 @@ tx_free 64 1 0 1 0 0 tx_free_next 64 1 0 1 0 0 0 0 0 0 64 tx_add 194 3 0 3 0 0 0 0 0 0 194 tx_add_next 194 3 0 3 0 0 0 0 0 0 194 -tx_add_large 2952 21 0 21 0 0 0 0 0 0 2952 -tx_add_lnext 1090 8 0 8 0 0 0 0 0 0 1090 +tx_add_large 1640 21 0 21 0 0 0 0 0 0 1640 +tx_add_lnext 803 8 0 8 0 0 0 0 0 0 803 pmalloc 324 5 0 5 0 0 0 0 0 0 324 pfree 259 4 0 4 0 0 0 0 0 0 259 pmalloc_stack 129 2 0 2 0 0 0 0 0 0 129 diff --git a/src/test/obj_persist_count/out1.log.match b/src/test/obj_persist_count/out1.log.match index 6f517be7fc051985a42a7a5195cfecfb077534a3..f21a163d2aa1a37b7879f49868ce844df4f1e505 100644 --- a/src/test/obj_persist_count/out1.log.match +++ b/src/test/obj_persist_count/out1.log.match @@ -13,8 +13,8 @@ tx_free 1 1 1 0 0 0 tx_free_next 1 1 1 0 0 0 0 0 0 0 1 tx_add 3 3 1 0 0 1 1 0 1 1 1 tx_add_next 3 3 1 0 0 1 1 0 1 1 1 -tx_add_large 850 13 6 0 4 3 165 2 675 2 10 -tx_add_lnext 323 5 1 0 0 2 161 0 161 2 1 +tx_add_large 178 13 6 0 4 3 165 2 3 2 10 +tx_add_lnext 164 5 1 0 0 2 161 0 2 2 1 pmalloc 6 3 0 0 2 1 4 2 0 0 2 pfree 5 3 0 0 2 1 3 2 0 0 2 pmalloc_stack 2 2 1 0 0 1 1 0 0 0 1