diff --git a/src/scheduler/src/resource.c b/src/scheduler/src/resource.c index a87ae778..eb9ddbdf 100644 --- a/src/scheduler/src/resource.c +++ b/src/scheduler/src/resource.c @@ -127,8 +127,25 @@ int put_resvals(zarray_t* array, int nres, int* idxs, float* vals) return nres; } +/* uncomit_at_ - the uncommitted amount of resource index `idx`. + * + * bug-1892: the context's uncommitted vectors are dense, keyed by resource + * index (schedobj.c fills them with xvec_idval()), unlike the positional + * total/inuse/wanted arrays unsatisfy_() compares them with. An index past + * the vector's end (sim_loop.c passes a short all-zero one) or a NULL + * vector means nothing is uncommitted. */ +static float uncomit_at_(const xvec_t* uncomitted, int idx) +{ + if ((uncomitted == NULL) || (uncomitted->data == NULL) || (idx < 0) + || (idx >= uncomitted->count)) { + return 0.0f; + } + + return ((const float*)uncomitted->data)[idx]; +} + static int unsatisfy_(xce_sched_core_t* core, int nres, int* idxs, float* total, - float* inuse, float* uncomitted, float* wanted, + float* inuse, const xvec_t* uncomitted, float* wanted, int* unmeet_idxs, float* unmeet_vals) { int i, j; @@ -136,10 +153,11 @@ static int unsatisfy_(xce_sched_core_t* core, int nres, int* idxs, float* total, count = 0; for (i = 0; i < nres; i++) { - if (total[i] < inuse[i] + uncomitted[i] + wanted[i]) { + float uncomit = uncomit_at_(uncomitted, idxs[i]); + + if (total[i] < inuse[i] + uncomit + wanted[i]) { TRACE("%s resource %d total %f inuse %f uncomit %f wanted %f", - __func__, idxs[i], total[i], inuse[i], uncomitted[i], - wanted[i]); + __func__, idxs[i], total[i], inuse[i], uncomit, wanted[i]); count++; } @@ -151,9 +169,11 @@ static int unsatisfy_(xce_sched_core_t* core, int nres, int* idxs, float* total, j = 0; for (i = 0; i < nres; i++) { - if (total[i] < inuse[i] + uncomitted[i] + wanted[i]) { + float uncomit = uncomit_at_(uncomitted, idxs[i]); + + if (total[i] < inuse[i] + uncomit + wanted[i]) { xce_resource_t* r = NULL; - float need = inuse[i] + uncomitted[i] + wanted[i] - total[i]; + float need = inuse[i] + uncomit + wanted[i] - total[i]; vget(core->resmgr, idxs[i], (void**)&r); TRACE("%s unmet resource %d (%s) shortfall %f", __func__, idxs[i], @@ -299,7 +319,7 @@ static int sched_check_shared_satisfy_(xce_sched_core_t* core, xvec_t* total, } rc = unsatisfy_(core, nres, (int*)wanted->idxs.data, (float*)total->data, - (float*)inuse->data, (float*)uncomitted->data, + (float*)inuse->data, uncomitted, (float*)wanted->vals.data, (int*)unmeet->idxs.data, (float*)unmeet->vals.data); if (rc < 0) { @@ -374,7 +394,7 @@ static int sched_check_host_satisfy_(xce_sched_core_t* core, } rc = unsatisfy_(core, nres, (int*)wanted->idxs.data, (float*)total->data, - (float*)inuse->data, (float*)uncomitted->data, + (float*)inuse->data, uncomitted, (float*)wanted->vals.data, (int*)unmeet->idxs.data, (float*)unmeet->vals.data); if (rc < 0) { diff --git a/src/scheduler/testsuite/test_resource_accounting.c b/src/scheduler/testsuite/test_resource_accounting.c index 98c95083..9c62480d 100644 --- a/src/scheduler/testsuite/test_resource_accounting.c +++ b/src/scheduler/testsuite/test_resource_accounting.c @@ -1285,6 +1285,153 @@ test_fill_limit_requests_host_limit_on_global_resource_reads_shared(void) xlist_fini(&limits); } +/* + * bug-1892: sched_check_host_satisfy()/sched_check_shared_satisfy() get the + * context's uncommitted vector dense, keyed by resource index (1024 floats, + * filled by xvec_idval(), schedobj.c), while total/inuse/wanted are + * positional (entry i is resource wanted->idxs[i]). unsatisfy_() used to + * read uncomitted[i] instead of uncomitted[wanted->idxs[i]]. + * + * Fixture: resource idx 5 has total 10, nothing in use; the request wants + * 4 of it. + */ +#define B1892_RESIDX 5 + +static void b1892_dense_uncomit(xvec_t* v, int residx, float val) +{ + TEST_ASSERT_EQUAL_INT(1024, xvec_init(v, 1024, sizeof(float))); + ((float*)v->data)[residx] = val; +} + +static void b1892_want(xvec_idval_t* wanted, float val) +{ + TEST_ASSERT_TRUE(xvec_idval_init(wanted, 1) >= 0); + xvec_idval_store(wanted, B1892_RESIDX, val); +} + +static xce_sched_worker_t* b1892_host(xce_sched_core_t* core) +{ + xce_sched_worker_t* w; + int idx[1] = { B1892_RESIDX }; + float val[1] = { 10.0f }; + + setup_core_with_worker(core, "hostA"); + w = sched_get_worker(core, "hostA"); + TEST_ASSERT_NOT_NULL(w); + TEST_ASSERT_TRUE(put_resvals(&(w->resvals), 1, idx, val) >= 0); + + return w; +} + +/* An earlier primary's uncommitted claim of 8 on the same resource leaves + * 2: the request for 4 must not fit (it used to read uncomitted[0] = 0). */ +static void test_host_satisfy_counts_uncommitted_of_the_wanted_resource(void) +{ + xce_sched_core_t core; + xce_sched_worker_t* w; + xvec_t uncomit; + xvec_idval_t wanted; + xvec_idval_t unmeet; + int rc; + + w = b1892_host(&core); + b1892_dense_uncomit(&uncomit, B1892_RESIDX, 8.0f); + b1892_want(&wanted, 4.0f); + memset(&unmeet, 0, sizeof(unmeet)); + + rc = sched_check_host_satisfy(&core, w, &uncomit, &wanted, &unmeet); + + TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc, "8 uncommitted + 4 wanted > 10"); + TEST_ASSERT_EQUAL_INT(1, unmeet.count); + TEST_ASSERT_EQUAL_INT(B1892_RESIDX, ((int*)unmeet.idxs.data)[0]); + TEST_ASSERT_EQUAL_FLOAT(2.0f, ((float*)unmeet.vals.data)[0]); + + xvec_idval_fini(&unmeet); + xvec_idval_fini(&wanted); + xvec_fini(&uncomit); +} + +/* An uncommitted claim on a different resource (idx 0) must not be charged + * against resource 5: the request fits. */ +static void test_host_satisfy_ignores_uncommitted_of_another_resource(void) +{ + xce_sched_core_t core; + xce_sched_worker_t* w; + xvec_t uncomit; + xvec_idval_t wanted; + xvec_idval_t unmeet; + int rc; + + w = b1892_host(&core); + b1892_dense_uncomit(&uncomit, 0, 8.0f); + b1892_want(&wanted, 4.0f); + memset(&unmeet, 0, sizeof(unmeet)); + + rc = sched_check_host_satisfy(&core, w, &uncomit, &wanted, &unmeet); + + TEST_ASSERT_EQUAL_INT_MESSAGE(1, rc, + "resource 0's uncommitted amount is not " + "resource 5's"); + + xvec_idval_fini(&unmeet); + xvec_idval_fini(&wanted); + xvec_fini(&uncomit); +} + +/* Same as the host case, against the shared pool. */ +static void test_shared_satisfy_counts_uncommitted_of_the_wanted_resource(void) +{ + xce_sched_core_t core; + xvec_t uncomit; + xvec_idval_t wanted; + xvec_idval_t unmeet; + int idx[1] = { B1892_RESIDX }; + float val[1] = { 10.0f }; + int rc; + + TEST_ASSERT_EQUAL_INT(0, xce_sched_core_init(&core)); + TEST_ASSERT_TRUE(put_resvals(&(core.sharedInst), 1, idx, val) >= 0); + b1892_dense_uncomit(&uncomit, B1892_RESIDX, 8.0f); + b1892_want(&wanted, 4.0f); + memset(&unmeet, 0, sizeof(unmeet)); + + rc = sched_check_shared_satisfy(&core, &uncomit, &wanted, &unmeet); + + TEST_ASSERT_EQUAL_INT_MESSAGE(0, rc, "8 uncommitted + 4 wanted > 10"); + TEST_ASSERT_EQUAL_INT(1, unmeet.count); + TEST_ASSERT_EQUAL_FLOAT(2.0f, ((float*)unmeet.vals.data)[0]); + + xvec_idval_fini(&unmeet); + xvec_idval_fini(&wanted); + xvec_fini(&uncomit); +} + +/* sim_loop.c's reconcile checks pass an all-zero uncommitted vector sized + * to the request (here 1 entry) -- shorter than the wanted resource index. + * An index past its end counts as 0, never an out-of-bounds read. */ +static void test_host_satisfy_short_uncommitted_vector_counts_as_zero(void) +{ + xce_sched_core_t core; + xce_sched_worker_t* w; + xvec_t uncomit; + xvec_idval_t wanted; + xvec_idval_t unmeet; + int rc; + + w = b1892_host(&core); + TEST_ASSERT_EQUAL_INT(1, xvec_init(&uncomit, 1, sizeof(float))); + b1892_want(&wanted, 4.0f); + memset(&unmeet, 0, sizeof(unmeet)); + + rc = sched_check_host_satisfy(&core, w, &uncomit, &wanted, &unmeet); + + TEST_ASSERT_EQUAL_INT(1, rc); + + xvec_idval_fini(&unmeet); + xvec_idval_fini(&wanted); + xvec_fini(&uncomit); +} + int main(void) { UNITY_BEGIN(); @@ -1310,5 +1457,9 @@ int main(void) RUN_TEST(test_fill_limit_requests_global_limit_on_slots_reads_host); RUN_TEST(test_fill_limit_requests_host_limit_on_host_resource_reads_host); RUN_TEST(test_fill_limit_requests_host_limit_on_global_resource_reads_shared); + RUN_TEST(test_host_satisfy_counts_uncommitted_of_the_wanted_resource); + RUN_TEST(test_host_satisfy_ignores_uncommitted_of_another_resource); + RUN_TEST(test_shared_satisfy_counts_uncommitted_of_the_wanted_resource); + RUN_TEST(test_host_satisfy_short_uncommitted_vector_counts_as_zero); return UNITY_END(); }