From c38045af0ad1950a748e437f0e67b7df649c4c1d Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Fri, 27 Sep 2024 17:31:08 -0400 Subject: [PATCH] i#6822 unscheduled: Fix bug making small timeouts infinite (#7006) 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 --- clients/drcachesim/scheduler/scheduler.cpp | 6 ++ .../drcachesim/tests/scheduler_unit_tests.cpp | 71 +++++++++++++++++-- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler.cpp b/clients/drcachesim/scheduler/scheduler.cpp index 78fc5d908f2..ffbb3004309 100644 --- a/clients/drcachesim/scheduler/scheduler.cpp +++ b/clients/drcachesim/scheduler/scheduler.cpp @@ -3478,6 +3478,9 @@ scheduler_tmpl_t::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()); @@ -3506,6 +3509,9 @@ scheduler_tmpl_t::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()); diff --git a/clients/drcachesim/tests/scheduler_unit_tests.cpp b/clients/drcachesim/tests/scheduler_unit_tests.cpp index 53d37ae1bae..b151c1ff9b9 100644 --- a/clients/drcachesim/tests/scheduler_unit_tests.cpp +++ b/clients/drcachesim/tests/scheduler_unit_tests.cpp @@ -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. @@ -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 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 readers; + readers.emplace_back(std::unique_ptr(new mock_reader_t(refs_A)), + std::unique_ptr(new mock_reader_t()), TID_A); + static const char *const CORE0_SCHED_STRING = "...A......._A."; + + std::vector 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 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() { @@ -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();