From be023c936f4fbd56c64af06dc1aaf3d420b3eecf Mon Sep 17 00:00:00 2001 From: Bruno Faccini Date: Mon, 13 Feb 2023 14:46:51 +0100 Subject: [PATCH 1/4] obj: handle ENOMEM during vector allocations When missing, Handle ENOMEM during vector allocations to avoid later crashing. Ref: pmem/issues#5515 Signed-off-by: Bruno Faccini --- src/libpmemobj/memops.c | 8 +++++--- src/libpmemobj/memops.h | 4 ++-- src/libpmemobj/recycler.c | 5 +++-- src/libpmemobj/tx.c | 19 ++++++++++++++----- src/libpmemobj/ulog.c | 6 +++--- 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/libpmemobj/memops.c b/src/libpmemobj/memops.c index fc5fe7d227a..82ae8ea612b 100644 --- a/src/libpmemobj/memops.c +++ b/src/libpmemobj/memops.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: BSD-3-Clause -/* Copyright 2016-2022, Intel Corporation */ +/* Copyright 2016-2023, Intel Corporation */ /* * memops.c -- aggregated memory operations helper implementation @@ -590,7 +590,7 @@ operation_user_buffer_verify_align(struct operation_context *ctx, /* * operation_add_user_buffer -- add user buffer to the ulog */ -void +int operation_add_user_buffer(struct operation_context *ctx, struct user_buffer_def *userbuf) { @@ -614,9 +614,11 @@ operation_add_user_buffer(struct operation_context *ctx, last_log->next = buffer_offset; pmemops_persist(ctx->p_ops, &last_log->next, next_size); - VEC_PUSH_BACK(&ctx->next, buffer_offset); + if (VEC_PUSH_BACK(&ctx->next, buffer_offset) != 0) + return -1; ctx->ulog_capacity += capacity; operation_set_any_user_buffer(ctx, 1); + return 0; } /* diff --git a/src/libpmemobj/memops.h b/src/libpmemobj/memops.h index b05948ed08c..04192d0f3a3 100644 --- a/src/libpmemobj/memops.h +++ b/src/libpmemobj/memops.h @@ -1,5 +1,5 @@ /* SPDX-License-Identifier: BSD-3-Clause */ -/* Copyright 2016-2020, Intel Corporation */ +/* Copyright 2016-2023, Intel Corporation */ /* * memops.h -- aggregated memory operations helper definitions @@ -63,7 +63,7 @@ int operation_add_typed_entry(struct operation_context *ctx, ulog_operation_type type, enum operation_log_type log_type); int operation_user_buffer_verify_align(struct operation_context *ctx, struct user_buffer_def *userbuf); -void operation_add_user_buffer(struct operation_context *ctx, +int operation_add_user_buffer(struct operation_context *ctx, struct user_buffer_def *userbuf); void operation_set_auto_reserve(struct operation_context *ctx, int auto_reserve); diff --git a/src/libpmemobj/recycler.c b/src/libpmemobj/recycler.c index 4e5b83eb13e..916ae4a6033 100644 --- a/src/libpmemobj/recycler.c +++ b/src/libpmemobj/recycler.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: BSD-3-Clause -/* Copyright 2016-2021, Intel Corporation */ +/* Copyright 2016-2023, Intel Corporation */ /* * recycler.c -- implementation of run recycler @@ -274,7 +274,8 @@ recycler_recalc(struct recycler *r, int force) if (VEC_PUSH_BACK(&runs, nm) != 0) ASSERT(0); /* XXX: fix after refactoring */ } else { - VEC_PUSH_BACK(&r->recalc, e); + if (VEC_PUSH_BACK(&r->recalc, e) != 0) + ASSERT(0); /* XXX: fix after refactoring */ } } while (found_units < search_limit); diff --git a/src/libpmemobj/tx.c b/src/libpmemobj/tx.c index b119230dac9..3f2e7616869 100644 --- a/src/libpmemobj/tx.c +++ b/src/libpmemobj/tx.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: BSD-3-Clause -/* Copyright 2015-2021, Intel Corporation */ +/* Copyright 2015-2023, Intel Corporation */ /* * tx.c -- transactions implementation @@ -194,7 +194,8 @@ tx_action_add(struct tx *tx) if (tx_action_reserve(tx, 1) != 0) return NULL; - VEC_INC_BACK(&tx->actions); + if (VEC_INC_BACK(&tx->actions) == -1) + return NULL; return &VEC_BACK(&tx->actions); } @@ -710,7 +711,8 @@ tx_construct_user_buffer(struct tx *tx, void *addr, size_t size, tx->redo_userbufs_capacity += userbuf.size - TX_INTENT_LOG_BUFFER_OVERHEAD; } else { - operation_add_user_buffer(ctx, &userbuf); + if (operation_add_user_buffer(ctx, &userbuf) == -1) + goto err; } return 0; @@ -1013,8 +1015,11 @@ pmemobj_tx_commit(void) operation_start(tx->lane->external); struct user_buffer_def *userbuf; + struct operation_context *ctx = tx->lane->external; VEC_FOREACH_BY_PTR(userbuf, &tx->redo_userbufs) - operation_add_user_buffer(tx->lane->external, userbuf); + if (operation_add_user_buffer(ctx, userbuf) == -1) + FATAL("%s: failed to allocate the next vector", + __func__); palloc_publish(&pop->heap, VEC_ARR(&tx->actions), VEC_SIZE(&tx->actions), tx->lane->external); @@ -1921,7 +1926,11 @@ pmemobj_tx_xpublish(struct pobj_action *actv, size_t actvcnt, uint64_t flags) } for (size_t i = 0; i < actvcnt; ++i) { - VEC_PUSH_BACK(&tx->actions, actv[i]); + if (VEC_PUSH_BACK(&tx->actions, actv[i]) != 0) { + int ret = obj_tx_fail_err(ENOMEM, flags); + PMEMOBJ_API_END(); + return ret; + } } PMEMOBJ_API_END(); diff --git a/src/libpmemobj/ulog.c b/src/libpmemobj/ulog.c index 80569373d85..8975ff4fcbe 100644 --- a/src/libpmemobj/ulog.c +++ b/src/libpmemobj/ulog.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: BSD-3-Clause -/* Copyright 2015-2021, Intel Corporation */ +/* Copyright 2015-2023, Intel Corporation */ /* * ulog.c -- unified log implementation @@ -225,7 +225,7 @@ ulog_rebuild_next_vec(struct ulog *ulog, struct ulog_next *next, { do { if (ulog->next != 0) - VEC_PUSH_BACK(next, ulog->next); + ASSERT(VEC_PUSH_BACK(next, ulog->next) == 0); } while ((ulog = ulog_next(ulog, p_ops)) != NULL); } @@ -257,7 +257,7 @@ ulog_reserve(struct ulog *ulog, while (capacity < *new_capacity) { if (extend(p_ops->base, &ulog->next, gen_num) != 0) return -1; - VEC_PUSH_BACK(next, ulog->next); + ASSERT(VEC_PUSH_BACK(next, ulog->next) == 0); ulog = ulog_next(ulog, p_ops); ASSERTne(ulog, NULL); From 5a2cc1b4d1bc45e0f95d15c9fca5cb84b5e770aa Mon Sep 17 00:00:00 2001 From: Bruno Faccini Date: Thu, 6 Apr 2023 16:06:58 +0200 Subject: [PATCH 2/4] obj: do not embed VEC_PUSH_BACK() in ASSERT() Looks like doing so affects VEC_PUSH_BACK() execution, surprisingly for non-debug build only, preventing next vector allocation... Signed-off-by: Bruno Faccini --- src/libpmemobj/ulog.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libpmemobj/ulog.c b/src/libpmemobj/ulog.c index 8975ff4fcbe..3fe0cb07ea5 100644 --- a/src/libpmemobj/ulog.c +++ b/src/libpmemobj/ulog.c @@ -225,7 +225,9 @@ ulog_rebuild_next_vec(struct ulog *ulog, struct ulog_next *next, { do { if (ulog->next != 0) - ASSERT(VEC_PUSH_BACK(next, ulog->next) == 0); + if (VEC_PUSH_BACK(next, ulog->next) == 0) + FATAL("%s: failed to allocate a next vector", + __func__); } while ((ulog = ulog_next(ulog, p_ops)) != NULL); } @@ -257,7 +259,9 @@ ulog_reserve(struct ulog *ulog, while (capacity < *new_capacity) { if (extend(p_ops->base, &ulog->next, gen_num) != 0) return -1; - ASSERT(VEC_PUSH_BACK(next, ulog->next) == 0); + if (VEC_PUSH_BACK(next, ulog->next) == 0) + FATAL("%s: failed to allocate a next vector", + __func__); ulog = ulog_next(ulog, p_ops); ASSERTne(ulog, NULL); From 1eb27e79767118ad7b44626a47f717988b9bb34b Mon Sep 17 00:00:00 2001 From: Bruno Faccini Date: Thu, 6 Apr 2023 16:24:09 +0200 Subject: [PATCH 3/4] obj: comply to tab vs space rule for pmdk... Signed-off-by: Bruno Faccini --- src/libpmemobj/ulog.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libpmemobj/ulog.c b/src/libpmemobj/ulog.c index 3fe0cb07ea5..09fbc63d478 100644 --- a/src/libpmemobj/ulog.c +++ b/src/libpmemobj/ulog.c @@ -227,7 +227,7 @@ ulog_rebuild_next_vec(struct ulog *ulog, struct ulog_next *next, if (ulog->next != 0) if (VEC_PUSH_BACK(next, ulog->next) == 0) FATAL("%s: failed to allocate a next vector", - __func__); + __func__); } while ((ulog = ulog_next(ulog, p_ops)) != NULL); } @@ -261,7 +261,7 @@ ulog_reserve(struct ulog *ulog, return -1; if (VEC_PUSH_BACK(next, ulog->next) == 0) FATAL("%s: failed to allocate a next vector", - __func__); + __func__); ulog = ulog_next(ulog, p_ops); ASSERTne(ulog, NULL); From e717f1a2f06d4360660a398b908309d485953747 Mon Sep 17 00:00:00 2001 From: Bruno Faccini Date: Thu, 13 Apr 2023 09:49:32 +0200 Subject: [PATCH 4/4] obj: oops wrong/reverse test testing return value from VEC_PUSH_BACK() equal to 0 for error is not good !!... Ref: pmem/issues#5515 Signed-off-by: Bruno Faccini --- src/libpmemobj/tx.c | 2 +- src/libpmemobj/ulog.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libpmemobj/tx.c b/src/libpmemobj/tx.c index 3f2e7616869..a2ad1f25de9 100644 --- a/src/libpmemobj/tx.c +++ b/src/libpmemobj/tx.c @@ -1018,7 +1018,7 @@ pmemobj_tx_commit(void) struct operation_context *ctx = tx->lane->external; VEC_FOREACH_BY_PTR(userbuf, &tx->redo_userbufs) if (operation_add_user_buffer(ctx, userbuf) == -1) - FATAL("%s: failed to allocate the next vector", + FATAL("%s: failed to allocate a next vector", __func__); palloc_publish(&pop->heap, VEC_ARR(&tx->actions), diff --git a/src/libpmemobj/ulog.c b/src/libpmemobj/ulog.c index 09fbc63d478..c0fae006c8a 100644 --- a/src/libpmemobj/ulog.c +++ b/src/libpmemobj/ulog.c @@ -225,7 +225,7 @@ ulog_rebuild_next_vec(struct ulog *ulog, struct ulog_next *next, { do { if (ulog->next != 0) - if (VEC_PUSH_BACK(next, ulog->next) == 0) + if (VEC_PUSH_BACK(next, ulog->next) != 0) FATAL("%s: failed to allocate a next vector", __func__); } while ((ulog = ulog_next(ulog, p_ops)) != NULL); @@ -259,7 +259,7 @@ ulog_reserve(struct ulog *ulog, while (capacity < *new_capacity) { if (extend(p_ops->base, &ulog->next, gen_num) != 0) return -1; - if (VEC_PUSH_BACK(next, ulog->next) == 0) + if (VEC_PUSH_BACK(next, ulog->next) != 0) FATAL("%s: failed to allocate a next vector", __func__); ulog = ulog_next(ulog, p_ops);