DAOS-18376 container: Fix inconsistent ds_cont_child state (#17314)

Fix inconsistent ds_cont_child state when ds_cont_csummer_init() yields
in md-on-ssd mode:

In md-on-ssd mode, ds_cont_csummer_init() persists properties to VOS and calls
umem_tx_commit(). Because umem_tx_commit() may yield, ds_cont_child can be left
in an inconsistent state:
- sc_props_fetched remains false
- sc_csummer is already set

A concurrent ULT may observe this state and hit an assertion.

Signed-off-by: Liang Zhen <[email protected]>

Author: gnailzenh

src/container/srv_target.c

@@ -132,23 +132,25 @@ ds_cont_csummer_init(struct ds_cont_child *cont)
 	bool			dedup_only = false;
 
 	D_ASSERT(cont != NULL);
-	cont_props = &cont->sc_props;
+	while (cont->sc_csummer_initing) {
+		ABT_mutex_lock(cont->sc_mutex);
+		ABT_cond_wait(cont->sc_init_cond, cont->sc_mutex);
+		ABT_mutex_unlock(cont->sc_mutex);
+	}
 
-	if (cont->sc_props_fetched)
+	if (cont->sc_csummer_inited)
 		return 0;
 
+	D_ASSERT(cont->sc_csummer == NULL);
+	cont->sc_csummer_initing = 1;
 	/** Get the container csum related properties
 	 * Need the pool for the IV namespace
 	 */
-	D_ASSERT(cont->sc_csummer == NULL);
+	cont_props = &cont->sc_props;
 	rc = ds_cont_get_props(cont_props, cont->sc_pool_uuid, cont->sc_uuid);
 	if (rc != 0)
 		goto done;
 
-	/* Check again since IV fetch yield */
-	if (cont->sc_props_fetched)
-		goto done;
-
 	csum_val = cont_props->dcp_csum_type;
 	if (!daos_cont_csum_prop_is_enabled(csum_val)) {
 		dedup_only = true;
@@ -178,9 +180,13 @@ ds_cont_csummer_init(struct ds_cont_child *cont)
 		       DP_UUID(cont->sc_uuid), DP_RC(rc));
 		rc = 0;
 	}
-	cont->sc_props_fetched = 1;
-
+	D_ASSERT(!cont->sc_csummer_inited); /* nobody else can do this except me */
+	cont->sc_csummer_inited = 1;
 done:
+	if (cont->sc_csummer_initing) {
+		cont->sc_csummer_initing = 0;
+		ABT_cond_broadcast(cont->sc_init_cond);
+	}
 	return rc;
 }
 
@@ -218,7 +224,7 @@ cont_aggregate_runnable(struct ds_cont_child *cont, struct sched_request *req,
 				DP_CONT(cont->sc_pool->spc_uuid, cont->sc_uuid));
 	}
 
-	if (!cont->sc_props_fetched)
+	if (!cont->sc_csummer_inited)
 		ds_cont_csummer_init(cont);
 
 	if (cont->sc_props.dcp_dedup_enabled ||
@@ -655,6 +661,25 @@ cont_child_obj(struct daos_llink *llink)
 	return container_of(llink, struct ds_cont_child, sc_list);
 }
 
+static void
+cont_child_fini_abt(struct ds_cont_child *cont)
+{
+	if (cont->sc_dtx_resync_cond)
+		ABT_cond_free(&cont->sc_dtx_resync_cond);
+	if (cont->sc_scrub_cond)
+		ABT_cond_free(&cont->sc_scrub_cond);
+	if (cont->sc_rebuild_cond)
+		ABT_cond_free(&cont->sc_rebuild_cond);
+	if (cont->sc_init_cond)
+		ABT_cond_free(&cont->sc_init_cond);
+	if (cont->sc_fini_cond)
+		ABT_cond_free(&cont->sc_fini_cond);
+	if (cont->sc_mutex)
+		ABT_mutex_free(&cont->sc_mutex);
+	if (cont->sc_open_mutex)
+		ABT_mutex_free(&cont->sc_open_mutex);
+}
+
 static int
 cont_child_alloc_ref(void *co_uuid, unsigned int ksize, void *po_uuid,
 		     struct daos_llink **link)
