From 8e4fd76b132d7a599002ed48e1b557b41a5539dc Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Fri, 6 Sep 2024 11:38:06 -0400 Subject: [PATCH 1/2] i#6938 sched migrate: Fix quantum underflow Fix bug underflowing quantum time when trying to correct overshoot but the quantum just expired and so is 0; or, we're replaying. Adds an assert. A test will come in the forthcoming rebalance test for per-output runqueues, which is where this bug was discovered. Issue: #6938 --- clients/drcachesim/scheduler/scheduler.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler.cpp b/clients/drcachesim/scheduler/scheduler.cpp index f5107f2e2f9..8fe6f2c8130 100644 --- a/clients/drcachesim/scheduler/scheduler.cpp +++ b/clients/drcachesim/scheduler/scheduler.cpp @@ -3488,7 +3488,7 @@ scheduler_tmpl_t::next_record(output_ordinal_t output, VPRINT(this, 4, "next_record[%d]: input %d hit end of instr quantum\n", output, input->index); - preempt = !need_new_input; + preempt = true; need_new_input = true; input->instrs_in_quantum = 0; ++outputs_[output] @@ -3510,10 +3510,12 @@ scheduler_tmpl_t::next_record(output_ordinal_t output, // in between (e.g., scatter/gather long sequence of reads/writes) by // setting input->switching_pre_instruction. record_type_is_instr_boundary(record, outputs_[output].last_record)) { - VPRINT(this, 4, - "next_record[%d]: hit end of time quantum after %" PRIu64 "\n", - output, input->time_spent_in_quantum); - preempt = !need_new_input; + VPRINT( + this, 4, + "next_record[%d]: input %d hit end of time quantum after %" PRIu64 + "\n", + output, input->index, input->time_spent_in_quantum); + preempt = true; need_new_input = true; input->time_spent_in_quantum = 0; ++outputs_[output] @@ -3555,12 +3557,16 @@ scheduler_tmpl_t::next_record(output_ordinal_t output, lock.lock(); VPRINT(this, 5, "next_record_mid[%d]: switching from %d to %d\n", output, prev_input, outputs_[output].cur_input); - if (!preempt) { + if (!preempt && // Already reset to 0 for preempt. + options_.mapping == MAP_TO_ANY_OUTPUT) { if (options_.quantum_unit == QUANTUM_INSTRUCTIONS && record_type_is_instr_boundary(record, outputs_[output].last_record)) { + assert(inputs_[prev_input].instrs_in_quantum > 0); --inputs_[prev_input].instrs_in_quantum; } else if (options_.quantum_unit == QUANTUM_TIME) { + assert(inputs_[prev_input].time_spent_in_quantum >= + cur_time - prev_time_in_quantum); inputs_[prev_input].time_spent_in_quantum -= (cur_time - prev_time_in_quantum); } From 86b1a23a9edfde4abdc3361722f881d0cbded5d5 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Tue, 10 Sep 2024 14:58:59 -0400 Subject: [PATCH 2/2] Clarify comments --- clients/drcachesim/scheduler/scheduler.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler.cpp b/clients/drcachesim/scheduler/scheduler.cpp index 8fe6f2c8130..6a789d8d2e2 100644 --- a/clients/drcachesim/scheduler/scheduler.cpp +++ b/clients/drcachesim/scheduler/scheduler.cpp @@ -3546,9 +3546,10 @@ scheduler_tmpl_t::next_record(output_ordinal_t output, res != sched_type_t::STATUS_SKIPPED) return res; if (outputs_[output].cur_input != prev_input) { - // TODO i#5843: Queueing here and in a few other places gets the - // ordinals off: we need to undo the ordinal increases to avoid - // over-counting while queued and double-counting when we resume. + // TODO i#5843: Queueing here and in a few other places gets the stream + // record and instruction ordinals off: we need to undo the ordinal + // increases to avoid over-counting while queued and double-counting + // when we resume. // In some cases we need to undo this on the output stream too. // So we should set suppress_ref_count_ in the input to get // is_record_synthetic() (and have our stream class check that @@ -3557,8 +3558,10 @@ scheduler_tmpl_t::next_record(output_ordinal_t output, lock.lock(); VPRINT(this, 5, "next_record_mid[%d]: switching from %d to %d\n", output, prev_input, outputs_[output].cur_input); - if (!preempt && // Already reset to 0 for preempt. - options_.mapping == MAP_TO_ANY_OUTPUT) { + // We need to offset the {instrs,time_spent}_in_quantum values from + // overshooting during dynamic scheduling, unless this is a preempt when + // we've already reset to 0. + if (!preempt && options_.mapping == MAP_TO_ANY_OUTPUT) { if (options_.quantum_unit == QUANTUM_INSTRUCTIONS && record_type_is_instr_boundary(record, outputs_[output].last_record)) {