Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAOS-6628 dtx: code style issues found by coverity #4518

Merged
merged 2 commits into from
Feb 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dtx/dtx_rpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ dtx_commit(struct ds_cont_child *cont, struct dtx_entry **dtes,
out:
D_CDEBUG(rc < 0 || rc1 < 0 || rc2 < 0, DLOG_ERR, DB_IO,
"Commit DTXs "DF_DTI", count %d: rc %d %d %d\n",
DP_DTI(&dti[0]), count, rc, rc1, rc2);
DP_DTI(&dtes[0]->dte_xid), count, rc, rc1, rc2);

D_FREE(dti);

Expand Down
6 changes: 5 additions & 1 deletion src/dtx/dtx_srv.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,11 @@ dtx_handler(crt_rpc_t *rpc)
/* Commit the DTX after replied the original refresh request to
* avoid further query the same DTX.
*/
dtx_commit(cont, pdte, j, true);
rc = dtx_commit(cont, pdte, j, true);
if (rc < 0)
D_WARN("Failed to commit DTX "DF_DTI", count %d: "
DF_RC"\n", DP_DTI(&dtes[0].dte_xid), j,
DP_RC(rc));

for (i = 0; i < j; i++)
D_FREE(pdte[i]->dte_mbs);
Expand Down
41 changes: 24 additions & 17 deletions src/object/srv_obj.c
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,9 @@ obj_set_reply_sizes(crt_rpc_t *rpc, daos_iod_t *iods, int iod_nr)
/* Re-entry case.*/
if (orwo->orw_iod_sizes.ca_count != 0) {
D_ASSERT(orwo->orw_iod_sizes.ca_count == iod_nr);
D_ASSERT(orwo->orw_iod_sizes.ca_arrays != NULL);

sizes = orwo->orw_iod_sizes.ca_arrays;
} else {
D_ALLOC_ARRAY(sizes, iod_nr);
if (sizes == NULL)
Expand Down Expand Up @@ -2872,7 +2875,7 @@ obj_local_punch(struct obj_punch_in *opi, crt_opcode_t opc,
D_GOTO(out, rc = -DER_NOSYS);
}

if (obj_dtx_need_refresh(dth, rc)) {
if (dth != NULL && obj_dtx_need_refresh(dth, rc)) {
rc = dtx_refresh(dth, ioc->ioc_coc);
if (rc == -DER_AGAIN)
goto again;
Expand Down Expand Up @@ -4006,8 +4009,8 @@ obj_obj_dtx_leader(struct dtx_leader_handle *dlh, void *arg, int idx,
static int
ds_obj_dtx_leader_prep_handle(struct daos_cpd_sub_head *dcsh,
struct daos_cpd_sub_req *dcsrs,
struct daos_shard_tgt *tgts, uint32_t tgt_cnt,
uint32_t req_cnt, uint32_t *flags)
struct daos_shard_tgt *tgts,
int tgt_cnt, int req_cnt, uint32_t *flags)
Nasf-Fan marked this conversation as resolved.
Show resolved Hide resolved
{
int rc = 0;
int i;
Expand Down Expand Up @@ -4046,20 +4049,20 @@ ds_obj_dtx_leader_prep_handle(struct daos_cpd_sub_head *dcsh,
static void
ds_obj_dtx_leader_ult(void *arg)
{
struct daos_cpd_args *dca = arg;
struct dtx_leader_handle dlh;
struct ds_obj_exec_arg exec_arg;
struct obj_cpd_in *oci = crt_req_get(dca->dca_rpc);
struct obj_cpd_out *oco = crt_reply_get(dca->dca_rpc);
struct daos_cpd_sub_head *dcsh;
struct daos_cpd_disp_ent *dcde;
struct daos_cpd_sub_req *dcsrs = NULL;
struct daos_shard_tgt *tgts;
uint32_t flags = 0;
uint32_t dtx_flags = DTX_DIST;
uint32_t tgt_cnt = 0;
uint32_t req_cnt = 0;
int rc = 0;
struct daos_cpd_args *dca = arg;
struct dtx_leader_handle dlh;
struct ds_obj_exec_arg exec_arg;
struct obj_cpd_in *oci = crt_req_get(dca->dca_rpc);
struct obj_cpd_out *oco = crt_reply_get(dca->dca_rpc);
struct daos_cpd_sub_head *dcsh;
struct daos_cpd_disp_ent *dcde;
struct daos_cpd_sub_req *dcsrs = NULL;
struct daos_shard_tgt *tgts;
uint32_t flags = 0;
uint32_t dtx_flags = DTX_DIST;
int tgt_cnt = 0;
int req_cnt = 0;
int rc = 0;

/* TODO: For the daos targets in the first redundancy (modification)
* group, they are the DTX leader candidates when DTX recovery.
Expand Down Expand Up @@ -4146,6 +4149,10 @@ ds_obj_dtx_leader_ult(void *arg)
req_cnt = ds_obj_cpd_get_dcsr_cnt(dca->dca_rpc, dca->dca_idx);
tgt_cnt = ds_obj_cpd_get_tgt_cnt(dca->dca_rpc, dca->dca_idx);

if (dcde == NULL || dcsrs == NULL || tgts == NULL ||
req_cnt < 0 || tgt_cnt < 0)
D_GOTO(out, rc = -DER_INVAL);

/* Refuse any modification with old epoch. */
if (dcde->dcde_write_cnt != 0 &&
dcsh->dcsh_epoch.oe_value < dss_get_start_epoch())
Expand Down
13 changes: 4 additions & 9 deletions src/tests/suite/daos_dist_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -2876,11 +2876,8 @@ dtx_sub_setup(void **state)
{
int rc;

if (state != NULL) {
saved_dtx_arg = *state;
*state = NULL;
}

saved_dtx_arg = *state;
*state = NULL;
rc = test_setup(state, SETUP_CONT_CONNECT, true, SMALL_POOL_SIZE,
0, NULL);
return rc;
Expand All @@ -2892,10 +2889,8 @@ dtx_sub_teardown(void **state)
int rc;

rc = test_teardown(state);
if (state != NULL && saved_dtx_arg != NULL) {
*state = saved_dtx_arg;
saved_dtx_arg = NULL;
}
*state = saved_dtx_arg;
saved_dtx_arg = NULL;

return rc;
}
Expand Down
18 changes: 10 additions & 8 deletions src/vos/tests/vts_mvcc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1482,20 +1482,22 @@ uncertainty_check_exec_one(struct io_test_args *arg, int i, int j, bool empty,

if (!daos_is_zero_dti(&wtx->th_saved_xid)) {
if (wtx->th_skip_commit)
vos_dtx_commit(arg->ctx.tc_co_hdl, &wtx->th_saved_xid,
1, NULL);
rc = vos_dtx_commit(arg->ctx.tc_co_hdl,
&wtx->th_saved_xid, 1, NULL);
else
vos_dtx_abort(arg->ctx.tc_co_hdl, DAOS_EPOCH_MAX,
&wtx->th_saved_xid, 1);
rc = vos_dtx_abort(arg->ctx.tc_co_hdl, DAOS_EPOCH_MAX,
&wtx->th_saved_xid, 1);
assert(rc >= 0 || rc == -DER_NONEXIST);
}

if (!daos_is_zero_dti(&atx->th_saved_xid)) {
if (atx->th_skip_commit)
vos_dtx_commit(arg->ctx.tc_co_hdl, &atx->th_saved_xid,
1, NULL);
rc = vos_dtx_commit(arg->ctx.tc_co_hdl,
&atx->th_saved_xid, 1, NULL);
else
vos_dtx_abort(arg->ctx.tc_co_hdl, DAOS_EPOCH_MAX,
&atx->th_saved_xid, 1);
rc = vos_dtx_abort(arg->ctx.tc_co_hdl, DAOS_EPOCH_MAX,
&atx->th_saved_xid, 1);
assert(rc >= 0 || rc == -DER_NONEXIST);
}

#undef DP_CASE
Expand Down
58 changes: 40 additions & 18 deletions src/vos/vos_dtx.c
Original file line number Diff line number Diff line change
Expand Up @@ -1768,7 +1768,7 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,
struct vos_dtx_blob_df *dbd;
struct vos_dtx_blob_df *dbd_prev;
umem_off_t dbd_off;
struct vos_dtx_cmt_ent_df *dce_df;
struct vos_dtx_cmt_ent_df *dce_df = NULL;
int committed = 0;
int slots = 0;
int cur = 0;
Expand All @@ -1777,6 +1777,7 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,
int i;
int j;
bool fatal = false;
bool allocated = false;

dbd = umem_off2ptr(umm, cont_df->cd_dtx_committed_tail);
if (dbd != NULL)
Expand All @@ -1787,7 +1788,7 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,

rc = umem_tx_add_ptr(umm, &dbd->dbd_count, sizeof(dbd->dbd_count));
if (rc != 0)
return rc;
D_GOTO(out, fatal = true);

again:
if (slots > count)
Expand All @@ -1802,10 +1803,12 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,
DP_DTI(&dtis[cur]));

/* non-fatal, former handled ones can be committed. */
return committed > 0 ? committed : -DER_NOMEM;
D_GOTO(out, rc1 = -DER_NOMEM);
}
allocated = true;
} else {
dce_df = &dbd->dbd_committed_data[dbd->dbd_count];
allocated = false;
}

for (i = 0, j = 0; i < slots && rc1 == 0; i++, cur++) {
Expand All @@ -1816,7 +1819,7 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,
daes != NULL ? &daes[cur] : NULL,
&fatal);
if (fatal)
return rc;
goto out;

if (rc == 0 && (daes == NULL || daes[cur] != NULL))
committed++;
Expand Down Expand Up @@ -1850,7 +1853,7 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,
dbd->dbd_count += j;

if (count == 0 || rc1 != 0)
return committed > 0 ? committed : rc1;
goto out;

if (j < slots) {
slots -= j;
Expand All @@ -1865,7 +1868,8 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,
if (umoff_is_null(dbd_off)) {
D_ERROR("No space to store committed DTX %d "DF_DTI"\n",
count, DP_DTI(&dtis[cur]));
return -DER_NOSPACE;
fatal = true;
D_GOTO(out, rc = -DER_NOSPACE);
}

dbd = umem_off2ptr(umm, dbd_off);
Expand All @@ -1883,10 +1887,12 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,
if (dce_df == NULL) {
D_ERROR("Not enough DRAM to commit "DF_DTI"\n",
DP_DTI(&dtis[cur]));
return committed > 0 ? committed : -DER_NOMEM;
D_GOTO(out, rc1 = -DER_NOMEM);
}
allocated = true;
} else {
dce_df = &dbd->dbd_committed_data[0];
allocated = false;
}

if (dbd_prev == NULL) {
Expand All @@ -1898,21 +1904,21 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,
sizeof(cont_df->cd_dtx_committed_head) +
sizeof(cont_df->cd_dtx_committed_tail));
if (rc != 0)
return rc;
D_GOTO(out, fatal = true);

cont_df->cd_dtx_committed_head = dbd_off;
} else {
rc = umem_tx_add_ptr(umm, &dbd_prev->dbd_next,
sizeof(dbd_prev->dbd_next));
if (rc != 0)
return rc;
D_GOTO(out, fatal = true);

dbd_prev->dbd_next = dbd_off;

rc = umem_tx_add_ptr(umm, &cont_df->cd_dtx_committed_tail,
sizeof(cont_df->cd_dtx_committed_tail));
if (rc != 0)
return rc;
D_GOTO(out, fatal = true);
}

cont_df->cd_dtx_committed_tail = dbd_off;
Expand All @@ -1925,7 +1931,7 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,
daes != NULL ? &daes[cur] : NULL,
&fatal);
if (fatal)
return rc;
goto out;

if (rc == 0 && (daes == NULL || daes[cur] != NULL))
committed++;
Expand All @@ -1951,7 +1957,11 @@ vos_dtx_commit_internal(struct vos_container *cont, struct dtx_id *dtis,

dbd->dbd_count = j;

return committed > 0 ? committed : rc1;
out:
if (allocated)
D_FREE(dce_df);

return fatal ? rc : (committed > 0 ? committed : rc1);
}

void
Expand Down Expand Up @@ -2110,8 +2120,11 @@ vos_dtx_aggregate(daos_handle_t coh)
dce = d_list_entry(cont->vc_dtx_committed_list.next,
struct vos_dtx_cmt_ent, dce_committed_link);
d_iov_set(&kiov, &DCE_XID(dce), sizeof(DCE_XID(dce)));
dbtree_delete(cont->vc_dtx_committed_hdl, BTR_PROBE_EQ,
&kiov, NULL);
rc = dbtree_delete(cont->vc_dtx_committed_hdl, BTR_PROBE_EQ,
&kiov, NULL);
if (rc != 0)
D_WARN("Failed to remove cmt DTX entry: "DF_RC"\n",
DP_RC(rc));
}

tmp = umem_off2ptr(umm, dbd->dbd_next);
Expand Down Expand Up @@ -2565,10 +2578,19 @@ vos_dtx_cache_reset(daos_handle_t coh)
cont = vos_hdl2cont(coh);
D_ASSERT(cont != NULL);

if (daos_handle_is_valid(cont->vc_dtx_active_hdl))
dbtree_destroy(cont->vc_dtx_active_hdl, NULL);
if (daos_handle_is_valid(cont->vc_dtx_committed_hdl))
dbtree_destroy(cont->vc_dtx_committed_hdl, NULL);
if (daos_handle_is_valid(cont->vc_dtx_active_hdl)) {
rc = dbtree_destroy(cont->vc_dtx_active_hdl, NULL);
if (rc != 0)
D_WARN("Failed to destroy act DTX tree: "DF_RC"\n",
DP_RC(rc));
}

if (daos_handle_is_valid(cont->vc_dtx_committed_hdl)) {
rc = dbtree_destroy(cont->vc_dtx_committed_hdl, NULL);
if (rc != 0)
D_WARN("Failed to destroy cmt DTX tree: "DF_RC"\n",
DP_RC(rc));
}

if (cont->vc_dtx_array)
lrua_array_free(cont->vc_dtx_array);
Expand Down