@@ -678,34 +703,39 @@ cont_child_alloc_ref(void *co_uuid, unsigned int ksize, void *po_uuid,
 	rc = ABT_mutex_create(&cont->sc_mutex);
 	if (rc != ABT_SUCCESS) {
 		rc = dss_abterr2der(rc);
-		goto out_open_mutex;
+		goto out_abt;
 	}
 
 	rc = ABT_cond_create(&cont->sc_dtx_resync_cond);
 	if (rc != ABT_SUCCESS) {
 		rc = dss_abterr2der(rc);
-		goto out_mutex;
+		goto out_abt;
 	}
 	rc = ABT_cond_create(&cont->sc_scrub_cond);
 	if (rc != ABT_SUCCESS) {
 		rc = dss_abterr2der(rc);
-		goto out_resync_cond;
+		goto out_abt;
 	}
 	rc = ABT_cond_create(&cont->sc_rebuild_cond);
 	if (rc != ABT_SUCCESS) {
 		rc = dss_abterr2der(rc);
-		goto out_scrub_cond;
+		goto out_abt;
+	}
+	rc = ABT_cond_create(&cont->sc_init_cond);
+	if (rc != ABT_SUCCESS) {
+		rc = dss_abterr2der(rc);
+		goto out_abt;
 	}
 	rc = ABT_cond_create(&cont->sc_fini_cond);
 	if (rc != ABT_SUCCESS) {
 		rc = dss_abterr2der(rc);
-		goto out_rebuild_cond;
+		goto out_abt;
 	}
 
 	cont->sc_pool = ds_pool_child_lookup(po_uuid);
 	if (cont->sc_pool == NULL) {
 		rc = -DER_NO_HDL;
-		goto out_finish_cond;
+		goto out_abt;
 	}
 
 	rc = vos_cont_open(cont->sc_pool->spc_hdl, co_uuid, &cont->sc_hdl);
@@ -745,18 +775,8 @@ cont_child_alloc_ref(void *co_uuid, unsigned int ksize, void *po_uuid,
 	vos_cont_close(cont->sc_hdl);
 out_pool:
 	ds_pool_child_put(cont->sc_pool);
-out_finish_cond:
-	ABT_cond_free(&cont->sc_fini_cond);
-out_rebuild_cond:
-	ABT_cond_free(&cont->sc_rebuild_cond);
-out_scrub_cond:
-	ABT_cond_free(&cont->sc_scrub_cond);
-out_resync_cond:
-	ABT_cond_free(&cont->sc_dtx_resync_cond);
-out_mutex:
-	ABT_mutex_free(&cont->sc_mutex);
-out_open_mutex:
-	ABT_mutex_free(&cont->sc_open_mutex);
+out_abt:
+	cont_child_fini_abt(cont);
 out:
 	D_FREE(cont);
 	return rc;
@@ -777,14 +797,10 @@ cont_child_free_ref(struct daos_llink *llink)
 	cont_tgt_track_eph_fini(cont);
 	vos_cont_close(cont->sc_hdl);
 	ds_pool_child_put(cont->sc_pool);
-	daos_csummer_destroy(&cont->sc_csummer);
+	if (cont->sc_csummer)
+		daos_csummer_destroy(&cont->sc_csummer);
 	D_FREE(cont->sc_snapshots);
-	ABT_cond_free(&cont->sc_dtx_resync_cond);
-	ABT_cond_free(&cont->sc_scrub_cond);
-	ABT_cond_free(&cont->sc_rebuild_cond);
-	ABT_cond_free(&cont->sc_fini_cond);
-	ABT_mutex_free(&cont->sc_mutex);
-	ABT_mutex_free(&cont->sc_open_mutex);
+	cont_child_fini_abt(cont);
 	D_FREE(cont);
 }
 

src/include/daos_srv/container.h

@@ -76,9 +76,11 @@ struct ds_cont_child {
 	ABT_cond		 sc_scrub_cond;
 	ABT_cond		 sc_rebuild_cond;
 	ABT_cond		 sc_fini_cond;
+	ABT_cond                 sc_init_cond;
 	uint32_t                 sc_dtx_resyncing : 1, sc_dtx_reindex : 1, sc_dtx_reindex_abort : 1,
-	    sc_dtx_delay_reset : 1, sc_dtx_registered : 1, sc_props_fetched : 1, sc_stopping : 1,
-	    sc_destroying : 1, sc_vos_agg_active : 1, sc_ec_agg_active : 1,
+	    sc_dtx_delay_reset : 1, sc_dtx_registered : 1, sc_csummer_inited : 1,
+	    sc_csummer_initing : 1, sc_stopping : 1, sc_destroying : 1, sc_vos_agg_active : 1,
+	    sc_ec_agg_active : 1,
 	    /* flag of CONT_CAPA_READ_DATA/_WRITE_DATA disabled */
 	    sc_rw_disabled : 1, sc_scrubbing : 1, sc_rebuilding : 1,
 	    /* flag of sc_ec_agg_eph_boundary valid */

src/include/daos_srv/vos.h

@@ -1590,7 +1590,7 @@ struct cont_scrub {
 	void			*scs_cont_src;
 	daos_handle_t		 scs_cont_hdl;
 	uuid_t			 scs_cont_uuid;
-	bool			 scs_props_fetched;
+	bool                     scs_csummer_inited;
 };
 
 /*

src/pool/srv_pool_scrub_ult.c

@@ -82,7 +82,7 @@ cont_lookup_cb(uuid_t pool_uuid, uuid_t cont_uuid, void *arg,
 	cont->scs_cont_hdl = cont_child->sc_hdl;
 	uuid_copy(cont->scs_cont_uuid, cont_uuid);
 	cont->scs_cont_src = cont_child;
-	cont->scs_props_fetched = cont_child->sc_props_fetched;
+	cont->scs_csummer_inited = cont_child->sc_csummer_inited;
 
 	ABT_mutex_lock(cont_child->sc_mutex);
 	cont_child->sc_scrubbing = 1;

src/vos/tests/pool_scrubbing_tests.c

@@ -1,5 +1,6 @@
 /**
  * (C) Copyright 2020-2022 Intel Corporation.
+ * (C) Copyright 2025 Hewlett Packard Enterprise Development LP
  *
  * SPDX-License-Identifier: BSD-2-Clause-Patent
  */
@@ -497,7 +498,7 @@ sts_ctx_setup_scrub_ctx(struct sts_context *ctx)
 	ctx->tsc_scrub_ctx.sc_drain_pool_tgt_fn = fake_target_drain;
 	ctx->tsc_scrub_ctx.sc_pool = &ctx->tsc_pool;
 	ctx->tsc_scrub_ctx.sc_dmi = &ctx->tsc_dmi;
-	ctx->tsc_scrub_ctx.sc_cont.scs_props_fetched = true;
+	ctx->tsc_scrub_ctx.sc_cont.scs_csummer_inited = true;
 }
 
 static void

src/vos/vos_pool_scrub.c

@@ -878,7 +878,7 @@ cont_iter_is_loaded_cb(daos_handle_t ih, vos_iter_entry_t *entry,
 	 * initialized if csums are enabled
 	 */
 	if (!args->args_found_unloaded_container)
-		args->args_found_unloaded_container = !args->args_ctx->sc_cont.scs_props_fetched;
+		args->args_found_unloaded_container = !args->args_ctx->sc_cont.scs_csummer_inited;
 
 	sc_cont_teardown(ctx);
 	return 0;