i#6822 unscheduled: Add unscheduled-input drmemtrace support #6826
Conversation
Augments the drmemtrace scheduler with a new notion of an "unscheduled" input which, if it has no timeout, is not runnable indefinitely until another input explicitly wakes it up. Adds 3 new markers: + TRACE_MARKER_TYPE_SYSCALL_UNSCHEDULE, which makes the caller "unscheduled". + TRACE_MARKER_TYPE_SYSCALL_SCHEDULE, which makes a target no longer "unscheduled". + TRACE_MARKER_TYPE_SYSCALL_ARG_TIMEOUT which adds a timeout parameter to TRACE_MARKER_TYPE_SYSCALL_UNSCHEDULE and TRACE_MARKER_TYPE_DIRECT_THREAD_SWITCH Adds handling for the new marker types. Changes TRACE_MARKER_TYPE_DIRECT_THREAD_SWITCH to make the source input unscheduled, unless a timeout is passed which is honored (previously it was ignored and the syscall latency was used as the block time if it was over the thresholds). Adds a fallback in case unscheduled inputs are left with no schedulable inputs. Adds unit tests. Also tested on a large trace with many real-world cases of these direct switches and unschedulable threads. Issue: #6822
|
I started the review but I feel I need to spend more time with this PR. This review will be a bit delayed. Sorry about that. |
| case TRACE_MARKER_TYPE_SYSCALL_UNSCHEDULE: | ||
| if (!options_.honor_direct_switches) | ||
| break; | ||
| if (input.skip_next_unscheduled) { |
There was a problem hiding this comment.
Explain this further. It's interesting that a prior "schedule" marker may have such an effect on a future "unschedule", especially because the "schedule" marker docs say "if it were currently unschedulable".
What real world scenario is this trying to model? What if the "do-not-unschedule" input was blocked on a I/O syscall?
Also, (not saying that we should) what's the rationale behind skip_next_unscheduled being a bool and not an int (maybe multiple other threads want to "schedule" the thread), therefore the thread should get that many get-out-of-unschedulable-state tokens.
There was a problem hiding this comment.
The real-world kernel allows only a single skip. I believe the real-world feature's rationale is to avoid the need for synchronization in some not-uncommon pattern of calls but I'm not sure I know what that pattern is.
There was a problem hiding this comment.
The real-world kernel allows only a single skip.
Multiple threads could want some blocked thread to be scheduled, perhaps at different times in the app's execution, but at the same time when the trace is simulated/scheduled.
There was a problem hiding this comment.
especially because the "schedule" marker docs say "if it were currently unschedulable".
This was updated, as noted in the other thread.
Multiple threads could want some blocked thread to be scheduled, perhaps at different times in the app's execution, but at the same time when the trace is simulated/scheduled.
Maybe. There are many theoretical cases. This re-scheduling is always going to be best-effort. Trying to have a queue of wakers seems just as likely to cause non-representative behavior than to fix it though so it does not seem worth going in that direction. Added a comment.
There was a problem hiding this comment.
If the "schedule" events get lost due to getting accumulated, then the target thready may experience a hang if some of its "unschedulable" events don't get matched up with a "schedule".
Augments the drmemtrace scheduler with a new notion of an "unscheduled" input which, if it has no timeout, is not runnable indefinitely until another input explicitly wakes it up.
Adds 3 new markers:
Adds handling for the new marker types.
Changes TRACE_MARKER_TYPE_DIRECT_THREAD_SWITCH to make the source input unscheduled, unless a timeout is passed which is honored (previously it was ignored and the syscall latency was used as the block time if it was over the thresholds).
Adds a fallback in case a state is reached with no schedulable inputs yet some number of unscheduled inputs who would otherwise hang forever.
Adds unit tests.
Also tested on a large trace with many real-world cases of these direct switches and unschedulable threads.
Issue: #6822