diff --git a/.kiro/specs/scheduler-sim-decision-trace/tasks.md b/.kiro/specs/scheduler-sim-decision-trace/tasks.md index 40ed4168..238bb221 100644 --- a/.kiro/specs/scheduler-sim-decision-trace/tasks.md +++ b/.kiro/specs/scheduler-sim-decision-trace/tasks.md @@ -67,7 +67,7 @@ - Done when the full scheduler test suite builds and passes under ASAN with the new includes and no hooks in place yet. - _Requirements: 11.4_ -- [ ] 1.7 Add the trace attachment and trace-only op to scheduling decisions +- [x] 1.7 Add the trace attachment and trace-only op to scheduling decisions - Decisions gain a trace attachment, which every existing constructor leaves empty. - A new trace-only op, with its own constructor, is appended after the existing ops without renumbering them. - The live driver's drain loop skips the trace-only op and frees any attached trace defensively. Live never produces either. @@ -344,3 +344,4 @@ - 1.5: the per-limit summary tally goes through `sched_trace_host_limit_hit(rec, limit, scope)` (added; not in the design's interface); 2.2 calls it for each unmet limit row in a host scope and keeps setting `host_row_verdicts |= SHORTFALL` directly. At summary level `pos_end` stores a POS_SUMMARY in place of POS_END, keeping the end state, error and host; 4.1 must render a scan error in that line. The search-end statement is written only when rc==0. - 1.5 OPEN for 3.4/4.1: a requirement the allocator declines (alloc_runner.c ~474) or whose plan generator fails to start (~427, returns 0) never reaches `sched_trace_search_end` when that call sits only at the last return, so `reqs_none_generated` misses it (Req 4.7, 9.3); calling search_end on that path as well double-reports "no plan generated". Settle it in 3.4, e.g. `sched_trace_no_plan` bumps the counter and search_end skips its statement when a no-plan entry is open. `test_search_end_nonzero_rc_has_no_statement` models a generator failure as rc -12, but the runner returns 0 there. - 1.6: 27 tests include a hooked source and now `#include` sched_trace.c and sched_trace_capture.c. test_snapshot_load_allocator_regression.c matches the grep only in a comment and links scheduler_srcs, which already has both. Still open for the hook tasks: `sched_sim_isolation_lib` (src/scheduler/meson.build ~165-173) compiles the hooked `sched_election.c` without the trace sources (1.8/3.1 must add them or a stub). The 19 repl/sim tests matched via repl.c/sim_loop.c belong to 5.1. +- 1.7 OPEN for 5.3: the sim decision-free sites at sim_loop.c ~1327 (reconstructed dispatch) and ~1709 (drain loop) don't yet call `sched_trace_rec_free(d->trace)`, and the TRACE op still falls to the sim's switch defaults. 5.3 must add the early TRACE branch before verify/record/breakpoint, or sim_verify reports false mismatches and breakpoints count it as "NONE". decision.h forward-declares `struct xce_sched_trace_rec_s` rather than including sched/trace.h. diff --git a/src/scheduler/include/decision.h b/src/scheduler/include/decision.h index c08d725f..1aa3fb5d 100644 --- a/src/scheduler/include/decision.h +++ b/src/scheduler/include/decision.h @@ -10,9 +10,17 @@ typedef enum { XCE_DECISION_DISPATCH, XCE_DECISION_PEND, XCE_DECISION_NEW_SESSION, - XCE_DECISION_PREEMPT + XCE_DECISION_PREEMPT, + /* scheduler-sim-decision-trace: a trace-only decision carrying just a + * trace record (a selected job that produced no decision). Appended + * after the existing ops; produced only by the sim REPL's tracing, + * never in live. */ + XCE_DECISION_TRACE } xce_decision_op_t; +/* Defined in sched/trace.h, not included here to keep this header light. */ +struct xce_sched_trace_rec_s; + /* * xce_preempt_victim_t / xce_preempt_t - PREEMPT decision payload * (scheduler-core spec, task 4.4, Req 5 AC 6). Scheduler-owned (not @@ -46,10 +54,21 @@ typedef struct { xce_pendreason_t* reason; xce_preempt_t* preempt; } u; + /* + * Trace record attached to this decision (scheduler-sim-decision-trace, + * design.md "DecisionTransport"). NULL from every constructor except + * make_trace_decision(), and never set in live. The drain loop that + * receives the decision owns it: the REPL takes it by setting this to + * NULL, and every receiver calls sched_trace_rec_free(d->trace) right + * before free(d). + */ + struct xce_sched_trace_rec_s* trace; } xce_decision_t; extern xce_decision_t* make_pend_decision(xce_pendreason_t*); extern xce_decision_t* make_dispatch_decision(xce_job_alloc_t*); extern xce_decision_t* make_preempt_decision(xce_preempt_t*); +/* Trace-only decision: op XCE_DECISION_TRACE, zeroed union, trace = rec. */ +extern xce_decision_t* make_trace_decision(struct xce_sched_trace_rec_s* rec); #endif diff --git a/src/scheduler/src/decision.c b/src/scheduler/src/decision.c index 2ebd7cfa..2ffe37da 100644 --- a/src/scheduler/src/decision.c +++ b/src/scheduler/src/decision.c @@ -46,3 +46,18 @@ xce_decision_t* make_preempt_decision(xce_preempt_t* preempt) return decision; } + +xce_decision_t* make_trace_decision(struct xce_sched_trace_rec_s* rec) +{ + xce_decision_t* decision; + + decision = (xce_decision_t*)calloc(1, sizeof(*decision)); + if (decision == NULL) { + return NULL; + } + + decision->op = XCE_DECISION_TRACE; + decision->trace = rec; + + return decision; +} diff --git a/src/scheduler/src/driver.c b/src/scheduler/src/driver.c index 6954842c..64cb72ed 100644 --- a/src/scheduler/src/driver.c +++ b/src/scheduler/src/driver.c @@ -9,6 +9,7 @@ #include "driver.h" #include "recording.h" #include "scheduler.h" +#include "sched/trace.h" #include "utils.h" /* @@ -1067,6 +1068,16 @@ void xce_sched_driver_run_normal_mode_round(void* driver_ctx) break; } + /* scheduler-sim-decision-trace (design.md "DecisionTransport"): + * trace-only decisions and attached traces come only from the sim + * REPL's tracing, never in live. Defensive: a trace-only decision + * is neither recorded nor published, just freed with its record. */ + if (decision->op == XCE_DECISION_TRACE) { + sched_trace_rec_free(decision->trace); + free(decision); + continue; + } + /* Req 5 AC 10: "In the decision drain loop ... append a * ReplayDecision to the current ReplaySession.decisions list" -- * captured BEFORE the publish callback below, since the decision's @@ -1089,7 +1100,9 @@ void xce_sched_driver_run_normal_mode_round(void* driver_ctx) /* bug-1885: the decision struct is the round's (make_*_decision()); * its allocation or pend reason belongs to the job, and the - * callback and the recorder copy what they keep */ + * callback and the recorder copy what they keep. An attached trace + * (never set in live) is the round's too. */ + sched_trace_rec_free(decision->trace); free(decision); } diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index 857b28f5..6dc666e7 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -2532,6 +2532,20 @@ test('scheduler trace model test', sched_trace_model_test, suite: 'scheduler', env: ['ASAN_OPTIONS=detect_leaks=0']) +# scheduler-sim-decision-trace task 1.7: the decision trace attachment and +# the trace-only op (design.md "DecisionTransport"). White-box #include of +# src/decision.c; links the record model for its constructor and free. +sched_decision_trace_test = executable( + 'sched_decision_trace_test', + ['test_decision_trace.c', '../src/sched_trace.c'], + include_directories:[incdir], + dependencies: [scheduler_test_deps], + install: false) + +test('scheduler decision trace test', + sched_decision_trace_test, suite: 'scheduler', + env: ['ASAN_OPTIONS=detect_leaks=0']) + # bug-1923: shared (sharedInst) and per-worker resource values at sparse # resource indices survive snapshot save and load, in memory and through # the real sched_record_start() -> .replay -> sched_sim_driver_start() seam diff --git a/src/scheduler/testsuite/test_decision_trace.c b/src/scheduler/testsuite/test_decision_trace.c new file mode 100644 index 00000000..b49581f0 --- /dev/null +++ b/src/scheduler/testsuite/test_decision_trace.c @@ -0,0 +1,130 @@ +/* + * test_decision_trace.c - the decision transport's trace attachment and + * trace-only op (scheduler-sim-decision-trace spec, task 1.7; design.md + * "DecisionTransport"; Requirements 2.4, 10.1, 11.4). + * + * - every existing constructor (pend, dispatch, preempt) leaves the trace + * attachment NULL; + * - the existing op values are unchanged and the trace-only op is + * appended after XCE_DECISION_PREEMPT; + * - make_trace_decision() builds a trace-only decision: op TRACE, zeroed + * union, the record attached. + * + * The live driver's handling of both is covered in + * test_driver_session_ordering.c. + * + * White-box #include of src/decision.c; links src/sched_trace.c for the + * record constructor and free. + */ +#include +#include + +#include + +#include "sched/trace.h" + +#include "../src/decision.c" + +void setUp(void) { return; } +void tearDown(void) { return; } + +static xce_sched_trace_ctl_t make_ctl(void) +{ + xce_sched_trace_ctl_t ctl; + + memset(&ctl, 0, sizeof(ctl)); + ctl.all = true; + ctl.level = XCE_TRACE_LEVEL_SUMMARY; + ctl.line_bound = XCE_TRACE_LINE_BOUND_DEFAULT; + ctl.session_no = 1; + ctl.policy_name = "fcfs"; + return ctl; +} + +static void test_existing_op_values_unchanged(void) +{ + TEST_ASSERT_EQUAL_INT(0, XCE_DECISION_NONE); + TEST_ASSERT_EQUAL_INT(1, XCE_DECISION_DISPATCH); + TEST_ASSERT_EQUAL_INT(2, XCE_DECISION_PEND); + TEST_ASSERT_EQUAL_INT(3, XCE_DECISION_NEW_SESSION); + TEST_ASSERT_EQUAL_INT(4, XCE_DECISION_PREEMPT); + TEST_ASSERT_EQUAL_INT(5, XCE_DECISION_TRACE); +} + +static void test_existing_constructors_leave_trace_null(void) +{ + xce_pendreason_t reason; + xce_job_alloc_t alloc; + xce_preempt_t preempt; + xce_decision_t* d; + + memset(&reason, 0, sizeof(reason)); + memset(&alloc, 0, sizeof(alloc)); + memset(&preempt, 0, sizeof(preempt)); + + d = make_pend_decision(&reason); + TEST_ASSERT_NOT_NULL(d); + TEST_ASSERT_EQUAL_INT(XCE_DECISION_PEND, d->op); + TEST_ASSERT_EQUAL_PTR(&reason, d->u.reason); + TEST_ASSERT_NULL(d->trace); + free(d); + + d = make_dispatch_decision(&alloc); + TEST_ASSERT_NOT_NULL(d); + TEST_ASSERT_EQUAL_INT(XCE_DECISION_DISPATCH, d->op); + TEST_ASSERT_EQUAL_PTR(&alloc, d->u.alloc); + TEST_ASSERT_NULL(d->trace); + free(d); + + d = make_preempt_decision(&preempt); + TEST_ASSERT_NOT_NULL(d); + TEST_ASSERT_EQUAL_INT(XCE_DECISION_PREEMPT, d->op); + TEST_ASSERT_EQUAL_PTR(&preempt, d->u.preempt); + TEST_ASSERT_NULL(d->trace); + free(d); +} + +static void test_make_trace_decision_attaches_record(void) +{ + xce_sched_trace_ctl_t ctl = make_ctl(); + xce_sched_trace_rec_t* rec; + xce_decision_t* d; + + rec = sched_trace_rec_new(&ctl, 42, XCE_TRACE_REC_DECISION); + TEST_ASSERT_NOT_NULL(rec); + sched_trace_rec_finish(rec, XCE_TRACE_REC_NO_SCHEDOBJ, 0); + + d = make_trace_decision(rec); + TEST_ASSERT_NOT_NULL(d); + TEST_ASSERT_EQUAL_INT(XCE_DECISION_TRACE, d->op); + TEST_ASSERT_NULL(d->u.alloc); + TEST_ASSERT_NULL(d->u.reason); + TEST_ASSERT_NULL(d->u.preempt); + TEST_ASSERT_EQUAL_PTR(rec, d->trace); + TEST_ASSERT_EQUAL_INT(XCE_TRACE_REC_NO_SCHEDOBJ, d->trace->kind); + TEST_ASSERT_EQUAL_UINT64(42, d->trace->jobid); + + /* the receiver owns d->trace: free it, then the decision */ + sched_trace_rec_free(d->trace); + free(d); +} + +static void test_make_trace_decision_accepts_null_record(void) +{ + xce_decision_t* d = make_trace_decision(NULL); + + TEST_ASSERT_NOT_NULL(d); + TEST_ASSERT_EQUAL_INT(XCE_DECISION_TRACE, d->op); + TEST_ASSERT_NULL(d->trace); + free(d); +} + +int main(void) +{ + UNITY_BEGIN(); + RUN_TEST(test_existing_op_values_unchanged); + RUN_TEST(test_existing_constructors_leave_trace_null); + RUN_TEST(test_make_trace_decision_attaches_record); + RUN_TEST(test_make_trace_decision_accepts_null_record); + return UNITY_END(); +} diff --git a/src/scheduler/testsuite/test_driver_session_ordering.c b/src/scheduler/testsuite/test_driver_session_ordering.c index 841bba4e..3edbddbb 100644 --- a/src/scheduler/testsuite/test_driver_session_ordering.c +++ b/src/scheduler/testsuite/test_driver_session_ordering.c @@ -684,6 +684,14 @@ static void* picker_thread_fn(void* arg) * (test-owned) reason before the end-of-session marker */ static xce_pendreason_t* g_emit_pend_reason; +/* scheduler-sim-decision-trace task 1.7: when set, the stand-in also emits + * a trace-only decision carrying g_emit_trace_only (recorded in + * g_emitted_trace_decision), and attaches g_emit_pend_trace to the PEND + * decision. Live never produces either; the driver frees both. */ +static xce_sched_trace_rec_t* g_emit_trace_only; +static xce_sched_trace_rec_t* g_emit_pend_trace; +static xce_decision_t* g_emitted_trace_decision; + static void* stand_in_allocator_thread_fn(void* arg) { xce_sched_core_t* core = (xce_sched_core_t*)arg; @@ -694,9 +702,19 @@ static void* stand_in_allocator_thread_fn(void* arg) (void)jobid; /* always 0 here -- this fixture pushes no jobs */ log_event(EVT_ALLOCATOR_DRAINED_JBUF_SENTINEL); + /* scheduler-sim-decision-trace task 1.7: a trace-only decision ahead + * of anything else, as the sim allocator would push it */ + if (g_emit_trace_only != NULL) { + xce_decision_t* tr = make_trace_decision(g_emit_trace_only); + + g_emitted_trace_decision = tr; + cbuf_put(&(core->decisions), &tr); + } + if (g_emit_pend_reason != NULL) { xce_decision_t* pend = make_pend_decision(g_emit_pend_reason); + pend->trace = g_emit_pend_trace; cbuf_put(&(core->decisions), &pend); } @@ -1020,6 +1038,96 @@ static void test_round_frees_decision_after_callback(void) #endif } +/* + * scheduler-sim-decision-trace task 1.7 (design.md "DecisionTransport"): + * the live round loop skips a trace-only decision without publishing or + * recording it, and frees it and its record; a trace attached to a + * published decision is freed with the decision. Neither occurs in live -- + * this is the defensive path. Checked through ASAN's shadow memory. + */ +static void test_round_skips_trace_only_and_frees_traces(void) +{ +#if defined(__SANITIZE_ADDRESS__) + xce_scheduler_t sched; + pthread_t picker_tid; + pthread_t allocator_tid; + test_driver_ctx_t driver_ctx; + xce_pendreason_t* reason; + xce_sched_trace_ctl_t ctl; + xce_sched_trace_rec_t* trace_only; + xce_sched_trace_rec_t* pend_trace; + int rc; + + memset(&g_events, 0, sizeof(g_events)); + memset(&sched, 0, sizeof(sched)); + memset(&driver_ctx, 0, sizeof(driver_ctx)); + memset(&ctl, 0, sizeof(ctl)); + ctl.all = true; + ctl.line_bound = XCE_TRACE_LINE_BOUND_DEFAULT; + + rc = xce_sched_core_init(&(sched.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + lock(sched.core.plugins, RTTI_LOCK_WRITE); + rc = key_add(sched.core.plugins, (void*)g_stub_picker_plugin.name, + (void*)&g_stub_picker_plugin); + TEST_ASSERT_GREATER_OR_EQUAL(0, rc); + unlock(sched.core.plugins); + sched.core.job_pick_plugin = g_stub_picker_plugin.name; + + reason = make_pendreason(4243); + TEST_ASSERT_NOT_NULL(reason); + trace_only = sched_trace_rec_new(&ctl, 4244, XCE_TRACE_REC_DECISION); + TEST_ASSERT_NOT_NULL(trace_only); + sched_trace_rec_finish(trace_only, XCE_TRACE_REC_NOT_FOUND, 0); + pend_trace = sched_trace_rec_new(&ctl, 4243, XCE_TRACE_REC_DECISION); + TEST_ASSERT_NOT_NULL(pend_trace); + + g_emit_pend_reason = reason; + g_emit_trace_only = trace_only; + g_emit_pend_trace = pend_trace; + g_emitted_trace_decision = NULL; + + rc = xce_core_set_decision_cb(&(sched.core), test_on_decision, &driver_ctx); + TEST_ASSERT_EQUAL_INT(0, rc); + + rc = pthread_create(&picker_tid, NULL, picker_thread_fn, &(sched.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + rc = pthread_create(&allocator_tid, NULL, stand_in_allocator_thread_fn, + &(sched.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + xce_sched_driver_run_normal_mode_round(&sched); + + TEST_ASSERT_EQUAL_INT(0, pthread_join(picker_tid, NULL)); + TEST_ASSERT_EQUAL_INT(0, pthread_join(allocator_tid, NULL)); + g_emit_pend_reason = NULL; + g_emit_trace_only = NULL; + g_emit_pend_trace = NULL; + + /* only the PEND reached the publish callback */ + TEST_ASSERT_EQUAL_INT(1, driver_ctx.calls); + TEST_ASSERT_NOT_NULL(driver_ctx.last); + TEST_ASSERT_NOT_NULL(g_emitted_trace_decision); + TEST_ASSERT_TRUE_MESSAGE( + __asan_address_is_poisoned(g_emitted_trace_decision), + "the round must free the trace-only decision"); + TEST_ASSERT_TRUE_MESSAGE(__asan_address_is_poisoned(trace_only), + "the round must free the trace-only record"); + TEST_ASSERT_TRUE_MESSAGE(__asan_address_is_poisoned(pend_trace), + "the round must free a decision's attached " + "trace"); + TEST_ASSERT_TRUE(__asan_address_is_poisoned(driver_ctx.last)); + TEST_ASSERT_FALSE(__asan_address_is_poisoned(reason)); + TEST_ASSERT_EQUAL_UINT64(4243, reason->jobid); + + g_emitted_trace_decision = NULL; + free_PendReason_ptr(&reason); +#else + TEST_IGNORE_MESSAGE("needs an ASAN build"); +#endif +} + int main(void) { UNITY_BEGIN(); @@ -1028,5 +1136,6 @@ int main(void) RUN_TEST(test_null_driver_ctx_is_safe_noop); RUN_TEST(test_recorded_begin_round_equals_session_start); RUN_TEST(test_round_frees_decision_after_callback); + RUN_TEST(test_round_skips_trace_only_and_frees_traces); return UNITY_END(); }