| Message ID | 20231103-bb-timetravel-patches-v1-6-e2c68efcf664@uni-rostock.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Several Time Travel Mode Enhancements | expand |
On Fri, 2023-11-03 at 16:41 +0000, Benjamin Beichler wrote: > The number of UM_TIMETRAVEL_REQUEST msgs is tried to reduced, if they > are redundant via the time_travel_ext_prev_request_valid flag. > This ... doesn't parse so well. I think I get what you're trying to say, but it's a bit awkward? Maybe say "The code attempts to reduce the number of ... messages if they are redundant. This is achieved with the ... flag." or something like that? At least that's what I think what you're trying to say :) > This can > create a race condition, when an interrupt happens and the idle loop > wants to wait. > > This means, sometimes the UM_TIMETRAVEL_REQUEST are sent(when the idle > loop already started waiting) and sometimes not, which makes it harder > to implement the other end of the scheduler. Not sure I get that though, why does it matter? Just keep track of the last request? Do you have some kind of (calendar) logs that would show the different message behaviour? FWIW, we've also got work pending for a while now that we should submit that implements scheduling via shared memory for the most part, so that all these REQUEST and time update messages just go away _entirely_, which helps a lot for performance too. > +++ b/arch/um/kernel/time.c > @@ -446,6 +446,7 @@ void time_travel_add_irq_event(struct time_travel_event *e) > BUG_ON(time_travel_mode != TT_MODE_EXTERNAL); > > time_travel_ext_get_time(); > + time_travel_ext_prev_request_valid = false; > At the very least I think there should be a comment here, but I'm not really convinced yet why this makes sense :) johannes
diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c index abd5fd0f62ee..54fc4a69cb59 100644 --- a/arch/um/kernel/time.c +++ b/arch/um/kernel/time.c @@ -446,6 +446,7 @@ void time_travel_add_irq_event(struct time_travel_event *e) BUG_ON(time_travel_mode != TT_MODE_EXTERNAL); time_travel_ext_get_time(); + time_travel_ext_prev_request_valid = false; /* * We could model interrupt latency here, for now just * don't have any latency at all and request the exact
The number of UM_TIMETRAVEL_REQUEST msgs is tried to reduced, if they are redundant via the time_travel_ext_prev_request_valid flag. This can create a race condition, when an interrupt happens and the idle loop wants to wait. This means, sometimes the UM_TIMETRAVEL_REQUEST are sent(when the idle loop already started waiting) and sometimes not, which makes it harder to implement the other end of the scheduler. Therefore, this fix make the time travel protocol a bit more deterministic by always sending the UM_TIMETRAVEL_REQUEST msg. Signed-off-by: Benjamin Beichler <benjamin.beichler@uni-rostock.de> --- arch/um/kernel/time.c | 1 + 1 file changed, 1 insertion(+)