From da3b90a1bfc12e39104286ed631be9ecadecbcdc Mon Sep 17 00:00:00 2001 From: Fan Yong Date: Sat, 21 Dec 2024 20:33:28 +0800 Subject: [PATCH] DAOS-16876 vos: set cont parameter when deregister modification from DTX As long as the container is not destroyed, then anytime want to deregister a modification from related active DTX entry (that is usually triggered for vos discard or aggregation), the caller needs to offer container handle to vos_dtx_deregister_record() for locating the DTX entry in active DTX table. Otherwise, if the caller offers empty container handle, then it will cause dangling reference in related DTX entry as to data corruption in subsequent DTX commit or abort. On the other hand, if the container will be destroyed, then all related DTX entries for such container will be useless any more. We need to destroy DTX table firstly to avoid generating dangling DTX references during destroying the container. Signed-off-by: Fan Yong --- src/vos/tests/vts_aggregate.c | 12 +++--------- src/vos/vos_dtx.c | 18 ++++++++---------- src/vos/vos_gc.c | 15 ++++++++++----- src/vos/vos_obj.c | 2 +- src/vos/vos_obj_index.c | 5 ++++- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/vos/tests/vts_aggregate.c b/src/vos/tests/vts_aggregate.c index aa183a41b52..2b24580af0e 100644 --- a/src/vos/tests/vts_aggregate.c +++ b/src/vos/tests/vts_aggregate.c @@ -22,10 +22,9 @@ static bool slow_test; -static void +static inline void cleanup(void) { - daos_fail_loc_set(DAOS_VOS_GC_CONT_NULL | DAOS_FAIL_ALWAYS); gc_wait(); } @@ -1314,24 +1313,19 @@ agg_punches_test(void **state, int record_type, bool discard) } } } - /** cleanup() sets the flag to assert if there are items in container garbage collection - * heap which will always be the case for these punch tests. So let's run garbage - * collection before cleanup in this case. - */ - gc_wait(); + + cleanup(); } static void discard_14(void **state) { agg_punches_test(state, DAOS_IOD_SINGLE, true); - cleanup(); } static void discard_15(void **state) { agg_punches_test(state, DAOS_IOD_ARRAY, true); - cleanup(); } static void diff --git a/src/vos/vos_dtx.c b/src/vos/vos_dtx.c index c378e9db7f4..88cba8906f4 100644 --- a/src/vos/vos_dtx.c +++ b/src/vos/vos_dtx.c @@ -1584,7 +1584,6 @@ vos_dtx_deregister_record(struct umem_instance *umm, daos_handle_t coh, uint32_t entry, daos_epoch_t epoch, umem_off_t record) { struct dtx_handle *dth = vos_dth_get(false); - struct vos_container *cont; struct vos_dtx_act_ent *dae; struct vos_dtx_act_ent_df *dae_df; umem_off_t *rec_df; @@ -1593,20 +1592,19 @@ vos_dtx_deregister_record(struct umem_instance *umm, daos_handle_t coh, int rc; int i; + /* + * If @coh is empty handle, then we are destroying the container. Under such case, + * both the in-DRAM and on-dish DTX entry have already been released or destroyed. + */ + if (daos_handle_is_inval(coh)) + return 0; + if (!vos_dtx_is_normal_entry(entry)) return 0; D_ASSERT(entry >= DTX_LID_RESERVED); - cont = vos_hdl2cont(coh); - /* If "cont" is NULL, then we are destroying the container. - * Under such case, the DTX entry in DRAM has been removed, - * The on-disk entry will be destroyed soon. - */ - if (cont == NULL) - return 0; - - found = lrua_lookupx(cont->vc_dtx_array, entry - DTX_LID_RESERVED, + found = lrua_lookupx(vos_hdl2cont(coh)->vc_dtx_array, entry - DTX_LID_RESERVED, epoch, &dae); if (!found) { D_WARN("Could not find active DTX record for lid=%d, epoch=" diff --git a/src/vos/vos_gc.c b/src/vos/vos_gc.c index 835b32093b0..fdb8e008b71 100644 --- a/src/vos/vos_gc.c +++ b/src/vos/vos_gc.c @@ -296,6 +296,15 @@ gc_drain_cont(struct vos_gc *gc, struct vos_pool *pool, daos_handle_t coh, int i; int rc; + /* + * When we prepaer to drain the container, we do not need DTX entry any long. + * Then destroy DTX table firstly to avoid dangling DXT records during drain + * the container (that may yield). + */ + rc = vos_dtx_table_destroy(&pool->vp_umm, cont); + if (rc != 0) + return rc; + /** Move any leftover bags to the pool gc */ for (i = GC_AKEY; i < GC_CONT; i++) { src_bin = &cont->cd_gc_bins[i]; @@ -331,11 +340,7 @@ gc_free_cont(struct vos_gc *gc, struct vos_pool *pool, daos_handle_t coh, struct } } - rc = vos_dtx_table_destroy(&pool->vp_umm, cd); - if (rc == 0) - rc = umem_free(&pool->vp_umm, item->it_addr); - - return rc; + return umem_free(&pool->vp_umm, item->it_addr); } static struct vos_gc gc_table[] = { diff --git a/src/vos/vos_obj.c b/src/vos/vos_obj.c index 52274f09d48..e09db1a8944 100644 --- a/src/vos/vos_obj.c +++ b/src/vos/vos_obj.c @@ -2599,7 +2599,7 @@ vos_obj_iter_aggregate(daos_handle_t ih, bool range_discard) * be aborted. Then it will be added and handled via GC when ktr_rec_free(). */ - rc = dbtree_iter_delete(oiter->it_hdl, NULL); + rc = dbtree_iter_delete(oiter->it_hdl, obj->obj_cont); D_ASSERT(rc != -DER_NONEXIST); } else if (rc == -DER_NONEXIST) { /* Key no longer exists at epoch but isn't empty */ diff --git a/src/vos/vos_obj_index.c b/src/vos/vos_obj_index.c index 7799f26a0db..772c1bf3e86 100644 --- a/src/vos/vos_obj_index.c +++ b/src/vos/vos_obj_index.c @@ -895,6 +895,7 @@ oi_iter_aggregate(daos_handle_t ih, bool range_discard) struct vos_oi_iter *oiter = iter2oiter(iter); struct vos_container *cont = oiter->oit_cont; struct vos_obj_df *obj; + struct oi_delete_arg del_arg; daos_unit_oid_t oid; d_iov_t rec_iov; bool delete = false, invisible = false; @@ -945,7 +946,9 @@ oi_iter_aggregate(daos_handle_t ih, bool range_discard) if (rc != 0) D_ERROR("Could not evict object "DF_UOID" "DF_RC"\n", DP_UOID(oid), DP_RC(rc)); - rc = dbtree_iter_delete(oiter->oit_hdl, NULL); + del_arg.cont = oiter->oit_cont; + del_arg.only_delete_entry = 0; + rc = dbtree_iter_delete(oiter->oit_hdl, &del_arg); D_ASSERT(rc != -DER_NONEXIST); } else if (rc == -DER_NONEXIST) { /** ilog isn't visible in range but still has some entries */