From 10ea92aca24c6b70769f6588559f2bee742b3068 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Thu, 16 Nov 2023 20:32:16 -0500 Subject: [PATCH] i#5843 scheduler: Fix assert to allow 0-latency syscalls (#6460) Fixes a < assert from PR #6458 to be <=, to allow the pre-syscall timestamp to equal the post-syscall timestamp. Adds a test that fails without the fix. Issue: #5843 --- clients/drcachesim/scheduler/scheduler.cpp | 2 +- .../drcachesim/tests/scheduler_unit_tests.cpp | 110 ++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/clients/drcachesim/scheduler/scheduler.cpp b/clients/drcachesim/scheduler/scheduler.cpp index 8a63171080b..14fda834445 100644 --- a/clients/drcachesim/scheduler/scheduler.cpp +++ b/clients/drcachesim/scheduler/scheduler.cpp @@ -1576,7 +1576,7 @@ scheduler_tmpl_t::syscall_incurs_switch(input_info_t *in return input->processing_maybe_blocking_syscall; } assert(input->pre_syscall_timestamp > 0); - assert(input->pre_syscall_timestamp < post_time); + assert(input->pre_syscall_timestamp <= post_time); uint64_t latency = post_time - input->pre_syscall_timestamp; VPRINT(this, 3, "input %d %ssyscall latency: %" PRIu64 "\n", input->index, input->processing_maybe_blocking_syscall ? "maybe-blocking " : "", latency); diff --git a/clients/drcachesim/tests/scheduler_unit_tests.cpp b/clients/drcachesim/tests/scheduler_unit_tests.cpp index 2544305358b..117e6f73e98 100644 --- a/clients/drcachesim/tests/scheduler_unit_tests.cpp +++ b/clients/drcachesim/tests/scheduler_unit_tests.cpp @@ -1653,12 +1653,122 @@ test_synthetic_with_syscalls_precise() assert(res); } +static void +test_synthetic_with_syscalls_latencies() +{ + std::cerr << "\n----------------\nTesting syscall latency switches\n"; + static constexpr memref_tid_t TID_A = 42; + static constexpr memref_tid_t TID_B = 99; + static constexpr int SYSNUM = 202; + std::vector refs_A = { + /* clang-format off */ + make_thread(TID_A), + make_pid(1), + make_version(TRACE_ENTRY_VERSION), + make_timestamp(20), + make_instr(10), + // Test 0 latency. + make_timestamp(120), + make_marker(TRACE_MARKER_TYPE_SYSCALL, SYSNUM), + make_timestamp(120), + make_instr(10), + // Test large but too-short latency. + make_timestamp(200), + make_marker(TRACE_MARKER_TYPE_SYSCALL, SYSNUM), + make_timestamp(699), + make_instr(10), + // Test just large enough latency, with func markers in between. + make_timestamp(1000), + make_marker(TRACE_MARKER_TYPE_SYSCALL, SYSNUM), + make_marker(TRACE_MARKER_TYPE_MAYBE_BLOCKING_SYSCALL, 0), + make_marker(TRACE_MARKER_TYPE_FUNC_ID, 100), + make_marker(TRACE_MARKER_TYPE_FUNC_ARG, 42), + make_timestamp(1100), + make_marker(TRACE_MARKER_TYPE_CPU_ID, 1), + make_marker(TRACE_MARKER_TYPE_FUNC_ID, 100), + make_marker(TRACE_MARKER_TYPE_FUNC_RETVAL, 0), + make_instr(12), + make_exit(TID_A), + /* clang-format on */ + }; + std::vector refs_B = { + /* clang-format off */ + make_thread(TID_B), + make_pid(1), + make_version(TRACE_ENTRY_VERSION), + make_timestamp(2000), + make_instr(20), + make_instr(21), + make_exit(TID_B), + /* clang-format on */ + }; + std::vector readers; + readers.emplace_back(std::unique_ptr(new mock_reader_t(refs_A)), + std::unique_ptr(new mock_reader_t()), TID_A); + readers.emplace_back(std::unique_ptr(new mock_reader_t(refs_B)), + std::unique_ptr(new mock_reader_t()), TID_B); + 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=*/4); + scheduler_t scheduler; + if (scheduler.init(sched_inputs, 1, sched_ops) != scheduler_t::STATUS_SUCCESS) + assert(false); + auto *stream = scheduler.get_stream(0); + memref_t memref; + std::vector refs; + for (scheduler_t::stream_status_t status = stream->next_record(memref); + status != scheduler_t::STATUS_EOF; status = stream->next_record(memref)) { + if (status == scheduler_t::STATUS_WAIT) + continue; + assert(status == scheduler_t::STATUS_OK); + refs.push_back(memref); + } + std::vector entries; + int idx = 0; + bool res = true; + res = res && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) && + check_ref(refs, idx, TID_A, TRACE_TYPE_INSTR) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) && + check_ref(refs, idx, TID_A, TRACE_TYPE_INSTR) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) && + check_ref(refs, idx, TID_A, TRACE_TYPE_INSTR) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_MAYBE_BLOCKING_SYSCALL) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_FUNC_ID) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_FUNC_ARG) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CPU_ID) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_FUNC_ID) && + check_ref(refs, idx, TID_A, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_FUNC_RETVAL) && + // Shouldn't switch until after all the syscall's markers. + check_ref(refs, idx, TID_B, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION) && + check_ref(refs, idx, TID_B, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP) && + check_ref(refs, idx, TID_B, TRACE_TYPE_INSTR) && + check_ref(refs, idx, TID_B, TRACE_TYPE_INSTR) && + check_ref(refs, idx, TID_B, TRACE_TYPE_THREAD_EXIT) && + check_ref(refs, idx, TID_A, TRACE_TYPE_INSTR) && + check_ref(refs, idx, TID_A, TRACE_TYPE_THREAD_EXIT); + assert(res); +} + static void test_synthetic_with_syscalls() { test_synthetic_with_syscalls_multiple(); test_synthetic_with_syscalls_single(); test_synthetic_with_syscalls_precise(); + test_synthetic_with_syscalls_latencies(); } #if (defined(X86_64) || defined(ARM_64)) && defined(HAS_ZIP)