From bc656ca3a54e028318e2390e6590bbb6cfdaa920 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 13 Feb 2025 20:39:48 -0500 Subject: [PATCH 01/19] i#7157: Inject syscall templates dynamically in scheduler Adds support to the drmemtrace scheduler for injecting system call trace templates dynamically during scheduling. This obviates the need to create a separate statically-injected trace with system call trace templates. Reuses context switch trace injection code to the extent possible. Adds a new analyzer flag -sched_syscall_file and new scheduler_options_t fields to allow specifying the system call trace template file. Issue: #7157 --- api/docs/release.dox | 5 + clients/drcachesim/analyzer_multi.cpp | 1 + clients/drcachesim/common/options.cpp | 10 +- clients/drcachesim/common/options.h | 1 + clients/drcachesim/scheduler/scheduler.h | 25 ++ .../scheduler/scheduler_dynamic.cpp | 8 +- .../drcachesim/scheduler/scheduler_impl.cpp | 186 ++++++++++--- clients/drcachesim/scheduler/scheduler_impl.h | 42 ++- .../drcachesim/tests/scheduler_unit_tests.cpp | 260 +++++++++++++++++- 9 files changed, 476 insertions(+), 62 deletions(-) diff --git a/api/docs/release.dox b/api/docs/release.dox index 56760ee2b5..21a6c4c209 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -141,6 +141,11 @@ Further non-compatibility-affecting changes include: instructions in the trace. This works for traces that have embedded instruction encodings in them, and also for legacy traces without embedded encodings where the encodings are obtained from the application binaries instead. + - Added a new drmemtrace analyzer flag -sched_syscall_file to allow specifying the + system call trace template file to be used for dynamic injection of system call trace + templates. Added similar options for the drmemtrace scheduler + #dynamorio::drmemtrace::scheduler_tmpl_t::scheduler_options_t::kernel_syscall_trace_path, + and #dynamorio::drmemtrace::scheduler_tmpl_t::scheduler_options_t::kernel_syscall_reader. **************************************************
diff --git a/clients/drcachesim/analyzer_multi.cpp b/clients/drcachesim/analyzer_multi.cpp index 3d864a29c8..be1d698d84 100644 --- a/clients/drcachesim/analyzer_multi.cpp +++ b/clients/drcachesim/analyzer_multi.cpp @@ -689,6 +689,7 @@ analyzer_multi_tmpl_t::init_dynamic_schedule() } #endif sched_ops.kernel_switch_trace_path = op_sched_switch_file.get_value(); + sched_ops.kernel_syscall_trace_path = op_sched_syscall_file.get_value(); return sched_ops; } diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index 69976820cd..a48a831a42 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -1039,12 +1039,20 @@ droption_t #endif droption_t op_sched_switch_file( DROPTION_SCOPE_FRONTEND, "sched_switch_file", "", - "Path to file holding context switch sequences", + "Path to file holding kernel context switch sequences", "Applies to -core_sharded and -core_serial. Path to file holding context switch " "sequences. The file can contain multiple sequences each with regular trace headers " "and the sequence proper bracketed by TRACE_MARKER_TYPE_CONTEXT_SWITCH_START and " "TRACE_MARKER_TYPE_CONTEXT_SWITCH_END markers."); +droption_t op_sched_syscall_file( + DROPTION_SCOPE_FRONTEND, "sched_syscall_file", "", + "Path to file holding kernel system call sequences", + "Applies to -core_sharded and -core_serial. Path to file holding system call " + "sequences. The file can contain multiple sequences each with regular trace headers " + "and the sequence proper bracketed by TRACE_MARKER_TYPE_SYSCALL_TRACE_START and " + "TRACE_MARKER_TYPE_SYSCALL_TRACE_END markers."); + droption_t op_sched_randomize( DROPTION_SCOPE_FRONTEND, "sched_randomize", false, "Pick next inputs randomly on context switches", diff --git a/clients/drcachesim/common/options.h b/clients/drcachesim/common/options.h index 562019f64a..50bbbadf43 100644 --- a/clients/drcachesim/common/options.h +++ b/clients/drcachesim/common/options.h @@ -216,6 +216,7 @@ extern dynamorio::droption::droption_t op_replay_file; extern dynamorio::droption::droption_t op_cpu_schedule_file; #endif extern dynamorio::droption::droption_t op_sched_switch_file; +extern dynamorio::droption::droption_t op_sched_syscall_file; extern dynamorio::droption::droption_t op_sched_randomize; extern dynamorio::droption::droption_t op_sched_disable_direct_switches; extern dynamorio::droption::droption_t op_sched_infinite_timeouts; diff --git a/clients/drcachesim/scheduler/scheduler.h b/clients/drcachesim/scheduler/scheduler.h index f5d6ae4ca4..f541afad43 100644 --- a/clients/drcachesim/scheduler/scheduler.h +++ b/clients/drcachesim/scheduler/scheduler.h @@ -829,6 +829,31 @@ template class scheduler_tmpl_t { * when raising this value on uneven inputs. */ double exit_if_fraction_inputs_left = 0.1; + /** + * Input file containing template sequences of kernel system call code. + * Each sequence must start with a #TRACE_MARKER_TYPE_SYSCALL_TRACE_START + * marker and end with #TRACE_MARKER_TYPE_SYSCALL_TRACE_END. + * The values of each marker must hold the system call number value + * indicating the system call it corresponds to. Sequences for + * multiple system calls are concatenated into a single file. + * Each sequence should be in the regular offline drmemtrace format. + * The sequence is inserted into the output stream on the + * #TRACE_MARKER_TYPE_SYSCALL marker with the indicated value. + * The same file (or reader) must be passed when replaying as this kernel + * code is not stored when recording. + * An alternative to passing the file path is to pass #kernel_syscall_reader + * and #kernel_syscall_reader_end. + */ + std::string kernel_syscall_trace_path; + /** + * An alternative to #kernel_syscall_trace_path is to pass a reader and + * #kernel_syscall_reader_end. See the description of #kernel_syscall_trace_path. + * This field is only examined if #kernel_syscall_trace_path is empty. + * The scheduler will call the init() function for the reader. + */ + std::unique_ptr kernel_syscall_reader; + /** The end reader for #kernel_syscall_reader. */ + std::unique_ptr kernel_syscall_reader_end; // When adding new options, also add to print_configuration(). }; diff --git a/clients/drcachesim/scheduler/scheduler_dynamic.cpp b/clients/drcachesim/scheduler/scheduler_dynamic.cpp index 8f8dad07eb..ff1b999b6d 100644 --- a/clients/drcachesim/scheduler/scheduler_dynamic.cpp +++ b/clients/drcachesim/scheduler/scheduler_dynamic.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2023-2024 Google, Inc. All rights reserved. + * Copyright (c) 2023-2025 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -596,16 +596,10 @@ scheduler_dynamic_tmpl_t::process_marker( case TRACE_MARKER_TYPE_CONTEXT_SWITCH_START: outputs_[output].in_context_switch_code = true; break; - case TRACE_MARKER_TYPE_SYSCALL_TRACE_START: - outputs_[output].in_syscall_code = true; - break; case TRACE_MARKER_TYPE_CONTEXT_SWITCH_END: // We have to delay until the next record. outputs_[output].hit_switch_code_end = true; break; - case TRACE_MARKER_TYPE_SYSCALL_TRACE_END: - outputs_[output].in_syscall_code = false; - break; case TRACE_MARKER_TYPE_TIMESTAMP: // Syscall sequences are not expected to have a timestamp. assert(!outputs_[output].in_syscall_code); diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 3ff55edf36..b527607e62 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1,5 +1,5 @@ /* ********************************************************** - * Copyright (c) 2023-2024 Google, Inc. All rights reserved. + * Copyright (c) 2023-2025 Google, Inc. All rights reserved. * **********************************************************/ /* @@ -629,6 +629,12 @@ scheduler_impl_tmpl_t::print_configuration() options_.honor_infinite_timeouts); VPRINT(this, 1, " %-25s : %f\n", "exit_if_fraction_inputs_left", options_.exit_if_fraction_inputs_left); + VPRINT(this, 1, " %-25s : %s\n", "kernel_syscall_trace_path", + options_.kernel_syscall_trace_path.c_str()); + VPRINT(this, 1, " %-25s : %p\n", "kernel_syscall_reader", + options_.kernel_syscall_reader.get()); + VPRINT(this, 1, " %-25s : %p\n", "kernel_syscall_reader_end", + options_.kernel_syscall_reader_end.get()); } template @@ -871,6 +877,10 @@ scheduler_impl_tmpl_t::init( if (res != sched_type_t::STATUS_SUCCESS) return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + res = read_syscall_sequences(); + if (res != sched_type_t::STATUS_SUCCESS) + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + // Determine whether we need to read ahead in the inputs. There are cases where we // do not want to do that as it would block forever if the inputs are not available // (e.g., online analysis IPC readers); it also complicates ordinals so we avoid it @@ -1385,28 +1395,54 @@ template typename scheduler_tmpl_t::scheduler_status_t scheduler_impl_tmpl_t::read_switch_sequences() { - std::unique_ptr reader, reader_end; - if (!options_.kernel_switch_trace_path.empty()) { - reader = get_reader(options_.kernel_switch_trace_path, verbosity_); + return read_kernel_sequences(switch_sequence_, options_.kernel_switch_trace_path, + std::move(options_.kernel_switch_reader), + std::move(options_.kernel_switch_reader_end), + TRACE_MARKER_TYPE_CONTEXT_SWITCH_START, + TRACE_MARKER_TYPE_CONTEXT_SWITCH_END, "context switch"); +} + +template +typename scheduler_tmpl_t::scheduler_status_t +scheduler_impl_tmpl_t::read_syscall_sequences() +{ + + return read_kernel_sequences(syscall_sequence_, options_.kernel_syscall_trace_path, + std::move(options_.kernel_syscall_reader), + std::move(options_.kernel_syscall_reader_end), + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, "system call"); +} + +template +template +typename scheduler_tmpl_t::scheduler_status_t +scheduler_impl_tmpl_t::read_kernel_sequences( + std::unordered_map, custom_hash_t> + &sequence, + std::string trace_path, std::unique_ptr reader, + std::unique_ptr reader_end, trace_marker_type_t start_marker, + trace_marker_type_t end_marker, std::string sequence_type) +{ + if (!trace_path.empty()) { + reader = get_reader(trace_path, verbosity_); if (!reader || !reader->init()) { - error_string_ += - "Failed to open kernel switch file " + options_.kernel_switch_trace_path; + error_string_ += "Failed to open file for kernel " + sequence_type + + " sequences: " + trace_path; return sched_type_t::STATUS_ERROR_FILE_OPEN_FAILED; } reader_end = get_default_reader(); - } else if (!options_.kernel_switch_reader) { + } else if (!reader) { // No switch data provided. return sched_type_t::STATUS_SUCCESS; } else { - if (!options_.kernel_switch_reader_end) { - error_string_ += "Provided kernel switch reader but no end"; + if (!reader_end) { + error_string_ += "Provided kernel " + sequence_type + " reader but no end"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } - reader = std::move(options_.kernel_switch_reader); - reader_end = std::move(options_.kernel_switch_reader_end); // We own calling init() as it can block. if (!reader->init()) { - error_string_ += "Failed to init kernel switch reader"; + error_string_ += "Failed to init kernel " + sequence_type + " reader"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } } @@ -1414,31 +1450,33 @@ scheduler_impl_tmpl_t::read_switch_sequences() // memory and don't need to stream them on every use. // We read a single stream, even if underneath these are split into subfiles // in an archive. - switch_type_t switch_type = sched_type_t::SWITCH_INVALID; + SequenceKey sequence_key; + bool in_sequence = false; while (*reader != *reader_end) { RecordType record = **reader; // Only remember the records between the markers. trace_marker_type_t marker_type = TRACE_MARKER_TYPE_RESERVED_END; uintptr_t marker_value = 0; if (record_type_is_marker(record, marker_type, marker_value) && - marker_type == TRACE_MARKER_TYPE_CONTEXT_SWITCH_START) { - switch_type = static_cast(marker_value); - if (!switch_sequence_[switch_type].empty()) { - error_string_ += "Duplicate context switch sequence type found"; + marker_type == start_marker) { + sequence_key = static_cast(marker_value); + in_sequence = true; + if (!sequence[sequence_key].empty()) { + error_string_ += "Duplicate " + sequence_type + " sequence found"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } } - if (switch_type != sched_type_t::SWITCH_INVALID) - switch_sequence_[switch_type].push_back(record); + if (in_sequence) + sequence[sequence_key].push_back(record); if (record_type_is_marker(record, marker_type, marker_value) && - marker_type == TRACE_MARKER_TYPE_CONTEXT_SWITCH_END) { - if (static_cast(marker_value) != switch_type) { - error_string_ += "Context switch marker values mismatched"; + marker_type == end_marker) { + if (static_cast(marker_value) != sequence_key) { + error_string_ += sequence_type + " marker values mismatched"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } - VPRINT(this, 1, "Read %zu kernel context switch records for type %d\n", - switch_sequence_[switch_type].size(), switch_type); - switch_type = sched_type_t::SWITCH_INVALID; + VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", + sequence[sequence_key].size(), sequence_type.c_str(), sequence_key); + in_sequence = false; } ++(*reader); } @@ -1602,6 +1640,28 @@ scheduler_impl_tmpl_t::get_initial_input_content( return sched_type_t::STATUS_SUCCESS; } +template +typename scheduler_tmpl_t::stream_status_t +scheduler_impl_tmpl_t::inject_kernel_sequence( + std::vector &sequence, input_info_t *input) +{ + // Inject kernel template code. Since the injected records belong to this + // input (the kernel is acting on behalf of this input) we insert them into the + // input's queue, but ahead of any prior queued items. This is why we walk in + // reverse, for the push_front calls to the deque. We update the tid of the + // records here to match. They are considered as is_record_synthetic() and do + // not affect input stream ordinals. + // XXX: These will appear before the top headers of a new thread which is slightly + // odd to have regular records with the new tid before the top headers. + if (sequence.empty()) + return stream_status_t::STATUS_EOF; + for (int i = static_cast(sequence.size()) - 1; i >= 0; --i) { + RecordType record = sequence[i]; + record_type_set_tid(record, input->tid); + input->queue.push_front(record); + } + return stream_status_t::STATUS_OK; +} template typename scheduler_tmpl_t::scheduler_status_t scheduler_impl_tmpl_t::open_reader( @@ -1778,7 +1838,7 @@ scheduler_impl_tmpl_t::is_record_synthetic( int index = outputs_[output].cur_input; if (index < 0) return false; - if (outputs_[output].in_context_switch_code) + if (outputs_[output].in_context_switch_code || outputs_[output].in_syscall_code) return true; return inputs_[index].reader->is_record_synthetic(); } @@ -2299,25 +2359,17 @@ scheduler_impl_tmpl_t::set_cur_input( switch_type = sched_type_t::SWITCH_PROCESS; else switch_type = sched_type_t::SWITCH_THREAD; - // Inject kernel context switch code. Since the injected records belong to this - // input (the kernel is acting on behalf of this input) we insert them into the - // input's queue, but ahead of any prior queued items. This is why we walk in - // reverse, for the push_front calls to the deque. We update the tid of the - // records here to match. They are considered as is_record_synthetic() and do - // not affect input stream ordinals. - // XXX: These will appear before the top headers of a new thread which is slightly - // odd to have regular records with the new tid before the top headers. - if (!switch_sequence_[switch_type].empty()) { - for (int i = static_cast(switch_sequence_[switch_type].size()) - 1; - i >= 0; --i) { - RecordType record = switch_sequence_[switch_type][i]; - record_type_set_tid(record, inputs_[input].tid); - inputs_[input].queue.push_front(record); + if (switch_sequence_.find(switch_type) != switch_sequence_.end()) { + stream_status_t res = + inject_kernel_sequence(switch_sequence_[switch_type], &inputs_[input]); + if (res == stream_status_t::STATUS_OK) { + VPRINT(this, 3, + "Inserted %zu switch records for type %d from %d.%d to %d.%d\n", + switch_sequence_[switch_type].size(), switch_type, prev_workload, + outputs_[output].prev_input, inputs_[input].workload, input); + } else if (res != stream_status_t::STATUS_EOF) { + return res; } - VPRINT(this, 3, - "Inserted %zu switch records for type %d from %d.%d to %d.%d\n", - switch_sequence_[switch_type].size(), switch_type, prev_workload, - outputs_[output].prev_input, inputs_[input].workload, input); } } @@ -2437,6 +2489,33 @@ scheduler_impl_tmpl_t::update_switch_stats( } } +template +void +scheduler_impl_tmpl_t::process_marker(RecordType record, + output_ordinal_t output) +{ + if (outputs_[output].hit_syscall_code_end) { + // We have to delay so the end marker is still in_syscall_code. + outputs_[output].in_syscall_code = false; + outputs_[output].hit_syscall_code_end = false; + } + + trace_marker_type_t marker_type; + uintptr_t marker_value = 0; + if (!record_type_is_marker(record, marker_type, marker_value)) + return; + switch (marker_type) { + case TRACE_MARKER_TYPE_SYSCALL_TRACE_START: + outputs_[output].in_syscall_code = true; + break; + case TRACE_MARKER_TYPE_SYSCALL_TRACE_END: + // We have to delay until the next record. + outputs_[output].hit_syscall_code_end = true; + break; + default: break; + } +} + template typename scheduler_tmpl_t::stream_status_t scheduler_impl_tmpl_t::next_record(output_ordinal_t output, @@ -2571,6 +2650,9 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp if (input->instrs_pre_read > 0 && record_type_is_instr(record)) --input->instrs_pre_read; VDO(this, 5, print_record(record);); + + process_marker(record, output); + bool need_new_input = false; bool preempt = false; uint64_t blocked_time = 0; @@ -2673,6 +2755,22 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp outputs_[output].last_record = record; record_type_has_tid(record, input->last_record_tid); record_type_has_pid(record, input->pid); + + trace_marker_type_t marker_type; + uintptr_t marker_value = 0; + if (record_type_is_marker(record, marker_type, marker_value) && + marker_type == TRACE_MARKER_TYPE_SYSCALL && + syscall_sequence_.find(marker_value) != syscall_sequence_.end()) { + stream_status_t res = + inject_kernel_sequence(syscall_sequence_[marker_value], input); + if (res == stream_status_t::STATUS_OK) { + VPRINT(this, 3, "Inserted %zu syscall records for syscall %lu to %d.%d\n", + syscall_sequence_[marker_value].size(), marker_value, input->workload, + input->index); + } else if (res != stream_status_t::STATUS_EOF) { + return res; + } + } return sched_type_t::STATUS_OK; } diff --git a/clients/drcachesim/scheduler/scheduler_impl.h b/clients/drcachesim/scheduler/scheduler_impl.h index 29a429be85..bf29d0e785 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.h +++ b/clients/drcachesim/scheduler/scheduler_impl.h @@ -409,6 +409,9 @@ template class scheduler_impl_tmpl_t update_switch_stats(output_ordinal_t output, input_ordinal_t prev_input, input_ordinal_t new_input); + void + process_marker(RecordType record, output_ordinal_t output); + /// /////////////////////////////////////////////////////////////////////////// @@ -481,6 +484,7 @@ template class scheduler_impl_tmpl_t // Indirected so we can store it in our vector. std::unique_ptr> active; bool in_syscall_code = false; + bool hit_syscall_code_end = false; bool in_context_switch_code = false; bool hit_switch_code_end = false; // Used for time-based quanta. @@ -542,6 +546,14 @@ template class scheduler_impl_tmpl_t uint64_t timestamp; }; + template struct custom_hash_t { + std::size_t + operator()(const IntCastable &st) const + { + return std::hash()(static_cast(st)); + } + }; + // Tracks data used while opening inputs. struct input_reader_info_t { std::set only_threads; @@ -736,9 +748,24 @@ template class scheduler_impl_tmpl_t std::vector> &start2stop, std::vector> &all_sched); + template + scheduler_status_t + read_kernel_sequences(std::unordered_map, + custom_hash_t> &sequence, + std::string trace_path, std::unique_ptr reader, + std::unique_ptr reader_end, + trace_marker_type_t start_marker, + trace_marker_type_t end_marker, std::string sequence_type); + scheduler_status_t read_switch_sequences(); + scheduler_status_t + read_syscall_sequences(); + + stream_status_t + inject_kernel_sequence(std::vector &sequence, input_info_t *input); + uint64_t get_time_micros(); @@ -976,15 +1003,14 @@ template class scheduler_impl_tmpl_t } }; std::unordered_map tid2input_; - struct switch_type_hash_t { - std::size_t - operator()(const switch_type_t &st) const - { - return std::hash()(static_cast(st)); - } - }; - std::unordered_map, switch_type_hash_t> + + std::unordered_map, + custom_hash_t> switch_sequence_; + // We specify a custom hash function only to make it generalize with + // switch_sequence_ defined above. + std::unordered_map, custom_hash_t> + syscall_sequence_; // For single_lockstep_output. std::unique_ptr global_stream_; // For online where we currently have to map dynamically observed thread ids diff --git a/clients/drcachesim/tests/scheduler_unit_tests.cpp b/clients/drcachesim/tests/scheduler_unit_tests.cpp index 51bd765e72..c8e3dd1ae3 100644 --- a/clients/drcachesim/tests/scheduler_unit_tests.cpp +++ b/clients/drcachesim/tests/scheduler_unit_tests.cpp @@ -5957,7 +5957,7 @@ test_kernel_switch_sequences() check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CONTEXT_SWITCH_START, scheduler_t::SWITCH_THREAD) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_TIMESTAMP, THREAD_SWITCH_TIMESTAMP) && + TRACE_MARKER_TYPE_TIMESTAMP, TIMESTAMP) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, @@ -5975,7 +5975,7 @@ test_kernel_switch_sequences() check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_CONTEXT_SWITCH_START, scheduler_t::SWITCH_PROCESS) && check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_TIMESTAMP, PROCESS_SWITCH_TIMESTAMP) && + TRACE_MARKER_TYPE_TIMESTAMP, TIMESTAMP) && check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_MARKER, @@ -5989,6 +5989,7 @@ test_kernel_switch_sequences() check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 4, TRACE_TYPE_INSTR); + assert(res); { // Test a bad input sequence. @@ -6035,6 +6036,260 @@ test_kernel_switch_sequences() } } +static void +test_kernel_syscall_sequences() +{ + std::cerr << "\n----------------\nTesting kernel syscall sequences\n"; + static constexpr memref_tid_t TID_IN_SYSCALLS = 1; + static constexpr int SYSCALL_NUM_1 = 42; + static constexpr int SYSCALL_NUM_2 = 43; + static constexpr addr_t SYSCALL_NUM_1_PC_START = 0xfeed101; + static constexpr addr_t SYSCALL_NUM_2_PC_START = 0xcafe101; + std::vector syscall_sequence = { + /* clang-format off */ + make_header(TRACE_ENTRY_VERSION), + make_thread(TID_IN_SYSCALLS), + make_pid(TID_IN_SYSCALLS), + make_version(TRACE_ENTRY_VERSION), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1), + make_instr(SYSCALL_NUM_1_PC_START), + make_instr(SYSCALL_NUM_1_PC_START + 1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1), + // XXX: Currently all syscall traces are concatenated. We may change + // this to use an archive file instead. + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_2), + make_instr(SYSCALL_NUM_2_PC_START), + make_instr(SYSCALL_NUM_2_PC_START + 1), + make_instr(SYSCALL_NUM_2_PC_START + 2), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_2), + make_exit(TID_IN_SYSCALLS), + make_footer(), + /* clang-format on */ + }; + auto syscall_reader = + std::unique_ptr(new mock_reader_t(syscall_sequence)); + auto syscall_reader_end = std::unique_ptr(new mock_reader_t()); + static constexpr int NUM_INPUTS = 3; + static constexpr int NUM_OUTPUTS = 2; + static constexpr int NUM_INSTRS = 9; + static constexpr int INSTR_QUANTUM = 3; + static constexpr uint64_t TIMESTAMP = 44226688; + static constexpr memref_tid_t TID_BASE = 100; + std::vector sched_inputs; + std::vector readers; + for (int input_idx = 0; input_idx < NUM_INPUTS; input_idx++) { + std::vector inputs; + inputs.push_back(make_header(TRACE_ENTRY_VERSION)); + memref_tid_t tid = TID_BASE + input_idx; + inputs.push_back(make_thread(tid)); + inputs.push_back(make_pid(1)); + inputs.push_back(make_version(TRACE_ENTRY_VERSION)); + inputs.push_back(make_timestamp(TIMESTAMP)); + for (int instr_idx = 0; instr_idx < NUM_INSTRS; instr_idx++) { + inputs.push_back(make_instr(42 + instr_idx * 4)); + if (instr_idx % 2 == 0) { + inputs.push_back(make_marker(TRACE_MARKER_TYPE_SYSCALL, + (instr_idx / 2) % 2 == 0 ? SYSCALL_NUM_1 + : SYSCALL_NUM_2)); + } + } + inputs.push_back(make_exit(tid)); + readers.emplace_back(std::unique_ptr(new mock_reader_t(inputs)), + std::unique_ptr(new mock_reader_t()), tid); + } + 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); + sched_ops.quantum_duration_instrs = INSTR_QUANTUM; + sched_ops.kernel_syscall_reader = std::move(syscall_reader); + sched_ops.kernel_syscall_reader_end = std::move(syscall_reader_end); + scheduler_t scheduler; + if (scheduler.init(sched_inputs, NUM_OUTPUTS, std::move(sched_ops)) != + scheduler_t::STATUS_SUCCESS) + assert(false); + + // We have a custom version of run_lockstep_simulation here for more precise + // testing of the markers and instructions and interfaces. + // We record the entire sequence for a detailed check of some records, along with + // a character representation for a higher-level view of the whole sequence. + std::vector outputs(NUM_OUTPUTS, nullptr); + std::vector eof(NUM_OUTPUTS, false); + for (int i = 0; i < NUM_OUTPUTS; i++) + outputs[i] = scheduler.get_stream(i); + int num_eof = 0; + std::vector> refs(NUM_OUTPUTS); + std::vector sched_as_string(NUM_OUTPUTS); + std::vector prev_tid(NUM_OUTPUTS, INVALID_THREAD_ID); + std::vector in_syscall(NUM_OUTPUTS, false); + std::vector prev_in_ord(NUM_OUTPUTS, 0); + std::vector prev_out_ord(NUM_OUTPUTS, 0); + while (num_eof < NUM_OUTPUTS) { + for (int i = 0; i < NUM_OUTPUTS; i++) { + if (eof[i]) + continue; + memref_t memref; + scheduler_t::stream_status_t status = outputs[i]->next_record(memref); + if (status == scheduler_t::STATUS_EOF) { + ++num_eof; + eof[i] = true; + continue; + } + if (status == scheduler_t::STATUS_IDLE) { + sched_as_string[i] += '_'; + continue; + } + assert(status == scheduler_t::STATUS_OK); + refs[i].push_back(memref); + if (memref.instr.tid != prev_tid[i]) { + if (!sched_as_string[i].empty()) + sched_as_string[i] += ','; + sched_as_string[i] += + 'A' + static_cast(memref.instr.tid - TID_BASE); + } + if (memref.marker.type == TRACE_TYPE_MARKER && + memref.marker.marker_type == TRACE_MARKER_TYPE_SYSCALL_TRACE_START) + in_syscall[i] = true; + if (in_syscall[i]) { + // Test that syscall code is marked synthetic. + assert(outputs[i]->is_record_synthetic()); + // Test that dynamically injected syscall code doesn't count toward + // input ordinals, but does toward output ordinals. + assert(outputs[i]->get_input_interface()->get_record_ordinal() == + prev_in_ord[i]); + assert(outputs[i]->get_record_ordinal() > prev_out_ord[i]); + } else + assert(!outputs[i]->is_record_synthetic()); + if (type_is_instr(memref.instr.type)) + sched_as_string[i] += 'i'; + else if (memref.marker.type == TRACE_TYPE_MARKER) { + switch (memref.marker.marker_type) { + case TRACE_MARKER_TYPE_VERSION: sched_as_string[i] += 'v'; break; + case TRACE_MARKER_TYPE_TIMESTAMP: sched_as_string[i] += '0'; break; + case TRACE_MARKER_TYPE_SYSCALL: sched_as_string[i] += 's'; break; + case TRACE_MARKER_TYPE_SYSCALL_TRACE_END: + in_syscall[i] = false; + ANNOTATE_FALLTHROUGH; + case TRACE_MARKER_TYPE_SYSCALL_TRACE_START: + if (memref.marker.marker_value == SYSCALL_NUM_1) + sched_as_string[i] += '1'; + else if (memref.marker.marker_value == SYSCALL_NUM_2) + sched_as_string[i] += '2'; + else + assert(false && "unknown system call num"); + break; + default: sched_as_string[i] += '?'; break; + } + } + prev_tid[i] = memref.instr.tid; + prev_in_ord[i] = outputs[i]->get_input_interface()->get_record_ordinal(); + prev_out_ord[i] = outputs[i]->get_record_ordinal(); + } + } + // Check the high-level strings. + for (int i = 0; i < NUM_OUTPUTS; i++) { + std::cerr << "cpu #" << i << " schedule: " << sched_as_string[i] << "\n"; + } + assert(sched_as_string[0] == + "Av0is1ii1,Cv0is1ii1,Aiis2iii2,Ciis2iii2,Aiis1ii1,Ciis1ii1,Aiis2iii2," + "Ciis2iii2,Aiis1ii1,Ciis1ii1"); + assert(sched_as_string[1] == + "Bv0is1ii1iis2iii2iis1ii1iis2iii2iis1ii1____________________________" + "___________"); + + // Zoom in and check the first few syscall sequences on the first output record + // by record with value checks. + int idx = 0; + bool res = true; + res = res && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP, + TIMESTAMP) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL, + SYSCALL_NUM_1) && + // Syscall_1 trace on first thread. + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) && + + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_VERSION) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_TIMESTAMP, TIMESTAMP) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) && + // Syscall_1 trace on second thread. + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL, + SYSCALL_NUM_2) && + // Syscall_2 trace on first thread. + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_2) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_2); + assert(res); + + { + // Test a bad input sequence. + std::vector bad_syscall_sequence = { + /* clang-format off */ + make_header(TRACE_ENTRY_VERSION), + make_thread(TID_IN_SYSCALLS), + make_pid(TID_IN_SYSCALLS), + make_version(TRACE_ENTRY_VERSION), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1), + make_instr(SYSCALL_NUM_1_PC_START), + make_instr(SYSCALL_NUM_1_PC_START + 1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1), + // Error: duplicate trace for the same syscall. + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1), + make_instr(SYSCALL_NUM_2_PC_START), + make_instr(SYSCALL_NUM_2_PC_START + 1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1), + make_exit(TID_IN_SYSCALLS), + make_footer(), + /* clang-format on */ + }; + auto bad_syscall_reader = + std::unique_ptr(new mock_reader_t(bad_syscall_sequence)); + auto bad_syscall_reader_end = std::unique_ptr(new mock_reader_t()); + std::vector test_sched_inputs; + std::vector readers; + std::vector inputs; + inputs.push_back(make_header(TRACE_ENTRY_VERSION)); + readers.emplace_back(std::unique_ptr(new mock_reader_t(inputs)), + std::unique_ptr(new mock_reader_t()), + TID_BASE); + test_sched_inputs.emplace_back(std::move(readers)); + scheduler_t::scheduler_options_t test_sched_ops( + scheduler_t::MAP_TO_ANY_OUTPUT, scheduler_t::DEPENDENCY_TIMESTAMPS, + scheduler_t::SCHEDULER_DEFAULTS); + test_sched_ops.kernel_syscall_reader = std::move(bad_syscall_reader); + test_sched_ops.kernel_syscall_reader_end = std::move(bad_syscall_reader_end); + scheduler_t test_scheduler; + if (test_scheduler.init(test_sched_inputs, NUM_OUTPUTS, + std::move(test_sched_ops)) != + scheduler_t::STATUS_ERROR_INVALID_PARAMETER) + assert(false); + } +} + void test_random_schedule() { @@ -6728,6 +6983,7 @@ test_main(int argc, const char *argv[]) test_direct_switch(); test_unscheduled(); test_kernel_switch_sequences(); + test_kernel_syscall_sequences(); test_random_schedule(); test_record_scheduler(); test_rebalancing(); From 0aa2f0477602c122ef93ba430ce098e60ba490c7 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 13 Feb 2025 20:56:52 -0500 Subject: [PATCH 02/19] Fix format error --- clients/drcachesim/scheduler/scheduler_impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index b527607e62..0ebf11d3b0 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -2764,7 +2764,7 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp stream_status_t res = inject_kernel_sequence(syscall_sequence_[marker_value], input); if (res == stream_status_t::STATUS_OK) { - VPRINT(this, 3, "Inserted %zu syscall records for syscall %lu to %d.%d\n", + VPRINT(this, 3, "Inserted %zu syscall records for syscall %u to %d.%d\n", syscall_sequence_[marker_value].size(), marker_value, input->workload, input->index); } else if (res != stream_status_t::STATUS_EOF) { From e3aa741b494223e9c7d25c807c697bde7b406c6e Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 13 Feb 2025 21:15:44 -0500 Subject: [PATCH 03/19] Fix format error --- clients/drcachesim/scheduler/scheduler_impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 0ebf11d3b0..5d3f9e5a5b 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -2764,7 +2764,7 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp stream_status_t res = inject_kernel_sequence(syscall_sequence_[marker_value], input); if (res == stream_status_t::STATUS_OK) { - VPRINT(this, 3, "Inserted %zu syscall records for syscall %u to %d.%d\n", + VPRINT(this, 3, "Inserted %zu syscall records for syscall %zu to %d.%d\n", syscall_sequence_[marker_value].size(), marker_value, input->workload, input->index); } else if (res != stream_status_t::STATUS_EOF) { From c71e4c50d2b64d9947271e2a335cd93c77bc8811 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 13 Feb 2025 21:25:25 -0500 Subject: [PATCH 04/19] Fix uninit error on Windows --- .../drcachesim/scheduler/scheduler_impl.cpp | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 5d3f9e5a5b..6aeaf1a3d5 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1466,17 +1466,19 @@ scheduler_impl_tmpl_t::read_kernel_sequences( return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } } - if (in_sequence) + if (in_sequence) { sequence[sequence_key].push_back(record); - if (record_type_is_marker(record, marker_type, marker_value) && - marker_type == end_marker) { - if (static_cast(marker_value) != sequence_key) { - error_string_ += sequence_type + " marker values mismatched"; - return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + if (record_type_is_marker(record, marker_type, marker_value) && + marker_type == end_marker) { + if (static_cast(marker_value) != sequence_key) { + error_string_ += sequence_type + " marker values mismatched"; + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + } + VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", + sequence[sequence_key].size(), sequence_type.c_str(), + sequence_key); + in_sequence = false; } - VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", - sequence[sequence_key].size(), sequence_type.c_str(), sequence_key); - in_sequence = false; } ++(*reader); } From 2e21613c0982b7425cb145a565f19bfc6261305e Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 13 Feb 2025 21:39:50 -0500 Subject: [PATCH 05/19] Fix Windows var declaration error --- .../drcachesim/tests/scheduler_unit_tests.cpp | 385 +++++++++--------- 1 file changed, 194 insertions(+), 191 deletions(-) diff --git a/clients/drcachesim/tests/scheduler_unit_tests.cpp b/clients/drcachesim/tests/scheduler_unit_tests.cpp index c8e3dd1ae3..cc1ddd46ab 100644 --- a/clients/drcachesim/tests/scheduler_unit_tests.cpp +++ b/clients/drcachesim/tests/scheduler_unit_tests.cpp @@ -6045,206 +6045,209 @@ test_kernel_syscall_sequences() static constexpr int SYSCALL_NUM_2 = 43; static constexpr addr_t SYSCALL_NUM_1_PC_START = 0xfeed101; static constexpr addr_t SYSCALL_NUM_2_PC_START = 0xcafe101; - std::vector syscall_sequence = { - /* clang-format off */ - make_header(TRACE_ENTRY_VERSION), - make_thread(TID_IN_SYSCALLS), - make_pid(TID_IN_SYSCALLS), - make_version(TRACE_ENTRY_VERSION), - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1), - make_instr(SYSCALL_NUM_1_PC_START), - make_instr(SYSCALL_NUM_1_PC_START + 1), - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1), - // XXX: Currently all syscall traces are concatenated. We may change - // this to use an archive file instead. - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_2), - make_instr(SYSCALL_NUM_2_PC_START), - make_instr(SYSCALL_NUM_2_PC_START + 1), - make_instr(SYSCALL_NUM_2_PC_START + 2), - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_2), - make_exit(TID_IN_SYSCALLS), - make_footer(), - /* clang-format on */ - }; - auto syscall_reader = - std::unique_ptr(new mock_reader_t(syscall_sequence)); - auto syscall_reader_end = std::unique_ptr(new mock_reader_t()); - static constexpr int NUM_INPUTS = 3; static constexpr int NUM_OUTPUTS = 2; - static constexpr int NUM_INSTRS = 9; - static constexpr int INSTR_QUANTUM = 3; - static constexpr uint64_t TIMESTAMP = 44226688; static constexpr memref_tid_t TID_BASE = 100; - std::vector sched_inputs; - std::vector readers; - for (int input_idx = 0; input_idx < NUM_INPUTS; input_idx++) { - std::vector inputs; - inputs.push_back(make_header(TRACE_ENTRY_VERSION)); - memref_tid_t tid = TID_BASE + input_idx; - inputs.push_back(make_thread(tid)); - inputs.push_back(make_pid(1)); - inputs.push_back(make_version(TRACE_ENTRY_VERSION)); - inputs.push_back(make_timestamp(TIMESTAMP)); - for (int instr_idx = 0; instr_idx < NUM_INSTRS; instr_idx++) { - inputs.push_back(make_instr(42 + instr_idx * 4)); - if (instr_idx % 2 == 0) { - inputs.push_back(make_marker(TRACE_MARKER_TYPE_SYSCALL, - (instr_idx / 2) % 2 == 0 ? SYSCALL_NUM_1 - : SYSCALL_NUM_2)); + { + std::vector syscall_sequence = { + /* clang-format off */ + make_header(TRACE_ENTRY_VERSION), + make_thread(TID_IN_SYSCALLS), + make_pid(TID_IN_SYSCALLS), + make_version(TRACE_ENTRY_VERSION), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1), + make_instr(SYSCALL_NUM_1_PC_START), + make_instr(SYSCALL_NUM_1_PC_START + 1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1), + // XXX: Currently all syscall traces are concatenated. We may change + // this to use an archive file instead. + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_2), + make_instr(SYSCALL_NUM_2_PC_START), + make_instr(SYSCALL_NUM_2_PC_START + 1), + make_instr(SYSCALL_NUM_2_PC_START + 2), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_2), + make_exit(TID_IN_SYSCALLS), + make_footer(), + /* clang-format on */ + }; + auto syscall_reader = + std::unique_ptr(new mock_reader_t(syscall_sequence)); + auto syscall_reader_end = std::unique_ptr(new mock_reader_t()); + static constexpr int NUM_INPUTS = 3; + static constexpr int NUM_INSTRS = 9; + static constexpr int INSTR_QUANTUM = 3; + static constexpr uint64_t TIMESTAMP = 44226688; + std::vector sched_inputs; + std::vector readers; + for (int input_idx = 0; input_idx < NUM_INPUTS; input_idx++) { + std::vector inputs; + inputs.push_back(make_header(TRACE_ENTRY_VERSION)); + memref_tid_t tid = TID_BASE + input_idx; + inputs.push_back(make_thread(tid)); + inputs.push_back(make_pid(1)); + inputs.push_back(make_version(TRACE_ENTRY_VERSION)); + inputs.push_back(make_timestamp(TIMESTAMP)); + for (int instr_idx = 0; instr_idx < NUM_INSTRS; instr_idx++) { + inputs.push_back(make_instr(42 + instr_idx * 4)); + if (instr_idx % 2 == 0) { + inputs.push_back(make_marker( + TRACE_MARKER_TYPE_SYSCALL, + (instr_idx / 2) % 2 == 0 ? SYSCALL_NUM_1 : SYSCALL_NUM_2)); + } } + inputs.push_back(make_exit(tid)); + readers.emplace_back( + std::unique_ptr(new mock_reader_t(inputs)), + std::unique_ptr(new mock_reader_t()), tid); } - inputs.push_back(make_exit(tid)); - readers.emplace_back(std::unique_ptr(new mock_reader_t(inputs)), - std::unique_ptr(new mock_reader_t()), tid); - } - 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); - sched_ops.quantum_duration_instrs = INSTR_QUANTUM; - sched_ops.kernel_syscall_reader = std::move(syscall_reader); - sched_ops.kernel_syscall_reader_end = std::move(syscall_reader_end); - scheduler_t scheduler; - if (scheduler.init(sched_inputs, NUM_OUTPUTS, std::move(sched_ops)) != - scheduler_t::STATUS_SUCCESS) - assert(false); + 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); + sched_ops.quantum_duration_instrs = INSTR_QUANTUM; + sched_ops.kernel_syscall_reader = std::move(syscall_reader); + sched_ops.kernel_syscall_reader_end = std::move(syscall_reader_end); + scheduler_t scheduler; + if (scheduler.init(sched_inputs, NUM_OUTPUTS, std::move(sched_ops)) != + scheduler_t::STATUS_SUCCESS) + assert(false); - // We have a custom version of run_lockstep_simulation here for more precise - // testing of the markers and instructions and interfaces. - // We record the entire sequence for a detailed check of some records, along with - // a character representation for a higher-level view of the whole sequence. - std::vector outputs(NUM_OUTPUTS, nullptr); - std::vector eof(NUM_OUTPUTS, false); - for (int i = 0; i < NUM_OUTPUTS; i++) - outputs[i] = scheduler.get_stream(i); - int num_eof = 0; - std::vector> refs(NUM_OUTPUTS); - std::vector sched_as_string(NUM_OUTPUTS); - std::vector prev_tid(NUM_OUTPUTS, INVALID_THREAD_ID); - std::vector in_syscall(NUM_OUTPUTS, false); - std::vector prev_in_ord(NUM_OUTPUTS, 0); - std::vector prev_out_ord(NUM_OUTPUTS, 0); - while (num_eof < NUM_OUTPUTS) { - for (int i = 0; i < NUM_OUTPUTS; i++) { - if (eof[i]) - continue; - memref_t memref; - scheduler_t::stream_status_t status = outputs[i]->next_record(memref); - if (status == scheduler_t::STATUS_EOF) { - ++num_eof; - eof[i] = true; - continue; - } - if (status == scheduler_t::STATUS_IDLE) { - sched_as_string[i] += '_'; - continue; - } - assert(status == scheduler_t::STATUS_OK); - refs[i].push_back(memref); - if (memref.instr.tid != prev_tid[i]) { - if (!sched_as_string[i].empty()) - sched_as_string[i] += ','; - sched_as_string[i] += - 'A' + static_cast(memref.instr.tid - TID_BASE); - } - if (memref.marker.type == TRACE_TYPE_MARKER && - memref.marker.marker_type == TRACE_MARKER_TYPE_SYSCALL_TRACE_START) - in_syscall[i] = true; - if (in_syscall[i]) { - // Test that syscall code is marked synthetic. - assert(outputs[i]->is_record_synthetic()); - // Test that dynamically injected syscall code doesn't count toward - // input ordinals, but does toward output ordinals. - assert(outputs[i]->get_input_interface()->get_record_ordinal() == - prev_in_ord[i]); - assert(outputs[i]->get_record_ordinal() > prev_out_ord[i]); - } else - assert(!outputs[i]->is_record_synthetic()); - if (type_is_instr(memref.instr.type)) - sched_as_string[i] += 'i'; - else if (memref.marker.type == TRACE_TYPE_MARKER) { - switch (memref.marker.marker_type) { - case TRACE_MARKER_TYPE_VERSION: sched_as_string[i] += 'v'; break; - case TRACE_MARKER_TYPE_TIMESTAMP: sched_as_string[i] += '0'; break; - case TRACE_MARKER_TYPE_SYSCALL: sched_as_string[i] += 's'; break; - case TRACE_MARKER_TYPE_SYSCALL_TRACE_END: - in_syscall[i] = false; - ANNOTATE_FALLTHROUGH; - case TRACE_MARKER_TYPE_SYSCALL_TRACE_START: - if (memref.marker.marker_value == SYSCALL_NUM_1) - sched_as_string[i] += '1'; - else if (memref.marker.marker_value == SYSCALL_NUM_2) - sched_as_string[i] += '2'; - else - assert(false && "unknown system call num"); - break; - default: sched_as_string[i] += '?'; break; + // We have a custom version of run_lockstep_simulation here for more precise + // testing of the markers and instructions and interfaces. + // We record the entire sequence for a detailed check of some records, along with + // a character representation for a higher-level view of the whole sequence. + std::vector outputs(NUM_OUTPUTS, nullptr); + std::vector eof(NUM_OUTPUTS, false); + for (int i = 0; i < NUM_OUTPUTS; i++) + outputs[i] = scheduler.get_stream(i); + int num_eof = 0; + std::vector> refs(NUM_OUTPUTS); + std::vector sched_as_string(NUM_OUTPUTS); + std::vector prev_tid(NUM_OUTPUTS, INVALID_THREAD_ID); + std::vector in_syscall(NUM_OUTPUTS, false); + std::vector prev_in_ord(NUM_OUTPUTS, 0); + std::vector prev_out_ord(NUM_OUTPUTS, 0); + while (num_eof < NUM_OUTPUTS) { + for (int i = 0; i < NUM_OUTPUTS; i++) { + if (eof[i]) + continue; + memref_t memref; + scheduler_t::stream_status_t status = outputs[i]->next_record(memref); + if (status == scheduler_t::STATUS_EOF) { + ++num_eof; + eof[i] = true; + continue; + } + if (status == scheduler_t::STATUS_IDLE) { + sched_as_string[i] += '_'; + continue; } + assert(status == scheduler_t::STATUS_OK); + refs[i].push_back(memref); + if (memref.instr.tid != prev_tid[i]) { + if (!sched_as_string[i].empty()) + sched_as_string[i] += ','; + sched_as_string[i] += + 'A' + static_cast(memref.instr.tid - TID_BASE); + } + if (memref.marker.type == TRACE_TYPE_MARKER && + memref.marker.marker_type == TRACE_MARKER_TYPE_SYSCALL_TRACE_START) + in_syscall[i] = true; + if (in_syscall[i]) { + // Test that syscall code is marked synthetic. + assert(outputs[i]->is_record_synthetic()); + // Test that dynamically injected syscall code doesn't count toward + // input ordinals, but does toward output ordinals. + assert(outputs[i]->get_input_interface()->get_record_ordinal() == + prev_in_ord[i]); + assert(outputs[i]->get_record_ordinal() > prev_out_ord[i]); + } else + assert(!outputs[i]->is_record_synthetic()); + if (type_is_instr(memref.instr.type)) + sched_as_string[i] += 'i'; + else if (memref.marker.type == TRACE_TYPE_MARKER) { + switch (memref.marker.marker_type) { + case TRACE_MARKER_TYPE_VERSION: sched_as_string[i] += 'v'; break; + case TRACE_MARKER_TYPE_TIMESTAMP: sched_as_string[i] += '0'; break; + case TRACE_MARKER_TYPE_SYSCALL: sched_as_string[i] += 's'; break; + case TRACE_MARKER_TYPE_SYSCALL_TRACE_END: + in_syscall[i] = false; + ANNOTATE_FALLTHROUGH; + case TRACE_MARKER_TYPE_SYSCALL_TRACE_START: + if (memref.marker.marker_value == SYSCALL_NUM_1) + sched_as_string[i] += '1'; + else if (memref.marker.marker_value == SYSCALL_NUM_2) + sched_as_string[i] += '2'; + else + assert(false && "unknown system call num"); + break; + default: sched_as_string[i] += '?'; break; + } + } + prev_tid[i] = memref.instr.tid; + prev_in_ord[i] = outputs[i]->get_input_interface()->get_record_ordinal(); + prev_out_ord[i] = outputs[i]->get_record_ordinal(); } - prev_tid[i] = memref.instr.tid; - prev_in_ord[i] = outputs[i]->get_input_interface()->get_record_ordinal(); - prev_out_ord[i] = outputs[i]->get_record_ordinal(); } + // Check the high-level strings. + for (int i = 0; i < NUM_OUTPUTS; i++) { + std::cerr << "cpu #" << i << " schedule: " << sched_as_string[i] << "\n"; + } + assert(sched_as_string[0] == + "Av0is1ii1,Cv0is1ii1,Aiis2iii2,Ciis2iii2,Aiis1ii1,Ciis1ii1,Aiis2iii2," + "Ciis2iii2,Aiis1ii1,Ciis1ii1"); + assert(sched_as_string[1] == + "Bv0is1ii1iis2iii2iis1ii1iis2iii2iis1ii1____________________________" + "___________"); + + // Zoom in and check the first few syscall sequences on the first output record + // by record with value checks. + int idx = 0; + bool res = true; + res = res && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_VERSION) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_TIMESTAMP, TIMESTAMP) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) && + // Syscall_1 trace on first thread. + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) && + + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_VERSION) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_TIMESTAMP, TIMESTAMP) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) && + // Syscall_1 trace on second thread. + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_2) && + // Syscall_2 trace on first thread. + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_2) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_2); + assert(res); } - // Check the high-level strings. - for (int i = 0; i < NUM_OUTPUTS; i++) { - std::cerr << "cpu #" << i << " schedule: " << sched_as_string[i] << "\n"; - } - assert(sched_as_string[0] == - "Av0is1ii1,Cv0is1ii1,Aiis2iii2,Ciis2iii2,Aiis1ii1,Ciis1ii1,Aiis2iii2," - "Ciis2iii2,Aiis1ii1,Ciis1ii1"); - assert(sched_as_string[1] == - "Bv0is1ii1iis2iii2iis1ii1iis2iii2iis1ii1____________________________" - "___________"); - - // Zoom in and check the first few syscall sequences on the first output record - // by record with value checks. - int idx = 0; - bool res = true; - res = res && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_TIMESTAMP, - TIMESTAMP) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL, - SYSCALL_NUM_1) && - // Syscall_1 trace on first thread. - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) && - - check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_VERSION) && - check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_TIMESTAMP, TIMESTAMP) && - check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) && - // Syscall_1 trace on second thread. - check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && - check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL, - SYSCALL_NUM_2) && - // Syscall_2 trace on first thread. - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_2) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && - check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_2); - assert(res); - { // Test a bad input sequence. std::vector bad_syscall_sequence = { From 562d4031e28f873e406a5a7febee7179d90548c4 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 13 Feb 2025 21:47:16 -0500 Subject: [PATCH 06/19] Fix Windows lossy conversion warning --- clients/drcachesim/scheduler/scheduler_impl.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 6aeaf1a3d5..24902be249 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -2762,12 +2762,14 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp uintptr_t marker_value = 0; if (record_type_is_marker(record, marker_type, marker_value) && marker_type == TRACE_MARKER_TYPE_SYSCALL && - syscall_sequence_.find(marker_value) != syscall_sequence_.end()) { + syscall_sequence_.find(static_cast(marker_value)) != + syscall_sequence_.end()) { + int syscall_num = static_cast(marker_value); stream_status_t res = - inject_kernel_sequence(syscall_sequence_[marker_value], input); + inject_kernel_sequence(syscall_sequence_[syscall_num], input); if (res == stream_status_t::STATUS_OK) { - VPRINT(this, 3, "Inserted %zu syscall records for syscall %zu to %d.%d\n", - syscall_sequence_[marker_value].size(), marker_value, input->workload, + VPRINT(this, 3, "Inserted %zu syscall records for syscall %d to %d.%d\n", + syscall_sequence_[syscall_num].size(), syscall_num, input->workload, input->index); } else if (res != stream_status_t::STATUS_EOF) { return res; From efe97745d2d37b09768aab3ca21645975e7728db Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 13 Feb 2025 23:24:45 -0500 Subject: [PATCH 07/19] Cleanup --- api/docs/release.dox | 6 ++--- clients/drcachesim/common/options.cpp | 14 +++++----- clients/drcachesim/scheduler/scheduler.h | 4 +-- .../drcachesim/scheduler/scheduler_impl.cpp | 27 ++++++++++--------- clients/drcachesim/scheduler/scheduler_impl.h | 3 ++- .../drcachesim/tests/scheduler_unit_tests.cpp | 5 ++++ 6 files changed, 34 insertions(+), 25 deletions(-) diff --git a/api/docs/release.dox b/api/docs/release.dox index 21a6c4c209..237203d88c 100644 --- a/api/docs/release.dox +++ b/api/docs/release.dox @@ -143,9 +143,9 @@ Further non-compatibility-affecting changes include: encodings are obtained from the application binaries instead. - Added a new drmemtrace analyzer flag -sched_syscall_file to allow specifying the system call trace template file to be used for dynamic injection of system call trace - templates. Added similar options for the drmemtrace scheduler - #dynamorio::drmemtrace::scheduler_tmpl_t::scheduler_options_t::kernel_syscall_trace_path, - and #dynamorio::drmemtrace::scheduler_tmpl_t::scheduler_options_t::kernel_syscall_reader. + templates. Added similar options for the drmemtrace scheduler: #dynamorio::drmemtrace:: + scheduler_tmpl_t::scheduler_options_t::kernel_syscall_trace_path, and #dynamorio:: + drmemtrace::scheduler_tmpl_t::scheduler_options_t::kernel_syscall_reader. **************************************************
diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index a48a831a42..4b6491c69a 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -1041,17 +1041,19 @@ droption_t op_sched_switch_file( DROPTION_SCOPE_FRONTEND, "sched_switch_file", "", "Path to file holding kernel context switch sequences", "Applies to -core_sharded and -core_serial. Path to file holding context switch " - "sequences. The file can contain multiple sequences each with regular trace headers " - "and the sequence proper bracketed by TRACE_MARKER_TYPE_CONTEXT_SWITCH_START and " - "TRACE_MARKER_TYPE_CONTEXT_SWITCH_END markers."); + "sequences. The file can contain multiple sequences each with regular trace " + "headers and the sequence proper bracketed by " + "TRACE_MARKER_TYPE_CONTEXT_SWITCH_START and TRACE_MARKER_TYPE_CONTEXT_SWITCH_END " + "markers."); droption_t op_sched_syscall_file( DROPTION_SCOPE_FRONTEND, "sched_syscall_file", "", "Path to file holding kernel system call sequences", "Applies to -core_sharded and -core_serial. Path to file holding system call " - "sequences. The file can contain multiple sequences each with regular trace headers " - "and the sequence proper bracketed by TRACE_MARKER_TYPE_SYSCALL_TRACE_START and " - "TRACE_MARKER_TYPE_SYSCALL_TRACE_END markers."); + "sequences. The file can contain multiple sequences each with regular trace " + "headers and the sequence proper bracketed by " + "TRACE_MARKER_TYPE_SYSCALL_TRACE_START and TRACE_MARKER_TYPE_SYSCALL_TRACE_END " + "markers."); droption_t op_sched_randomize( DROPTION_SCOPE_FRONTEND, "sched_randomize", false, diff --git a/clients/drcachesim/scheduler/scheduler.h b/clients/drcachesim/scheduler/scheduler.h index f541afad43..faf3359ec3 100644 --- a/clients/drcachesim/scheduler/scheduler.h +++ b/clients/drcachesim/scheduler/scheduler.h @@ -837,8 +837,8 @@ template class scheduler_tmpl_t { * indicating the system call it corresponds to. Sequences for * multiple system calls are concatenated into a single file. * Each sequence should be in the regular offline drmemtrace format. - * The sequence is inserted into the output stream on the - * #TRACE_MARKER_TYPE_SYSCALL marker with the indicated value. + * The sequence is inserted into the output stream after the + * #TRACE_MARKER_TYPE_SYSCALL markers with the indicated value. * The same file (or reader) must be passed when replaying as this kernel * code is not stored when recording. * An alternative to passing the file path is to pass #kernel_syscall_reader diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 24902be249..d9d4c9e479 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1457,8 +1457,8 @@ scheduler_impl_tmpl_t::read_kernel_sequences( // Only remember the records between the markers. trace_marker_type_t marker_type = TRACE_MARKER_TYPE_RESERVED_END; uintptr_t marker_value = 0; - if (record_type_is_marker(record, marker_type, marker_value) && - marker_type == start_marker) { + bool is_marker = record_type_is_marker(record, marker_type, marker_value); + if (is_marker && marker_type == start_marker) { sequence_key = static_cast(marker_value); in_sequence = true; if (!sequence[sequence_key].empty()) { @@ -1468,18 +1468,17 @@ scheduler_impl_tmpl_t::read_kernel_sequences( } if (in_sequence) { sequence[sequence_key].push_back(record); - if (record_type_is_marker(record, marker_type, marker_value) && - marker_type == end_marker) { - if (static_cast(marker_value) != sequence_key) { - error_string_ += sequence_type + " marker values mismatched"; - return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; - } - VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", - sequence[sequence_key].size(), sequence_type.c_str(), - sequence_key); - in_sequence = false; + } + if (is_marker && marker_type == end_marker) { + if (!in_sequence || static_cast(marker_value) != sequence_key) { + error_string_ += sequence_type + " marker values mismatched"; + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } + VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", + sequence[sequence_key].size(), sequence_type.c_str(), sequence_key); + in_sequence = false; } + ++(*reader); } return sched_type_t::STATUS_SUCCESS; @@ -2759,7 +2758,9 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp record_type_has_pid(record, input->pid); trace_marker_type_t marker_type; - uintptr_t marker_value = 0; + uintptr_t marker_value; + // Good to queue the injected records at this point, because we now surely will + // be done with TRACE_MARKER_TYPE_SYSCALL. if (record_type_is_marker(record, marker_type, marker_value) && marker_type == TRACE_MARKER_TYPE_SYSCALL && syscall_sequence_.find(static_cast(marker_value)) != diff --git a/clients/drcachesim/scheduler/scheduler_impl.h b/clients/drcachesim/scheduler/scheduler_impl.h index bf29d0e785..0d2cb6a703 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.h +++ b/clients/drcachesim/scheduler/scheduler_impl.h @@ -546,6 +546,7 @@ template class scheduler_impl_tmpl_t uint64_t timestamp; }; + // Custom hash function used for switch_type_t and syscall num (int). template struct custom_hash_t { std::size_t operator()(const IntCastable &st) const @@ -1007,7 +1008,7 @@ template class scheduler_impl_tmpl_t std::unordered_map, custom_hash_t> switch_sequence_; - // We specify a custom hash function only to make it generalize with + // We specify a custom hash function only to make it easier to generalize with // switch_sequence_ defined above. std::unordered_map, custom_hash_t> syscall_sequence_; diff --git a/clients/drcachesim/tests/scheduler_unit_tests.cpp b/clients/drcachesim/tests/scheduler_unit_tests.cpp index cc1ddd46ab..cbe5c9bcba 100644 --- a/clients/drcachesim/tests/scheduler_unit_tests.cpp +++ b/clients/drcachesim/tests/scheduler_unit_tests.cpp @@ -6193,6 +6193,8 @@ test_kernel_syscall_sequences() for (int i = 0; i < NUM_OUTPUTS; i++) { std::cerr << "cpu #" << i << " schedule: " << sched_as_string[i] << "\n"; } + // The instrs in the injected syscall sequence count towards the #instr + // quantum, but no context switch happens in the middle of the syscall seq. assert(sched_as_string[0] == "Av0is1ii1,Cv0is1ii1,Aiis2iii2,Ciis2iii2,Aiis1ii1,Ciis1ii1,Aiis2iii2," "Ciis2iii2,Aiis1ii1,Ciis1ii1"); @@ -6212,6 +6214,7 @@ test_kernel_syscall_sequences() check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) && + // Syscall_1 trace on first thread. check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && @@ -6227,6 +6230,7 @@ test_kernel_syscall_sequences() check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) && + // Syscall_1 trace on second thread. check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && @@ -6234,6 +6238,7 @@ test_kernel_syscall_sequences() check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) && + check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, From b8b27e2150b9341fe1f550f7415e7b4637f82f5b Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 13 Feb 2025 23:43:25 -0500 Subject: [PATCH 08/19] Fix Windows uninit warning --- clients/drcachesim/scheduler/scheduler_impl.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index d9d4c9e479..dedb46ba2f 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1470,7 +1470,11 @@ scheduler_impl_tmpl_t::read_kernel_sequences( sequence[sequence_key].push_back(record); } if (is_marker && marker_type == end_marker) { - if (!in_sequence || static_cast(marker_value) != sequence_key) { + if (!in_sequence) { + error_string_ += sequence_type + " end marker found without start"; + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + } + if (static_cast(marker_value) != sequence_key) { error_string_ += sequence_type + " marker values mismatched"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } From 5ace1271e0bc3c06e01ee9e7ace013edf73ba089 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Thu, 13 Feb 2025 23:59:28 -0500 Subject: [PATCH 09/19] Fix Windows uninit warning --- clients/drcachesim/scheduler/scheduler_impl.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index dedb46ba2f..97fbc9274b 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1470,13 +1470,12 @@ scheduler_impl_tmpl_t::read_kernel_sequences( sequence[sequence_key].push_back(record); } if (is_marker && marker_type == end_marker) { - if (!in_sequence) { - error_string_ += sequence_type + " end marker found without start"; - return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; - } - if (static_cast(marker_value) != sequence_key) { + if (in_sequence && static_cast(marker_value) != sequence_key) { error_string_ += sequence_type + " marker values mismatched"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + } else if (!in_sequence) { + error_string_ += sequence_type + " end marker found without start"; + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", sequence[sequence_key].size(), sequence_type.c_str(), sequence_key); From a97ee6bfe8fe58c2c74a2b9833480222c8f3d3e5 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 14 Feb 2025 00:18:26 -0500 Subject: [PATCH 10/19] Fix Windows uninit warning --- clients/drcachesim/scheduler/scheduler_impl.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 97fbc9274b..29dc00a616 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1470,15 +1470,19 @@ scheduler_impl_tmpl_t::read_kernel_sequences( sequence[sequence_key].push_back(record); } if (is_marker && marker_type == end_marker) { - if (in_sequence && static_cast(marker_value) != sequence_key) { - error_string_ += sequence_type + " marker values mismatched"; - return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; - } else if (!in_sequence) { + if (in_sequence) { + if (static_cast(marker_value) != sequence_key) { + error_string_ += sequence_type + " marker values mismatched"; + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + } else { + VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", + sequence[sequence_key].size(), sequence_type.c_str(), + sequence_key); + } + } else { error_string_ += sequence_type + " end marker found without start"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } - VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", - sequence[sequence_key].size(), sequence_type.c_str(), sequence_key); in_sequence = false; } From 06dec36effa72939b4b793ea1ce380718dd30b99 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 14 Feb 2025 01:33:33 -0500 Subject: [PATCH 11/19] Add more tests for syscall trace injection --- clients/drcachesim/analyzer.cpp | 8 ++++++++ clients/drcachesim/analyzer_multi.cpp | 3 ++- clients/drcachesim/common/options.cpp | 5 ++--- .../drcachesim/tests/burst_syscall_inject.cpp | 4 ++++ .../tests/mock_syscall_sequences.x64 | Bin 0 -> 228 bytes .../drcachesim/tests/scheduler_unit_tests.cpp | 1 + .../tests/syscall_file_invariants.templatex | 1 + .../tests/syscall_insertion.templatex | 8 ++++++++ .../syscall_insertion_core_sharded.templatex | 8 ++++++++ suite/tests/CMakeLists.txt | 18 ++++++++++++++++++ 10 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 clients/drcachesim/tests/mock_syscall_sequences.x64 create mode 100644 clients/drcachesim/tests/syscall_file_invariants.templatex create mode 100644 clients/drcachesim/tests/syscall_insertion.templatex create mode 100644 clients/drcachesim/tests/syscall_insertion_core_sharded.templatex diff --git a/clients/drcachesim/analyzer.cpp b/clients/drcachesim/analyzer.cpp index 2583422ac1..41eca3b5ac 100644 --- a/clients/drcachesim/analyzer.cpp +++ b/clients/drcachesim/analyzer.cpp @@ -324,6 +324,10 @@ analyzer_tmpl_t::init_scheduler_common( sched_ops = sched_type_t::make_scheduler_parallel_options(verbosity_); sched_ops.replay_as_traced_istream = options.replay_as_traced_istream; sched_ops.read_inputs_in_init = options.read_inputs_in_init; + sched_ops.kernel_syscall_trace_path = options.kernel_syscall_trace_path; + sched_ops.kernel_syscall_reader = std::move(options.kernel_syscall_reader); + sched_ops.kernel_syscall_reader_end = + std::move(options.kernel_syscall_reader_end); if (worker_count_ <= 0) worker_count_ = std::thread::hardware_concurrency(); output_count = worker_count_; @@ -331,6 +335,10 @@ analyzer_tmpl_t::init_scheduler_common( sched_ops = sched_type_t::make_scheduler_serial_options(verbosity_); sched_ops.replay_as_traced_istream = options.replay_as_traced_istream; sched_ops.read_inputs_in_init = options.read_inputs_in_init; + sched_ops.kernel_syscall_trace_path = options.kernel_syscall_trace_path; + sched_ops.kernel_syscall_reader = std::move(options.kernel_syscall_reader); + sched_ops.kernel_syscall_reader_end = + std::move(options.kernel_syscall_reader_end); worker_count_ = 1; output_count = 1; } diff --git a/clients/drcachesim/analyzer_multi.cpp b/clients/drcachesim/analyzer_multi.cpp index be1d698d84..ad77dad5f1 100644 --- a/clients/drcachesim/analyzer_multi.cpp +++ b/clients/drcachesim/analyzer_multi.cpp @@ -571,6 +571,8 @@ analyzer_multi_tmpl_t::analyzer_multi_tmpl_t() #endif } + sched_ops.kernel_syscall_trace_path = op_sched_syscall_file.get_value(); + if (!indirs.empty()) { std::vector tracedirs; for (const std::string &indir : indirs) @@ -689,7 +691,6 @@ analyzer_multi_tmpl_t::init_dynamic_schedule() } #endif sched_ops.kernel_switch_trace_path = op_sched_switch_file.get_value(); - sched_ops.kernel_syscall_trace_path = op_sched_syscall_file.get_value(); return sched_ops; } diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index 4b6491c69a..74e3936511 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -1049,9 +1049,8 @@ droption_t op_sched_switch_file( droption_t op_sched_syscall_file( DROPTION_SCOPE_FRONTEND, "sched_syscall_file", "", "Path to file holding kernel system call sequences", - "Applies to -core_sharded and -core_serial. Path to file holding system call " - "sequences. The file can contain multiple sequences each with regular trace " - "headers and the sequence proper bracketed by " + "Path to file holding system call sequences. The file can contain multiple " + "sequences each with regular trace headers and the sequence proper bracketed by " "TRACE_MARKER_TYPE_SYSCALL_TRACE_START and TRACE_MARKER_TYPE_SYSCALL_TRACE_END " "markers."); diff --git a/clients/drcachesim/tests/burst_syscall_inject.cpp b/clients/drcachesim/tests/burst_syscall_inject.cpp index aef4590ec9..b5898a5000 100644 --- a/clients/drcachesim/tests/burst_syscall_inject.cpp +++ b/clients/drcachesim/tests/burst_syscall_inject.cpp @@ -143,6 +143,10 @@ write_header_entries(std::unique_ptr &writer) } write_trace_entry(writer, make_marker(TRACE_MARKER_TYPE_CACHE_LINE_SIZE, 64)); write_trace_entry(writer, make_marker(TRACE_MARKER_TYPE_PAGE_SIZE, 4096)); + // Some header read-ahead logic uses the timestamp marker to know when + // to stop. It is important to not read-ahead any kernel syscall trace + // content, as then is_record_kernel() starts returning true on the stream. + write_trace_entry(writer, make_marker(TRACE_MARKER_TYPE_TIMESTAMP, 0)); } static void diff --git a/clients/drcachesim/tests/mock_syscall_sequences.x64 b/clients/drcachesim/tests/mock_syscall_sequences.x64 new file mode 100644 index 0000000000000000000000000000000000000000..6cc43cde700a99ad5715c51a18bb6c8cd34f99a1 GIT binary patch literal 228 zcmb1SU|?VeVi1sF;6Y+@GB`Lv#Kjm`7#JZ;1_>m#3!}gYxu&rc8jcxj-iFTYC@0l>tJS8jyN^xO!#=k8Ys3K)}Vo%)szrZ8t=UgJJFO ZQ*ga7_2NKDm^-C_>;;`+od4??7yxdN5BUH9 literal 0 HcmV?d00001 diff --git a/clients/drcachesim/tests/scheduler_unit_tests.cpp b/clients/drcachesim/tests/scheduler_unit_tests.cpp index cbe5c9bcba..72c829dd6c 100644 --- a/clients/drcachesim/tests/scheduler_unit_tests.cpp +++ b/clients/drcachesim/tests/scheduler_unit_tests.cpp @@ -6054,6 +6054,7 @@ test_kernel_syscall_sequences() make_thread(TID_IN_SYSCALLS), make_pid(TID_IN_SYSCALLS), make_version(TRACE_ENTRY_VERSION), + make_timestamp(0), make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1), make_instr(SYSCALL_NUM_1_PC_START), make_instr(SYSCALL_NUM_1_PC_START + 1), diff --git a/clients/drcachesim/tests/syscall_file_invariants.templatex b/clients/drcachesim/tests/syscall_file_invariants.templatex new file mode 100644 index 0000000000..30eaadb34c --- /dev/null +++ b/clients/drcachesim/tests/syscall_file_invariants.templatex @@ -0,0 +1 @@ +Trace invariant checks passed diff --git a/clients/drcachesim/tests/syscall_insertion.templatex b/clients/drcachesim/tests/syscall_insertion.templatex new file mode 100644 index 0000000000..91f6dc0466 --- /dev/null +++ b/clients/drcachesim/tests/syscall_insertion.templatex @@ -0,0 +1,8 @@ +Basic counts tool results: +Total counts: + [1-9][0-9][0-9][0-9][0-9][0-9] total \(fetched\) instructions + 5971 total unique \(fetched\) instructions + [1-9][0-9][0-9][0-9][0-9][0-9] total userspace instructions + [1-9][0-9][0-9] total kernel instructions + [1-9][0-9][0-9][0-9][0-9][0-9] total non-fetched instructions +.* diff --git a/clients/drcachesim/tests/syscall_insertion_core_sharded.templatex b/clients/drcachesim/tests/syscall_insertion_core_sharded.templatex new file mode 100644 index 0000000000..91f6dc0466 --- /dev/null +++ b/clients/drcachesim/tests/syscall_insertion_core_sharded.templatex @@ -0,0 +1,8 @@ +Basic counts tool results: +Total counts: + [1-9][0-9][0-9][0-9][0-9][0-9] total \(fetched\) instructions + 5971 total unique \(fetched\) instructions + [1-9][0-9][0-9][0-9][0-9][0-9] total userspace instructions + [1-9][0-9][0-9] total kernel instructions + [1-9][0-9][0-9][0-9][0-9][0-9] total non-fetched instructions +.* diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index aa1c39b6d2..52f6ddbc9b 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -4076,6 +4076,24 @@ if (BUILD_CLIENTS) "") set(tool.switch_file_invariants_rawtemp ON) # no preprocessor + set(syscall_file + "${PROJECT_SOURCE_DIR}/clients/drcachesim/tests/mock_syscall_sequences.x64") + + torunonly_simtool(syscall_insertion ${ci_shared_app} + "-indir ${thread_trace_dir} -tool basic_counts -sched_syscall_file ${syscall_file}" + "") + set(tool.syscall_insertion_rawtemp ON) # no preprocessor + + torunonly_simtool(syscall_insertion_core_sharded ${ci_shared_app} + "-indir ${thread_trace_dir} -tool basic_counts -core_sharded -sched_quantum 1000 -sched_syscall_file ${syscall_file}" + "") + set(tool.syscall_insertion_core_sharded_rawtemp ON) # no preprocessor + + torunonly_simtool(syscall_file_invariants ${ci_shared_app} + "-infile ${syscall_file} -tool invariant_checker" + "") + set(tool.syscall_file_invariants_rawtemp ON) # no preprocessor + # Test -multi_indir with 3 copies of our sample dir. torunonly_simtool(multi_indir ${ci_shared_app} "-multi_indir ${thread_trace_dir}:${thread_trace_dir}:${thread_trace_dir} -tool schedule_stats -cores 3" From 166c8169e4a2d9b6e2b49fb23e9adf64c6a1f94c Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 14 Feb 2025 01:40:19 -0500 Subject: [PATCH 12/19] Fix Windows uninit warning --- .../drcachesim/scheduler/scheduler_impl.cpp | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 29dc00a616..3ff7beb328 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1467,25 +1467,23 @@ scheduler_impl_tmpl_t::read_kernel_sequences( } } if (in_sequence) { + // We avoid uninitialized-use warnings by placing sequence_key uses + // inside this if-block (otherwise we'd need to add separate template + // specialized functions to get the default uninitialized value based + // on SequenceKey). Seems that the Windows compiler is able to + // determine this as okay. sequence[sequence_key].push_back(record); - } - if (is_marker && marker_type == end_marker) { - if (in_sequence) { + if (is_marker && marker_type == end_marker) { if (static_cast(marker_value) != sequence_key) { error_string_ += sequence_type + " marker values mismatched"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; - } else { - VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", - sequence[sequence_key].size(), sequence_type.c_str(), - sequence_key); } - } else { - error_string_ += sequence_type + " end marker found without start"; - return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", + sequence[sequence_key].size(), sequence_type.c_str(), + sequence_key); } in_sequence = false; } - ++(*reader); } return sched_type_t::STATUS_SUCCESS; From 94fea861b95c57f2c85a3f699c22f43bf4f6b6a8 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 14 Feb 2025 01:45:24 -0500 Subject: [PATCH 13/19] Add in_sequence check --- clients/drcachesim/scheduler/scheduler_impl.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 3ff7beb328..97d3d9be32 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1478,14 +1478,15 @@ scheduler_impl_tmpl_t::read_kernel_sequences( error_string_ += sequence_type + " marker values mismatched"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } + in_sequence = false; VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", sequence[sequence_key].size(), sequence_type.c_str(), sequence_key); } - in_sequence = false; } ++(*reader); } + assert(!in_sequence); return sched_type_t::STATUS_SUCCESS; } From 7344de455fd6fbd7f4a751fc4946681f7741de1c Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Fri, 14 Feb 2025 01:54:57 -0500 Subject: [PATCH 14/19] Fix comment --- clients/drcachesim/scheduler/scheduler_impl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 97d3d9be32..e4c9dd8646 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -1469,7 +1469,7 @@ scheduler_impl_tmpl_t::read_kernel_sequences( if (in_sequence) { // We avoid uninitialized-use warnings by placing sequence_key uses // inside this if-block (otherwise we'd need to add separate template - // specialized functions to get the default uninitialized value based + // specialized functions to get the default-initialized value based // on SequenceKey). Seems that the Windows compiler is able to // determine this as okay. sequence[sequence_key].push_back(record); From 4377c39bf39c53203983583a649eff274037b289 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 16 Feb 2025 13:20:02 -0500 Subject: [PATCH 15/19] Reviewer suggested edits --- clients/drcachesim/analyzer.cpp | 29 +- clients/drcachesim/analyzer.h | 11 +- clients/drcachesim/common/options.cpp | 7 +- clients/drcachesim/scheduler/scheduler.h | 4 +- .../drcachesim/scheduler/scheduler_impl.cpp | 85 ++++- clients/drcachesim/scheduler/scheduler_impl.h | 15 +- .../drcachesim/tests/scheduler_unit_tests.cpp | 336 ++++++++---------- .../tests/switch_insertion.templatex | 4 +- .../tests/syscall_insertion.templatex | 4 +- .../syscall_insertion_core_sharded.templatex | 4 +- 10 files changed, 255 insertions(+), 244 deletions(-) diff --git a/clients/drcachesim/analyzer.cpp b/clients/drcachesim/analyzer.cpp index 41eca3b5ac..b6cf14a8c8 100644 --- a/clients/drcachesim/analyzer.cpp +++ b/clients/drcachesim/analyzer.cpp @@ -320,27 +320,22 @@ analyzer_tmpl_t::init_scheduler_common( sched_ops.single_lockstep_output = true; worker_count_ = 1; } - } else if (parallel_) { - sched_ops = sched_type_t::make_scheduler_parallel_options(verbosity_); - sched_ops.replay_as_traced_istream = options.replay_as_traced_istream; - sched_ops.read_inputs_in_init = options.read_inputs_in_init; - sched_ops.kernel_syscall_trace_path = options.kernel_syscall_trace_path; - sched_ops.kernel_syscall_reader = std::move(options.kernel_syscall_reader); - sched_ops.kernel_syscall_reader_end = - std::move(options.kernel_syscall_reader_end); - if (worker_count_ <= 0) - worker_count_ = std::thread::hardware_concurrency(); - output_count = worker_count_; } else { - sched_ops = sched_type_t::make_scheduler_serial_options(verbosity_); + if (parallel_) { + sched_ops = sched_type_t::make_scheduler_parallel_options(verbosity_); + if (worker_count_ <= 0) + worker_count_ = std::thread::hardware_concurrency(); + output_count = worker_count_; + } else { + sched_ops = sched_type_t::make_scheduler_serial_options(verbosity_); + worker_count_ = 1; + output_count = 1; + } + // As noted in the init_scheduler_common() header comment, we preserve only + // some select fields. sched_ops.replay_as_traced_istream = options.replay_as_traced_istream; sched_ops.read_inputs_in_init = options.read_inputs_in_init; sched_ops.kernel_syscall_trace_path = options.kernel_syscall_trace_path; - sched_ops.kernel_syscall_reader = std::move(options.kernel_syscall_reader); - sched_ops.kernel_syscall_reader_end = - std::move(options.kernel_syscall_reader_end); - worker_count_ = 1; - output_count = 1; } sched_mapping_ = options.mapping; if (scheduler_.init(workloads, output_count, std::move(sched_ops)) != diff --git a/clients/drcachesim/analyzer.h b/clients/drcachesim/analyzer.h index 2e6c92c048..c3440d39eb 100644 --- a/clients/drcachesim/analyzer.h +++ b/clients/drcachesim/analyzer.h @@ -217,6 +217,7 @@ template class analyzer_tmpl_t { operator=(const analyzer_worker_data_t &) = delete; }; + // See comment on init_scheduler_common() for some noteworthy details. bool init_scheduler(const std::vector &trace_paths, // To include all threads/shards, use empty sets. @@ -224,15 +225,17 @@ template class analyzer_tmpl_t { const std::set &only_shards, int output_limit, int verbosity, typename sched_type_t::scheduler_options_t options); - // For core-sharded, worker_count_ must be set prior to calling this; for parallel - // mode if it is not set it will be set to the underlying core count. - // For core-sharded, all of "options" is used; otherwise, only the - // read_inputs_in_init field is preserved. + // See comment on init_scheduler_common() for some noteworthy details. bool init_scheduler(std::unique_ptr reader, std::unique_ptr reader_end, int verbosity, typename sched_type_t::scheduler_options_t options); + // For core-sharded, worker_count_ must be set prior to calling this; for parallel + // mode if it is not set it will be set to the underlying core count. + // For core-sharded, all of "options" is used; otherwise, the + // read_inputs_in_init, replay_as_traced_istream, and kernel_syscall_trace_path + // fields are preserved. bool init_scheduler_common(std::vector &workloads, typename sched_type_t::scheduler_options_t options); diff --git a/clients/drcachesim/common/options.cpp b/clients/drcachesim/common/options.cpp index 74e3936511..945bb161da 100644 --- a/clients/drcachesim/common/options.cpp +++ b/clients/drcachesim/common/options.cpp @@ -1041,10 +1041,9 @@ droption_t op_sched_switch_file( DROPTION_SCOPE_FRONTEND, "sched_switch_file", "", "Path to file holding kernel context switch sequences", "Applies to -core_sharded and -core_serial. Path to file holding context switch " - "sequences. The file can contain multiple sequences each with regular trace " - "headers and the sequence proper bracketed by " - "TRACE_MARKER_TYPE_CONTEXT_SWITCH_START and TRACE_MARKER_TYPE_CONTEXT_SWITCH_END " - "markers."); + "sequences. The file can contain multiple sequences each with regular trace headers " + "and the sequence proper bracketed by TRACE_MARKER_TYPE_CONTEXT_SWITCH_START and " + "TRACE_MARKER_TYPE_CONTEXT_SWITCH_END markers."); droption_t op_sched_syscall_file( DROPTION_SCOPE_FRONTEND, "sched_syscall_file", "", diff --git a/clients/drcachesim/scheduler/scheduler.h b/clients/drcachesim/scheduler/scheduler.h index faf3359ec3..1e4578cb22 100644 --- a/clients/drcachesim/scheduler/scheduler.h +++ b/clients/drcachesim/scheduler/scheduler.h @@ -837,8 +837,8 @@ template class scheduler_tmpl_t { * indicating the system call it corresponds to. Sequences for * multiple system calls are concatenated into a single file. * Each sequence should be in the regular offline drmemtrace format. - * The sequence is inserted into the output stream after the - * #TRACE_MARKER_TYPE_SYSCALL markers with the indicated value. + * Each sequence is inserted into the output stream after all + * #TRACE_MARKER_TYPE_SYSCALL markers with the same value. * The same file (or reader) must be passed when replaying as this kernel * code is not stored when recording. * An alternative to passing the file path is to pass #kernel_syscall_reader diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index e4c9dd8646..fbac22b36a 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -348,6 +348,23 @@ scheduler_impl_tmpl_t::insert_switch_tid_pid(input_info_t &i // We do nothing, as every record has a tid from the separate inputs. } +template <> +template <> +typename scheduler_tmpl_t::switch_type_t +scheduler_impl_tmpl_t::default_kernel_sequence_key() +{ + return switch_type_t::SWITCH_INVALID; +} + +template <> +template <> +int +scheduler_impl_tmpl_t::default_kernel_sequence_key() +{ + // System numbers are small non-negative integers. + return -1; +} + /****************************************************************************** * Specializations for scheduler_impl_tmpl_t, aka * record_scheduler_impl_t. @@ -568,6 +585,23 @@ scheduler_impl_tmpl_t::insert_switch_tid_pid( input.queue.push_front(tid); } +template <> +template <> +typename scheduler_tmpl_t::switch_type_t +scheduler_impl_tmpl_t::default_kernel_sequence_key() +{ + return switch_type_t::SWITCH_INVALID; +} + +template <> +template <> +int +scheduler_impl_tmpl_t::default_kernel_sequence_key() +{ + // System numbers are small non-negative integers. + return -1; +} + /*************************************************************************** * Scheduler. */ @@ -1433,7 +1467,7 @@ scheduler_impl_tmpl_t::read_kernel_sequences( } reader_end = get_default_reader(); } else if (!reader) { - // No switch data provided. + // No kernel data provided. return sched_type_t::STATUS_SUCCESS; } else { if (!reader_end) { @@ -1450,7 +1484,8 @@ scheduler_impl_tmpl_t::read_kernel_sequences( // memory and don't need to stream them on every use. // We read a single stream, even if underneath these are split into subfiles // in an archive. - SequenceKey sequence_key; + SequenceKey sequence_key = default_kernel_sequence_key(); + const SequenceKey DEFAULT_SEQ_KEY = default_kernel_sequence_key(); bool in_sequence = false; while (*reader != *reader_end) { RecordType record = **reader; @@ -1459,30 +1494,39 @@ scheduler_impl_tmpl_t::read_kernel_sequences( uintptr_t marker_value = 0; bool is_marker = record_type_is_marker(record, marker_type, marker_value); if (is_marker && marker_type == start_marker) { + if (in_sequence) { + error_string_ += "Found another " + sequence_type + + " sequence start without prior ending"; + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + } sequence_key = static_cast(marker_value); in_sequence = true; + if (sequence_key == DEFAULT_SEQ_KEY) { + error_string_ += + "Invalid " + sequence_type + " sequence found with default key"; + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + } if (!sequence[sequence_key].empty()) { error_string_ += "Duplicate " + sequence_type + " sequence found"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } } - if (in_sequence) { - // We avoid uninitialized-use warnings by placing sequence_key uses - // inside this if-block (otherwise we'd need to add separate template - // specialized functions to get the default-initialized value based - // on SequenceKey). Seems that the Windows compiler is able to - // determine this as okay. + if (in_sequence) sequence[sequence_key].push_back(record); - if (is_marker && marker_type == end_marker) { - if (static_cast(marker_value) != sequence_key) { - error_string_ += sequence_type + " marker values mismatched"; - return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; - } - in_sequence = false; - VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", - sequence[sequence_key].size(), sequence_type.c_str(), - sequence_key); + if (is_marker && marker_type == end_marker) { + if (!in_sequence) { + error_string_ += "Found " + sequence_type + + " sequence end marker without start marker"; + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; + } + if (static_cast(marker_value) != sequence_key) { + error_string_ += sequence_type + " marker values mismatched"; + return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } + sequence_key = DEFAULT_SEQ_KEY; + in_sequence = false; + VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", + sequence[sequence_key].size(), sequence_type.c_str(), sequence_key); } ++(*reader); } @@ -2762,7 +2806,14 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp outputs_[output].last_record = record; record_type_has_tid(record, input->last_record_tid); record_type_has_pid(record, input->pid); + return finalize_next_record(record, input); +} +template +typename scheduler_tmpl_t::stream_status_t +scheduler_impl_tmpl_t::finalize_next_record( + const RecordType &record, input_info_t *input) +{ trace_marker_type_t marker_type; uintptr_t marker_value; // Good to queue the injected records at this point, because we now surely will diff --git a/clients/drcachesim/scheduler/scheduler_impl.h b/clients/drcachesim/scheduler/scheduler_impl.h index 0d2cb6a703..d3a4c9a8ef 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.h +++ b/clients/drcachesim/scheduler/scheduler_impl.h @@ -749,6 +749,10 @@ template class scheduler_impl_tmpl_t std::vector> &start2stop, std::vector> &all_sched); + template + SequenceKey + default_kernel_sequence_key(); + template scheduler_status_t read_kernel_sequences(std::unordered_map, @@ -764,9 +768,6 @@ template class scheduler_impl_tmpl_t scheduler_status_t read_syscall_sequences(); - stream_status_t - inject_kernel_sequence(std::vector &sequence, input_info_t *input); - uint64_t get_time_micros(); @@ -870,6 +871,14 @@ template class scheduler_impl_tmpl_t void update_next_record(output_ordinal_t output, RecordType &record); + stream_status_t + inject_kernel_sequence(std::vector &sequence, input_info_t *input); + + // Actions that must be taken only when we know for sure that the given record + // is going to be the next record for some output stream. + stream_status_t + finalize_next_record(const RecordType &record, input_info_t *input); + // Used for diagnostics: prints record fields to stderr. void print_record(const RecordType &record); diff --git a/clients/drcachesim/tests/scheduler_unit_tests.cpp b/clients/drcachesim/tests/scheduler_unit_tests.cpp index bd733ac3a0..2b3223ba31 100644 --- a/clients/drcachesim/tests/scheduler_unit_tests.cpp +++ b/clients/drcachesim/tests/scheduler_unit_tests.cpp @@ -5769,6 +5769,115 @@ test_unscheduled() test_unscheduled_no_alternative(); } +static std::vector +run_lockstep_simulation_for_kernel_seq(scheduler_t &scheduler, int num_outputs, + memref_tid_t tid_base, int syscall_base, + std::vector> &refs) +{ + // We have a custom version of run_lockstep_simulation here for more precise + // testing of the markers and instructions and interfaces. + // We record the entire sequence for a detailed check of some records, along with + // a character representation for a higher-level view of the whole sequence. + std::vector outputs(num_outputs, nullptr); + std::vector eof(num_outputs, false); + for (int i = 0; i < num_outputs; i++) + outputs[i] = scheduler.get_stream(i); + int num_eof = 0; + refs.resize(num_outputs); + std::vector sched_as_string(num_outputs); + std::vector prev_tid(num_outputs, INVALID_THREAD_ID); + std::vector in_switch(num_outputs, false); + std::vector in_syscall(num_outputs, false); + std::vector prev_in_ord(num_outputs, 0); + std::vector prev_out_ord(num_outputs, 0); + while (num_eof < num_outputs) { + for (int i = 0; i < num_outputs; i++) { + if (eof[i]) + continue; + memref_t memref; + scheduler_t::stream_status_t status = outputs[i]->next_record(memref); + if (status == scheduler_t::STATUS_EOF) { + ++num_eof; + eof[i] = true; + continue; + } + if (status == scheduler_t::STATUS_IDLE) { + sched_as_string[i] += '_'; + continue; + } + assert(status == scheduler_t::STATUS_OK); + refs[i].push_back(memref); + if (memref.instr.tid != prev_tid[i]) { + if (!sched_as_string[i].empty()) + sched_as_string[i] += ','; + sched_as_string[i] += + 'A' + static_cast(memref.instr.tid - tid_base); + } + if (memref.marker.type == TRACE_TYPE_MARKER) { + if (memref.marker.marker_type == TRACE_MARKER_TYPE_CONTEXT_SWITCH_START) + in_switch[i] = true; + else if (memref.marker.marker_type == + TRACE_MARKER_TYPE_SYSCALL_TRACE_START) + in_syscall[i] = true; + } + if (in_switch[i]) { + // Test that switch code is marked synthetic. + assert(outputs[i]->is_record_synthetic()); + // Test that switch code doesn't count toward input ordinals, but + // does toward output ordinals. + assert(outputs[i]->get_input_interface()->get_record_ordinal() == + prev_in_ord[i] || + // Won't match if we just switched inputs. + (memref.marker.type == TRACE_TYPE_MARKER && + memref.marker.marker_type == + TRACE_MARKER_TYPE_CONTEXT_SWITCH_START)); + assert(outputs[i]->get_record_ordinal() > prev_out_ord[i]); + } else if (in_syscall[i]) { + // Test that syscall code is marked synthetic. + assert(outputs[i]->is_record_synthetic()); + // Test that dynamically injected syscall code doesn't count toward + // input ordinals, but does toward output ordinals. + assert(outputs[i]->get_input_interface()->get_record_ordinal() == + prev_in_ord[i]); + assert(outputs[i]->get_record_ordinal() > prev_out_ord[i]); + } else + assert(!outputs[i]->is_record_synthetic()); + if (type_is_instr(memref.instr.type)) + sched_as_string[i] += 'i'; + else if (memref.marker.type == TRACE_TYPE_MARKER) { + switch (memref.marker.marker_type) { + case TRACE_MARKER_TYPE_VERSION: sched_as_string[i] += 'v'; break; + case TRACE_MARKER_TYPE_TIMESTAMP: sched_as_string[i] += '0'; break; + case TRACE_MARKER_TYPE_CONTEXT_SWITCH_END: + in_switch[i] = false; + ANNOTATE_FALLTHROUGH; + case TRACE_MARKER_TYPE_CONTEXT_SWITCH_START: + if (memref.marker.marker_value == scheduler_t::SWITCH_PROCESS) + sched_as_string[i] += 'p'; + else if (memref.marker.marker_value == scheduler_t::SWITCH_THREAD) + sched_as_string[i] += 't'; + else + assert(false && "unknown context switch type"); + break; + case TRACE_MARKER_TYPE_SYSCALL: sched_as_string[i] += 's'; break; + case TRACE_MARKER_TYPE_SYSCALL_TRACE_END: + in_syscall[i] = false; + ANNOTATE_FALLTHROUGH; + case TRACE_MARKER_TYPE_SYSCALL_TRACE_START: + sched_as_string[i] += '1' + + static_cast(memref.marker.marker_value - syscall_base); + break; + default: sched_as_string[i] += '?'; break; + } + } + prev_tid[i] = memref.instr.tid; + prev_in_ord[i] = outputs[i]->get_input_interface()->get_record_ordinal(); + prev_out_ord[i] = outputs[i]->get_record_ordinal(); + } + } + return sched_as_string; +} + static void test_kernel_switch_sequences() { @@ -5850,86 +5959,9 @@ test_kernel_switch_sequences() scheduler_t::STATUS_SUCCESS) assert(false); - // We have a custom version of run_lockstep_simulation here for more precise - // testing of the markers and instructions and interfaces. - // We record the entire sequence for a detailed check of some records, along with - // a character representation for a higher-level view of the whole sequence. - std::vector outputs(NUM_OUTPUTS, nullptr); - std::vector eof(NUM_OUTPUTS, false); - for (int i = 0; i < NUM_OUTPUTS; i++) - outputs[i] = scheduler.get_stream(i); - int num_eof = 0; - std::vector> refs(NUM_OUTPUTS); - std::vector sched_as_string(NUM_OUTPUTS); - std::vector prev_tid(NUM_OUTPUTS, INVALID_THREAD_ID); - std::vector in_switch(NUM_OUTPUTS, false); - std::vector prev_in_ord(NUM_OUTPUTS, 0); - std::vector prev_out_ord(NUM_OUTPUTS, 0); - while (num_eof < NUM_OUTPUTS) { - for (int i = 0; i < NUM_OUTPUTS; i++) { - if (eof[i]) - continue; - memref_t memref; - scheduler_t::stream_status_t status = outputs[i]->next_record(memref); - if (status == scheduler_t::STATUS_EOF) { - ++num_eof; - eof[i] = true; - continue; - } - if (status == scheduler_t::STATUS_IDLE) { - sched_as_string[i] += '_'; - continue; - } - assert(status == scheduler_t::STATUS_OK); - refs[i].push_back(memref); - if (memref.instr.tid != prev_tid[i]) { - if (!sched_as_string[i].empty()) - sched_as_string[i] += ','; - sched_as_string[i] += - 'A' + static_cast(memref.instr.tid - TID_BASE); - } - if (memref.marker.type == TRACE_TYPE_MARKER && - memref.marker.marker_type == TRACE_MARKER_TYPE_CONTEXT_SWITCH_START) - in_switch[i] = true; - if (in_switch[i]) { - // Test that switch code is marked synthetic. - assert(outputs[i]->is_record_synthetic()); - // Test that switch code doesn't count toward input ordinals, but - // does toward output ordinals. - assert(outputs[i]->get_input_interface()->get_record_ordinal() == - prev_in_ord[i] || - // Won't match if we just switched inputs. - (memref.marker.type == TRACE_TYPE_MARKER && - memref.marker.marker_type == - TRACE_MARKER_TYPE_CONTEXT_SWITCH_START)); - assert(outputs[i]->get_record_ordinal() > prev_out_ord[i]); - } else - assert(!outputs[i]->is_record_synthetic()); - if (type_is_instr(memref.instr.type)) - sched_as_string[i] += 'i'; - else if (memref.marker.type == TRACE_TYPE_MARKER) { - switch (memref.marker.marker_type) { - case TRACE_MARKER_TYPE_VERSION: sched_as_string[i] += 'v'; break; - case TRACE_MARKER_TYPE_TIMESTAMP: sched_as_string[i] += '0'; break; - case TRACE_MARKER_TYPE_CONTEXT_SWITCH_END: - in_switch[i] = false; - ANNOTATE_FALLTHROUGH; - case TRACE_MARKER_TYPE_CONTEXT_SWITCH_START: - if (memref.marker.marker_value == scheduler_t::SWITCH_PROCESS) - sched_as_string[i] += 'p'; - else if (memref.marker.marker_value == scheduler_t::SWITCH_THREAD) - sched_as_string[i] += 't'; - else - assert(false && "unknown context switch type"); - break; - default: sched_as_string[i] += '?'; break; - } - } - prev_tid[i] = memref.instr.tid; - prev_in_ord[i] = outputs[i]->get_input_interface()->get_record_ordinal(); - prev_out_ord[i] = outputs[i]->get_record_ordinal(); - } - } + std::vector> refs; + std::vector sched_as_string = + run_lockstep_simulation_for_kernel_seq(scheduler, NUM_OUTPUTS, TID_BASE, 0, refs); // Check the high-level strings. for (int i = 0; i < NUM_OUTPUTS; i++) { std::cerr << "cpu #" << i << " schedule: " << sched_as_string[i] << "\n"; @@ -6043,10 +6075,8 @@ test_kernel_syscall_sequences() { std::cerr << "\n----------------\nTesting kernel syscall sequences\n"; static constexpr memref_tid_t TID_IN_SYSCALLS = 1; - static constexpr int SYSCALL_NUM_1 = 42; - static constexpr int SYSCALL_NUM_2 = 43; - static constexpr addr_t SYSCALL_NUM_1_PC_START = 0xfeed101; - static constexpr addr_t SYSCALL_NUM_2_PC_START = 0xcafe101; + static constexpr int SYSCALL_BASE = 42; + static constexpr addr_t SYSCALL_PC_START = 0xfeed101; static constexpr int NUM_OUTPUTS = 2; static constexpr memref_tid_t TID_BASE = 100; { @@ -6057,17 +6087,17 @@ test_kernel_syscall_sequences() make_pid(TID_IN_SYSCALLS), make_version(TRACE_ENTRY_VERSION), make_timestamp(0), - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1), - make_instr(SYSCALL_NUM_1_PC_START), - make_instr(SYSCALL_NUM_1_PC_START + 1), - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_BASE), + make_instr(SYSCALL_PC_START), + make_instr(SYSCALL_PC_START + 1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_BASE), // XXX: Currently all syscall traces are concatenated. We may change // this to use an archive file instead. - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_2), - make_instr(SYSCALL_NUM_2_PC_START), - make_instr(SYSCALL_NUM_2_PC_START + 1), - make_instr(SYSCALL_NUM_2_PC_START + 2), - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_2), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_BASE + 1), + make_instr(SYSCALL_PC_START + 10), + make_instr(SYSCALL_PC_START + 11), + make_instr(SYSCALL_PC_START + 12), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_BASE + 1), make_exit(TID_IN_SYSCALLS), make_footer(), /* clang-format on */ @@ -6092,9 +6122,8 @@ test_kernel_syscall_sequences() for (int instr_idx = 0; instr_idx < NUM_INSTRS; instr_idx++) { inputs.push_back(make_instr(42 + instr_idx * 4)); if (instr_idx % 2 == 0) { - inputs.push_back(make_marker( - TRACE_MARKER_TYPE_SYSCALL, - (instr_idx / 2) % 2 == 0 ? SYSCALL_NUM_1 : SYSCALL_NUM_2)); + inputs.push_back(make_marker(TRACE_MARKER_TYPE_SYSCALL, + SYSCALL_BASE + (instr_idx / 2) % 2)); } } inputs.push_back(make_exit(tid)); @@ -6114,84 +6143,9 @@ test_kernel_syscall_sequences() if (scheduler.init(sched_inputs, NUM_OUTPUTS, std::move(sched_ops)) != scheduler_t::STATUS_SUCCESS) assert(false); - - // We have a custom version of run_lockstep_simulation here for more precise - // testing of the markers and instructions and interfaces. - // We record the entire sequence for a detailed check of some records, along with - // a character representation for a higher-level view of the whole sequence. - std::vector outputs(NUM_OUTPUTS, nullptr); - std::vector eof(NUM_OUTPUTS, false); - for (int i = 0; i < NUM_OUTPUTS; i++) - outputs[i] = scheduler.get_stream(i); - int num_eof = 0; - std::vector> refs(NUM_OUTPUTS); - std::vector sched_as_string(NUM_OUTPUTS); - std::vector prev_tid(NUM_OUTPUTS, INVALID_THREAD_ID); - std::vector in_syscall(NUM_OUTPUTS, false); - std::vector prev_in_ord(NUM_OUTPUTS, 0); - std::vector prev_out_ord(NUM_OUTPUTS, 0); - while (num_eof < NUM_OUTPUTS) { - for (int i = 0; i < NUM_OUTPUTS; i++) { - if (eof[i]) - continue; - memref_t memref; - scheduler_t::stream_status_t status = outputs[i]->next_record(memref); - if (status == scheduler_t::STATUS_EOF) { - ++num_eof; - eof[i] = true; - continue; - } - if (status == scheduler_t::STATUS_IDLE) { - sched_as_string[i] += '_'; - continue; - } - assert(status == scheduler_t::STATUS_OK); - refs[i].push_back(memref); - if (memref.instr.tid != prev_tid[i]) { - if (!sched_as_string[i].empty()) - sched_as_string[i] += ','; - sched_as_string[i] += - 'A' + static_cast(memref.instr.tid - TID_BASE); - } - if (memref.marker.type == TRACE_TYPE_MARKER && - memref.marker.marker_type == TRACE_MARKER_TYPE_SYSCALL_TRACE_START) - in_syscall[i] = true; - if (in_syscall[i]) { - // Test that syscall code is marked synthetic. - assert(outputs[i]->is_record_synthetic()); - // Test that dynamically injected syscall code doesn't count toward - // input ordinals, but does toward output ordinals. - assert(outputs[i]->get_input_interface()->get_record_ordinal() == - prev_in_ord[i]); - assert(outputs[i]->get_record_ordinal() > prev_out_ord[i]); - } else - assert(!outputs[i]->is_record_synthetic()); - if (type_is_instr(memref.instr.type)) - sched_as_string[i] += 'i'; - else if (memref.marker.type == TRACE_TYPE_MARKER) { - switch (memref.marker.marker_type) { - case TRACE_MARKER_TYPE_VERSION: sched_as_string[i] += 'v'; break; - case TRACE_MARKER_TYPE_TIMESTAMP: sched_as_string[i] += '0'; break; - case TRACE_MARKER_TYPE_SYSCALL: sched_as_string[i] += 's'; break; - case TRACE_MARKER_TYPE_SYSCALL_TRACE_END: - in_syscall[i] = false; - ANNOTATE_FALLTHROUGH; - case TRACE_MARKER_TYPE_SYSCALL_TRACE_START: - if (memref.marker.marker_value == SYSCALL_NUM_1) - sched_as_string[i] += '1'; - else if (memref.marker.marker_value == SYSCALL_NUM_2) - sched_as_string[i] += '2'; - else - assert(false && "unknown system call num"); - break; - default: sched_as_string[i] += '?'; break; - } - } - prev_tid[i] = memref.instr.tid; - prev_in_ord[i] = outputs[i]->get_input_interface()->get_record_ordinal(); - prev_out_ord[i] = outputs[i]->get_record_ordinal(); - } - } + std::vector> refs; + std::vector sched_as_string = run_lockstep_simulation_for_kernel_seq( + scheduler, NUM_OUTPUTS, TID_BASE, SYSCALL_BASE, refs); // Check the high-level strings. for (int i = 0; i < NUM_OUTPUTS; i++) { std::cerr << "cpu #" << i << " schedule: " << sched_as_string[i] << "\n"; @@ -6216,15 +6170,15 @@ test_kernel_syscall_sequences() TRACE_MARKER_TYPE_TIMESTAMP, TIMESTAMP) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) && + TRACE_MARKER_TYPE_SYSCALL, SYSCALL_BASE) && // Syscall_1 trace on first thread. check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_BASE) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) && + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_BASE) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, TRACE_MARKER_TYPE_VERSION) && @@ -6232,28 +6186,28 @@ test_kernel_syscall_sequences() TRACE_MARKER_TYPE_TIMESTAMP, TIMESTAMP) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_1) && + TRACE_MARKER_TYPE_SYSCALL, SYSCALL_BASE) && // Syscall_1 trace on second thread. check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1) && + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_BASE) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE + 2, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1) && + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_BASE) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL, SYSCALL_NUM_2) && + TRACE_MARKER_TYPE_SYSCALL, SYSCALL_BASE + 1) && // Syscall_2 trace on first thread. check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_2) && + TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_BASE + 1) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_INSTR) && check_ref(refs[0], idx, TID_BASE, TRACE_TYPE_MARKER, - TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_2); + TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_BASE + 1); assert(res); } { @@ -6264,15 +6218,15 @@ test_kernel_syscall_sequences() make_thread(TID_IN_SYSCALLS), make_pid(TID_IN_SYSCALLS), make_version(TRACE_ENTRY_VERSION), - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1), - make_instr(SYSCALL_NUM_1_PC_START), - make_instr(SYSCALL_NUM_1_PC_START + 1), - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_BASE), + make_instr(SYSCALL_PC_START), + make_instr(SYSCALL_PC_START + 1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_BASE), // Error: duplicate trace for the same syscall. - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_NUM_1), - make_instr(SYSCALL_NUM_2_PC_START), - make_instr(SYSCALL_NUM_2_PC_START + 1), - make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_NUM_1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_START, SYSCALL_BASE), + make_instr(SYSCALL_PC_START), + make_instr(SYSCALL_PC_START + 1), + make_marker(TRACE_MARKER_TYPE_SYSCALL_TRACE_END, SYSCALL_BASE), make_exit(TID_IN_SYSCALLS), make_footer(), /* clang-format on */ diff --git a/clients/drcachesim/tests/switch_insertion.templatex b/clients/drcachesim/tests/switch_insertion.templatex index 91f6dc0466..fd1edafbb6 100644 --- a/clients/drcachesim/tests/switch_insertion.templatex +++ b/clients/drcachesim/tests/switch_insertion.templatex @@ -2,7 +2,7 @@ Basic counts tool results: Total counts: [1-9][0-9][0-9][0-9][0-9][0-9] total \(fetched\) instructions 5971 total unique \(fetched\) instructions - [1-9][0-9][0-9][0-9][0-9][0-9] total userspace instructions - [1-9][0-9][0-9] total kernel instructions + 638938 total userspace instructions + 726 total kernel instructions [1-9][0-9][0-9][0-9][0-9][0-9] total non-fetched instructions .* diff --git a/clients/drcachesim/tests/syscall_insertion.templatex b/clients/drcachesim/tests/syscall_insertion.templatex index 91f6dc0466..33f8729f99 100644 --- a/clients/drcachesim/tests/syscall_insertion.templatex +++ b/clients/drcachesim/tests/syscall_insertion.templatex @@ -2,7 +2,7 @@ Basic counts tool results: Total counts: [1-9][0-9][0-9][0-9][0-9][0-9] total \(fetched\) instructions 5971 total unique \(fetched\) instructions - [1-9][0-9][0-9][0-9][0-9][0-9] total userspace instructions - [1-9][0-9][0-9] total kernel instructions + 638938 total userspace instructions + 109 total kernel instructions [1-9][0-9][0-9][0-9][0-9][0-9] total non-fetched instructions .* diff --git a/clients/drcachesim/tests/syscall_insertion_core_sharded.templatex b/clients/drcachesim/tests/syscall_insertion_core_sharded.templatex index 91f6dc0466..33f8729f99 100644 --- a/clients/drcachesim/tests/syscall_insertion_core_sharded.templatex +++ b/clients/drcachesim/tests/syscall_insertion_core_sharded.templatex @@ -2,7 +2,7 @@ Basic counts tool results: Total counts: [1-9][0-9][0-9][0-9][0-9][0-9] total \(fetched\) instructions 5971 total unique \(fetched\) instructions - [1-9][0-9][0-9][0-9][0-9][0-9] total userspace instructions - [1-9][0-9][0-9] total kernel instructions + 638938 total userspace instructions + 109 total kernel instructions [1-9][0-9][0-9][0-9][0-9][0-9] total non-fetched instructions .* From e268f9743630fb8067c00da727648c4e59edc748 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Sun, 16 Feb 2025 22:09:57 -0500 Subject: [PATCH 16/19] Rename process_marker to avoid confusion --- clients/drcachesim/scheduler/scheduler_impl.cpp | 8 +++++--- clients/drcachesim/scheduler/scheduler_impl.h | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index fbac22b36a..47229d2cbe 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -2542,8 +2542,8 @@ scheduler_impl_tmpl_t::update_switch_stats( template void -scheduler_impl_tmpl_t::process_marker(RecordType record, - output_ordinal_t output) +scheduler_impl_tmpl_t::update_syscall_state( + RecordType record, output_ordinal_t output) { if (outputs_[output].hit_syscall_code_end) { // We have to delay so the end marker is still in_syscall_code. @@ -2702,7 +2702,9 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp --input->instrs_pre_read; VDO(this, 5, print_record(record);); - process_marker(record, output); + // We want check_for_input_switch() to have the updated state, so we process + // syscall trace related markers now. + update_syscall_state(record, output); bool need_new_input = false; bool preempt = false; diff --git a/clients/drcachesim/scheduler/scheduler_impl.h b/clients/drcachesim/scheduler/scheduler_impl.h index d3a4c9a8ef..f03cb38d63 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.h +++ b/clients/drcachesim/scheduler/scheduler_impl.h @@ -410,7 +410,7 @@ template class scheduler_impl_tmpl_t input_ordinal_t new_input); void - process_marker(RecordType record, output_ordinal_t output); + update_syscall_state(RecordType record, output_ordinal_t output); /// /////////////////////////////////////////////////////////////////////////// From 66d1b6cb607627f01b6e34f5b2d7293e64b60ebc Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Wed, 26 Feb 2025 14:51:25 -0500 Subject: [PATCH 17/19] Address reviewer suggested edits --- clients/drcachesim/scheduler/scheduler.h | 12 ++++++------ .../drcachesim/scheduler/scheduler_impl.cpp | 18 +++++++++--------- clients/drcachesim/scheduler/scheduler_impl.h | 5 ++++- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler.h b/clients/drcachesim/scheduler/scheduler.h index 1e4578cb22..2caa9356c0 100644 --- a/clients/drcachesim/scheduler/scheduler.h +++ b/clients/drcachesim/scheduler/scheduler.h @@ -833,12 +833,12 @@ template class scheduler_tmpl_t { * Input file containing template sequences of kernel system call code. * Each sequence must start with a #TRACE_MARKER_TYPE_SYSCALL_TRACE_START * marker and end with #TRACE_MARKER_TYPE_SYSCALL_TRACE_END. - * The values of each marker must hold the system call number value - * indicating the system call it corresponds to. Sequences for - * multiple system calls are concatenated into a single file. - * Each sequence should be in the regular offline drmemtrace format. - * Each sequence is inserted into the output stream after all - * #TRACE_MARKER_TYPE_SYSCALL markers with the same value. + * The value of each marker must hold the system call number for the system call + * it corresponds to. Sequences for multiple system calls are concatenated into a + * single file. Each sequence should be in the regular offline drmemtrace format. + * Whenever a #TRACE_MARKER_TYPE_SYSCALL marker is encountered in a trace, if a + * corresponding sequence with the same marker value exists it is inserted into + * the output stream after the #TRACE_MARKER_TYPE_SYSCALL marker. * The same file (or reader) must be passed when replaying as this kernel * code is not stored when recording. * An alternative to passing the file path is to pass #kernel_syscall_reader diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 10a2958fef..4212c04ad7 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -351,7 +351,7 @@ scheduler_impl_tmpl_t::insert_switch_tid_pid(input_info_t &i template <> template <> typename scheduler_tmpl_t::switch_type_t -scheduler_impl_tmpl_t::default_kernel_sequence_key() +scheduler_impl_tmpl_t::invalid_kernel_sequence_key() { return switch_type_t::SWITCH_INVALID; } @@ -359,7 +359,7 @@ scheduler_impl_tmpl_t::default_kernel_sequence_key() template <> template <> int -scheduler_impl_tmpl_t::default_kernel_sequence_key() +scheduler_impl_tmpl_t::invalid_kernel_sequence_key() { // System numbers are small non-negative integers. return -1; @@ -588,7 +588,7 @@ scheduler_impl_tmpl_t::insert_switch_tid_pid( template <> template <> typename scheduler_tmpl_t::switch_type_t -scheduler_impl_tmpl_t::default_kernel_sequence_key() +scheduler_impl_tmpl_t::invalid_kernel_sequence_key() { return switch_type_t::SWITCH_INVALID; } @@ -596,7 +596,7 @@ scheduler_impl_tmpl_t::default_kernel_sequence_k template <> template <> int -scheduler_impl_tmpl_t::default_kernel_sequence_key() +scheduler_impl_tmpl_t::invalid_kernel_sequence_key() { // System numbers are small non-negative integers. return -1; @@ -1484,8 +1484,8 @@ scheduler_impl_tmpl_t::read_kernel_sequences( // memory and don't need to stream them on every use. // We read a single stream, even if underneath these are split into subfiles // in an archive. - SequenceKey sequence_key = default_kernel_sequence_key(); - const SequenceKey DEFAULT_SEQ_KEY = default_kernel_sequence_key(); + SequenceKey sequence_key = invalid_kernel_sequence_key(); + const SequenceKey INVALID_SEQ_KEY = invalid_kernel_sequence_key(); bool in_sequence = false; while (*reader != *reader_end) { RecordType record = **reader; @@ -1501,7 +1501,7 @@ scheduler_impl_tmpl_t::read_kernel_sequences( } sequence_key = static_cast(marker_value); in_sequence = true; - if (sequence_key == DEFAULT_SEQ_KEY) { + if (sequence_key == INVALID_SEQ_KEY) { error_string_ += "Invalid " + sequence_type + " sequence found with default key"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; @@ -1523,10 +1523,10 @@ scheduler_impl_tmpl_t::read_kernel_sequences( error_string_ += sequence_type + " marker values mismatched"; return sched_type_t::STATUS_ERROR_INVALID_PARAMETER; } - sequence_key = DEFAULT_SEQ_KEY; - in_sequence = false; VPRINT(this, 1, "Read %zu kernel %s records for key %d\n", sequence[sequence_key].size(), sequence_type.c_str(), sequence_key); + sequence_key = INVALID_SEQ_KEY; + in_sequence = false; } ++(*reader); } diff --git a/clients/drcachesim/scheduler/scheduler_impl.h b/clients/drcachesim/scheduler/scheduler_impl.h index c189c843a7..666535c4ee 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.h +++ b/clients/drcachesim/scheduler/scheduler_impl.h @@ -197,6 +197,7 @@ template class scheduler_impl_tmpl_t // We use a deque so we can iterate over it. std::deque queue; bool cur_from_queue; + std::set binding; int priority = 0; std::vector regions_of_interest; @@ -483,6 +484,8 @@ template class scheduler_impl_tmpl_t // This is accessed by other outputs for stealing and rebalancing. // Indirected so we can store it in our vector. std::unique_ptr> active; + // XXX: in_syscall_code and hit_syscall_code_end arguably are tied to an input + // stream and must be a part of input_info_t instead. bool in_syscall_code = false; bool hit_syscall_code_end = false; bool in_context_switch_code = false; @@ -751,7 +754,7 @@ template class scheduler_impl_tmpl_t template SequenceKey - default_kernel_sequence_key(); + invalid_kernel_sequence_key(); template scheduler_status_t From eff4860ccb83813adccfb665a5fe7659d2305b5c Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Wed, 26 Feb 2025 15:25:50 -0500 Subject: [PATCH 18/19] Use common inject_kernel_sequence method for context switches too --- clients/drcachesim/common/memtrace_stream.h | 2 +- .../drcachesim/scheduler/scheduler_impl.cpp | 62 +++++++++---------- clients/drcachesim/scheduler/scheduler_impl.h | 2 +- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/clients/drcachesim/common/memtrace_stream.h b/clients/drcachesim/common/memtrace_stream.h index b02b4dfddf..4a9f255803 100644 --- a/clients/drcachesim/common/memtrace_stream.h +++ b/clients/drcachesim/common/memtrace_stream.h @@ -113,7 +113,7 @@ class memtrace_stream_t { */ SCHED_STAT_HIT_OUTPUT_LIMIT, /** - * Counts the instances when the kernel context switch sequence was injected. + * Counts the instances when the kernel context switch sequences were injected. */ SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS, /** Count of statistic types. */ diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index 4212c04ad7..c8565bbb03 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -2491,13 +2491,16 @@ scheduler_impl_tmpl_t::pick_next_input(output_ordinal_t } // We can't easily place these stats inside set_cur_input() as we call that to // temporarily give up our input. - on_context_switch(output, prev_index, index); + stream_status_t on_switch_res = on_context_switch(output, prev_index, index); + if (on_switch_res != stream_status_t::STATUS_OK) { + return on_switch_res; + } set_cur_input(output, index); return res; } template -void +typename scheduler_tmpl_t::stream_status_t scheduler_impl_tmpl_t::on_context_switch( output_ordinal_t output, input_ordinal_t prev_input, input_ordinal_t new_input) { @@ -2526,12 +2529,12 @@ scheduler_impl_tmpl_t::on_context_switch( // set_cur_input. Here we get the stolen input events too, and we don't have // to filter out the init-time set_cur_input cases. if (!do_inject_switch_seq) - return; + return stream_status_t::STATUS_OK; if (inputs_[new_input].pid != INVALID_PID) { insert_switch_tid_pid(inputs_[new_input]); } if (switch_sequence_.empty()) - return; + return stream_status_t::STATUS_OK; switch_type_t switch_type = sched_type_t::SWITCH_INVALID; if ( // XXX: idle-to-input transitions are assumed to be process switches // for now. But we may want to improve this heuristic. @@ -2540,31 +2543,24 @@ scheduler_impl_tmpl_t::on_context_switch( switch_type = sched_type_t::SWITCH_PROCESS; else switch_type = sched_type_t::SWITCH_THREAD; - if (switch_sequence_[switch_type].empty()) - return; - // Inject kernel context switch code. Since the injected records belong to - // this input (the kernel is acting on behalf of this input) we insert them - // into the input's queue, but ahead of any prior queued items. This is why - // we walk in reverse, for the push_front calls to the deque. We update the - // tid of the records here to match. They are considered as - // is_record_synthetic() and do not affect input stream ordinals. - // XXX: These will appear before the top headers of a new thread which is - // slightly odd to have regular records with the new tid before the top - // headers. - ++outputs_[output] - .stats[memtrace_stream_t::SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS]; - for (int i = static_cast(switch_sequence_[switch_type].size()) - 1; i >= 0; - --i) { - RecordType record = switch_sequence_[switch_type][i]; - record_type_set_tid(record, inputs_[new_input].tid); - inputs_[new_input].queue.emplace_front(record); - } - VPRINT(this, 3, "Inserted %zu switch for type %d from %d.%d to %d.%d\n", - switch_sequence_[switch_type].size(), switch_type, - prev_input != sched_type_t::INVALID_INPUT_ORDINAL - ? inputs_[prev_input].workload - : -1, - prev_input, inputs_[new_input].workload, new_input); + if (switch_sequence_.find(switch_type) == switch_sequence_.end() || + switch_sequence_[switch_type].empty()) + return stream_status_t::STATUS_OK; + stream_status_t res = + inject_kernel_sequence(switch_sequence_[switch_type], &inputs_[new_input]); + if (res == stream_status_t::STATUS_OK) { + ++outputs_[output] + .stats[memtrace_stream_t::SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS]; + VPRINT(this, 3, "Inserted %zu switch for type %d from %d.%d to %d.%d\n", + switch_sequence_[switch_type].size(), switch_type, + prev_input != sched_type_t::INVALID_INPUT_ORDINAL + ? inputs_[prev_input].workload + : -1, + prev_input, inputs_[new_input].workload, new_input); + } else if (res != stream_status_t::STATUS_EOF) { + return res; + } + return stream_status_t::STATUS_OK; } template @@ -2815,7 +2811,11 @@ scheduler_impl_tmpl_t::next_record(output_ordinal_t outp } else if (res == sched_type_t::STATUS_STOLE) { // We need to loop to get the new record. input = &inputs_[outputs_[output].cur_input]; - on_context_switch(output, prev_input, input->index); + stream_status_t on_switch_res = + on_context_switch(output, prev_input, input->index); + if (on_switch_res != stream_status_t::STATUS_OK) { + return on_switch_res; + } lock.unlock(); lock = std::unique_lock(*input->lock); lock.lock(); @@ -2862,7 +2862,7 @@ scheduler_impl_tmpl_t::finalize_next_record( return res; } } - return sched_type_t::STATUS_OK; + return stream_status_t::STATUS_OK; } template diff --git a/clients/drcachesim/scheduler/scheduler_impl.h b/clients/drcachesim/scheduler/scheduler_impl.h index 666535c4ee..e0d35b03ab 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.h +++ b/clients/drcachesim/scheduler/scheduler_impl.h @@ -406,7 +406,7 @@ template class scheduler_impl_tmpl_t uint64_t scale_blocked_time(uint64_t initial_time) const; - void + stream_status_t on_context_switch(output_ordinal_t output, input_ordinal_t prev_input, input_ordinal_t new_input); From 65173c4f56a76622d57843d6c198e85da216ddb5 Mon Sep 17 00:00:00 2001 From: Abhinav Anil Sharma Date: Wed, 26 Feb 2025 18:57:56 -0500 Subject: [PATCH 19/19] Cleanup --- clients/drcachesim/scheduler/scheduler_impl.cpp | 5 ++--- clients/drcachesim/scheduler/scheduler_impl.h | 4 +++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/clients/drcachesim/scheduler/scheduler_impl.cpp b/clients/drcachesim/scheduler/scheduler_impl.cpp index c8565bbb03..18687ce6a4 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.cpp +++ b/clients/drcachesim/scheduler/scheduler_impl.cpp @@ -2543,15 +2543,14 @@ scheduler_impl_tmpl_t::on_context_switch( switch_type = sched_type_t::SWITCH_PROCESS; else switch_type = sched_type_t::SWITCH_THREAD; - if (switch_sequence_.find(switch_type) == switch_sequence_.end() || - switch_sequence_[switch_type].empty()) + if (switch_sequence_.find(switch_type) == switch_sequence_.end()) return stream_status_t::STATUS_OK; stream_status_t res = inject_kernel_sequence(switch_sequence_[switch_type], &inputs_[new_input]); if (res == stream_status_t::STATUS_OK) { ++outputs_[output] .stats[memtrace_stream_t::SCHED_STAT_KERNEL_SWITCH_SEQUENCE_INJECTIONS]; - VPRINT(this, 3, "Inserted %zu switch for type %d from %d.%d to %d.%d\n", + VPRINT(this, 3, "Inserted %zu switch records for type %d from %d.%d to %d.%d\n", switch_sequence_[switch_type].size(), switch_type, prev_input != sched_type_t::INVALID_INPUT_ORDINAL ? inputs_[prev_input].workload diff --git a/clients/drcachesim/scheduler/scheduler_impl.h b/clients/drcachesim/scheduler/scheduler_impl.h index e0d35b03ab..5049614684 100644 --- a/clients/drcachesim/scheduler/scheduler_impl.h +++ b/clients/drcachesim/scheduler/scheduler_impl.h @@ -485,7 +485,9 @@ template class scheduler_impl_tmpl_t // Indirected so we can store it in our vector. std::unique_ptr> active; // XXX: in_syscall_code and hit_syscall_code_end arguably are tied to an input - // stream and must be a part of input_info_t instead. + // stream and must be a part of input_info_t instead. Today we do not context + // switch in the middle of injected kernel syscall code, but if we did, this + // state would be incorrect or lost. bool in_syscall_code = false; bool hit_syscall_code_end = false; bool in_context_switch_code = false;