Re: [PATCH RFC 06/11] um: always send UM_TIMETRAVEL_REQUEST from ISRs

[Johannes Berg <johannes@sipsolutions.net>]

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

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um