Skip to content

Commit

Permalink
i#6822 unscheduled: Fix bug making small timeouts infinite (#7006)
Browse files Browse the repository at this point in the history
Fixes a bug for going-unscheduled actions where scaling a small timeout
rounds it down to 0, which is then interpreted as an infinite timeout.

Adds a unit test that fails without the fix.

Issue: #6822
  • Loading branch information
derekbruening authored Sep 27, 2024
1 parent 1374d52 commit c38045a
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 4 deletions.
6 changes: 6 additions & 0 deletions clients/drcachesim/scheduler/scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3478,6 +3478,9 @@ scheduler_tmpl_t<RecordType, ReaderType>::process_marker(input_info_t &input,
input.unscheduled = true;
if (input.syscall_timeout_arg > 0) {
input.blocked_time = scale_blocked_time(input.syscall_timeout_arg);
// Clamp at 1 since 0 means an infinite timeout for unscheduled=true.
if (input.blocked_time == 0)
input.blocked_time = 1;
input.blocked_start_time = get_output_time(output);
VPRINT(this, 3, "input %d unscheduled for %" PRIu64 " @%" PRIu64 "\n",
input.index, input.blocked_time, input.reader->get_last_timestamp());
Expand Down Expand Up @@ -3506,6 +3509,9 @@ scheduler_tmpl_t<RecordType, ReaderType>::process_marker(input_info_t &input,
input.unscheduled = true;
if (input.syscall_timeout_arg > 0) {
input.blocked_time = scale_blocked_time(input.syscall_timeout_arg);
// Clamp at 1 since 0 means an infinite timeout for unscheduled=true.
if (input.blocked_time == 0)
input.blocked_time = 1;
input.blocked_start_time = get_output_time(output);
VPRINT(this, 3, "input %d unscheduled for %" PRIu64 " @%" PRIu64 "\n",
input.index, input.blocked_time, input.reader->get_last_timestamp());
Expand Down
71 changes: 67 additions & 4 deletions clients/drcachesim/tests/scheduler_unit_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4534,7 +4534,7 @@ test_direct_switch()
}

static void
test_unscheduled()
test_unscheduled_base()
{
std::cerr << "\n----------------\nTesting unscheduled inputs\n";
// We have just 1 output to better control the order and avoid flakiness.
Expand Down Expand Up @@ -5166,6 +5166,72 @@ test_unscheduled_initially_roi()
#endif
}

static void
test_unscheduled_small_timeout()
{
// Test that a small timeout scaled to 0 does not turn into an infinite timeout.
std::cerr << "\n----------------\nTesting unscheduled input with small timeout\n";
static constexpr int NUM_OUTPUTS = 1;
// 4*0.1 rounds to 0 (the scheduler's cast rounds any fraction down).
static constexpr int UNSCHEDULE_TIMEOUT = 4;
static constexpr double BLOCK_SCALE = 0.1;
static constexpr memref_tid_t TID_A = 100;
std::vector<trace_entry_t> refs_A = {
make_thread(TID_A),
make_pid(1),
make_version(TRACE_ENTRY_VERSION),
make_timestamp(1001),
make_marker(TRACE_MARKER_TYPE_CPU_ID, 0),
make_instr(/*pc=*/101),
make_timestamp(1002),
make_marker(TRACE_MARKER_TYPE_CPU_ID, 0),
make_marker(TRACE_MARKER_TYPE_SYSCALL, 999),
make_marker(TRACE_MARKER_TYPE_MAYBE_BLOCKING_SYSCALL, 0),
make_marker(TRACE_MARKER_TYPE_SYSCALL_ARG_TIMEOUT, UNSCHEDULE_TIMEOUT),
make_marker(TRACE_MARKER_TYPE_SYSCALL_UNSCHEDULE, 0),
make_timestamp(2002),
make_instr(/*pc=*/102),
make_exit(TID_A),
};
{
std::vector<scheduler_t::input_reader_t> readers;
readers.emplace_back(std::unique_ptr<mock_reader_t>(new mock_reader_t(refs_A)),
std::unique_ptr<mock_reader_t>(new mock_reader_t()), TID_A);
static const char *const CORE0_SCHED_STRING = "...A......._A.";

std::vector<scheduler_t::input_workload_t> sched_inputs;
sched_inputs.emplace_back(std::move(readers));
scheduler_t::scheduler_options_t sched_ops(scheduler_t::MAP_TO_ANY_OUTPUT,
scheduler_t::DEPENDENCY_TIMESTAMPS,
scheduler_t::SCHEDULER_DEFAULTS,
/*verbosity=*/3);
// We use our mock's time==instruction count for a deterministic result.
sched_ops.quantum_unit = scheduler_t::QUANTUM_TIME;
sched_ops.time_units_per_us = 1.;
sched_ops.block_time_multiplier = BLOCK_SCALE;
scheduler_t scheduler;
if (scheduler.init(sched_inputs, NUM_OUTPUTS, std::move(sched_ops)) !=
scheduler_t::STATUS_SUCCESS)
assert(false);
std::vector<std::string> sched_as_string =
run_lockstep_simulation(scheduler, NUM_OUTPUTS, TID_A, /*send_time=*/true);
for (int i = 0; i < NUM_OUTPUTS; i++) {
std::cerr << "cpu #" << i << " schedule: " << sched_as_string[i] << "\n";
}
assert(sched_as_string[0] == CORE0_SCHED_STRING);
}
}

static void
test_unscheduled()
{
test_unscheduled_base();
test_unscheduled_fallback();
test_unscheduled_initially();
test_unscheduled_initially_roi();
test_unscheduled_small_timeout();
}

static void
test_kernel_switch_sequences()
{
Expand Down Expand Up @@ -5802,9 +5868,6 @@ test_main(int argc, const char *argv[])
test_inactive();
test_direct_switch();
test_unscheduled();
test_unscheduled_fallback();
test_unscheduled_initially();
test_unscheduled_initially_roi();
test_kernel_switch_sequences();
test_random_schedule();
test_record_scheduler();
Expand Down

0 comments on commit c38045a

Please sign in to comment.