From cfafbd04cc4339f3055d48300deb3c34d3cbae65 Mon Sep 17 00:00:00 2001 From: Piotr Balcer Date: Thu, 20 Aug 2020 12:12:27 +0200 Subject: [PATCH 1/4] obj: fix failure atomicity bug in huge allocs This is a regression introduced in 5e137e0. That patch accidentally modified the order in which huge chunk headers are updated, leading to possible issue with failure atomicity where the chunk headers could be inconsistent if a crash happened in between two header updates. --- src/libpmemobj/heap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libpmemobj/heap.c b/src/libpmemobj/heap.c index 08dc7653ed4..66d7ad03c9e 100644 --- a/src/libpmemobj/heap.c +++ b/src/libpmemobj/heap.c @@ -928,11 +928,11 @@ heap_split_block(struct palloc_heap *heap, struct bucket *b, uint32_t new_chunk_id = m->chunk_id + units; uint32_t new_size_idx = m->size_idx - units; - *m = memblock_huge_init(heap, m->chunk_id, m->zone_id, units); - struct memory_block n = memblock_huge_init(heap, new_chunk_id, m->zone_id, new_size_idx); + *m = memblock_huge_init(heap, m->chunk_id, m->zone_id, units); + if (bucket_insert_block(b, &n) != 0) LOG(2, "failed to allocate memory block runtime tracking info"); From dd50012720b6419786a139f2dfefaeb4235e941f Mon Sep 17 00:00:00 2001 From: Piotr Balcer Date: Mon, 24 Aug 2020 17:08:54 +0200 Subject: [PATCH 2/4] obj: add missing drain after ulog processing This missing drain might have lead to failure atomicity bug if the store that invalidated the ulog was reordered by the CPU with any of the log entries and a crash happened. --- src/libpmemobj/memops.c | 10 ++++++++++ src/libpmemobj/tx.c | 3 ++- src/libpmemobj/ulog.c | 2 ++ src/test/obj_persist_count/out1.log.match | 8 ++++---- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/libpmemobj/memops.c b/src/libpmemobj/memops.c index 1ff9f46b0ea..b805275a63a 100644 --- a/src/libpmemobj/memops.c +++ b/src/libpmemobj/memops.c @@ -166,6 +166,14 @@ operation_transient_clean(void *base, const void *addr, size_t len, return 0; } +/* + * operation_transient_drain -- noop + */ +static void +operation_transient_drain(void *base) +{ +} + /* * operation_transient_memcpy -- transient memcpy wrapper */ @@ -210,10 +218,12 @@ operation_new(struct ulog *ulog, size_t ulog_base_nbytes, ctx->t_ops.base = NULL; ctx->t_ops.flush = operation_transient_clean; ctx->t_ops.memcpy = operation_transient_memcpy; + ctx->t_ops.drain = operation_transient_drain; ctx->s_ops.base = p_ops->base; ctx->s_ops.flush = operation_transient_clean; ctx->s_ops.memcpy = operation_transient_memcpy; + ctx->s_ops.drain = operation_transient_drain; VECQ_INIT(&ctx->merge_entries); diff --git a/src/libpmemobj/tx.c b/src/libpmemobj/tx.c index 7b77dd19cae..beea30c13e9 100644 --- a/src/libpmemobj/tx.c +++ b/src/libpmemobj/tx.c @@ -1,5 +1,5 @@ /* - * Copyright 2015-2019, Intel Corporation + * Copyright 2015-2020, Intel Corporation * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -411,6 +411,7 @@ tx_abort_set(PMEMobjpool *pop, struct lane *lane) ulog_foreach_entry((struct ulog *)&lane->layout->undo, tx_undo_entry_apply, NULL, &pop->p_ops); + pmemops_drain(&pop->p_ops); operation_finish(lane->undo, ULOG_INC_FIRST_GEN_NUM); } diff --git a/src/libpmemobj/ulog.c b/src/libpmemobj/ulog.c index 90dc77a7951..db0301584c2 100644 --- a/src/libpmemobj/ulog.c +++ b/src/libpmemobj/ulog.c @@ -38,6 +38,7 @@ #include #include "libpmemobj.h" +#include "pmemops.h" #include "ulog.h" #include "out.h" #include "util.h" @@ -815,6 +816,7 @@ ulog_process(struct ulog *ulog, ulog_check_offset_fn check, #endif ulog_foreach_entry(ulog, ulog_process_entry, NULL, p_ops); + pmemops_drain(p_ops); } /* diff --git a/src/test/obj_persist_count/out1.log.match b/src/test/obj_persist_count/out1.log.match index f21a163d2aa..05e34801246 100644 --- a/src/test/obj_persist_count/out1.log.match +++ b/src/test/obj_persist_count/out1.log.match @@ -3,7 +3,7 @@ obj_persist_count$(nW)TEST1: START: obj_persist_count task cl(all) drain(all) pmem_persist pmem_msync pmem_flush pmem_drain pmem_memcpy_cls pmem_memcpy_drain pmem_memset_cls pmem_memset_drain potential_cache_misses $(OPT)pool_create 49282 14 11 0 0 0 0 0 11 3 49275 $(OPX)pool_create 49602 24 11 5 0 5 0 0 11 3 49595 -root_alloc 9 4 0 0 3 1 4 2 2 1 5 +root_alloc 9 5 0 0 3 2 4 2 2 1 5 atomic_alloc 2 2 1 0 0 1 1 0 0 0 1 atomic_free 1 2 1 0 0 1 0 0 0 0 1 tx_begin_end 0 2 0 0 0 2 0 0 0 0 0 @@ -13,10 +13,10 @@ 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 178 13 6 0 4 3 165 2 3 2 10 +tx_add_large 178 14 6 0 4 4 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 6 4 0 0 2 2 4 2 0 0 2 +pfree 5 4 0 0 2 2 3 2 0 0 2 pmalloc_stack 2 2 1 0 0 1 1 0 0 0 1 pfree_stack 1 2 1 0 0 1 0 0 0 0 1 obj_persist_count$(nW)TEST1: DONE From afff09f6e8871b96e5806857eceb6edf155a9fc9 Mon Sep 17 00:00:00 2001 From: Piotr Balcer Date: Mon, 24 Aug 2020 10:41:56 +0200 Subject: [PATCH 3/4] test: (bash) fix pmreorder error code checking This prevented pmreorder tests from failling in certain scenarios. --- src/test/unittest/unittest.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/unittest/unittest.sh b/src/test/unittest/unittest.sh index 59ff9c8a55b..ebbbafa208d 100644 --- a/src/test/unittest/unittest.sh +++ b/src/test/unittest/unittest.sh @@ -3482,7 +3482,7 @@ function pmreorder_run_tool() # function pmreorder_expect_success() { - ret=$(pmreorder_run_tool "$@") + ret=$(pmreorder_run_tool "$@" | tail -n1) if [ "$ret" -ne "0" ]; then msg=$(interactive_red STDERR "failed with exit code $ret") @@ -3504,7 +3504,7 @@ function pmreorder_expect_success() # function pmreorder_expect_failure() { - ret=$(pmreorder_run_tool "$@") + ret=$(pmreorder_run_tool "$@" | tail -n1) if [ "$ret" -eq "0" ]; then msg=$(interactive_red STDERR "succeeded") From a5e5bfbc152d97ef1e4313fa4d8710af49aec839 Mon Sep 17 00:00:00 2001 From: Piotr Balcer Date: Mon, 24 Aug 2020 10:42:49 +0200 Subject: [PATCH 4/4] test: add copy-on-write option to obj reorder test Without this option, the consistency checking function can mask issues. --- src/test/obj_reorder_basic/obj_reorder_basic.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/obj_reorder_basic/obj_reorder_basic.c b/src/test/obj_reorder_basic/obj_reorder_basic.c index d2ef37e7010..d08b12b2b85 100644 --- a/src/test/obj_reorder_basic/obj_reorder_basic.c +++ b/src/test/obj_reorder_basic/obj_reorder_basic.c @@ -1,5 +1,5 @@ /* - * Copyright 2018, Intel Corporation + * Copyright 2018-2020, Intel Corporation * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -99,10 +99,16 @@ main(int argc, char *argv[]) if (argc != 3 || strchr("wc", argv[1][0]) == 0 || argv[1][1] != '\0') UT_FATAL("usage: %s w|c file", argv[0]); + char opt = argv[1][0]; + if (opt == 'c') { + int y = 1; + pmemobj_ctl_set(NULL, "copy_on_write.at_open", &y); + } + PMEMobjpool *pop = pmemobj_open(argv[2], LAYOUT_NAME); UT_ASSERT(pop != NULL); - char opt = argv[1][0]; + VALGRIND_EMIT_LOG("PMREORDER_MARKER_WRITE.BEGIN"); switch (opt) { case 'w':