All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] um: time-travel: actually apply "free-until" optimisation
@ 2020-12-03 14:36 Johannes Berg
  2020-12-10 22:28 ` Richard Weinberger
  0 siblings, 1 reply; 3+ messages in thread
From: Johannes Berg @ 2020-12-03 14:36 UTC (permalink / raw)
  To: linux-um; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Due a bug - we never checked the time_travel_ext_free_until value - we
were always requesting time for every single scheduling. This adds up
since we make reading time cost 256ns, and it's a fairly common call.
Fix this.

While at it, also make reading time only cost something when we're not
currently waiting for our scheduling turn - otherwise things get mixed
up in a very confusing way. We should never get here, since we're not
actually running, but it's possible if you stick printk() or such into
the virtio code that must handle the external interrupts.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/kernel/time.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
index 303565ce8c64..5ac084deac7e 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -183,6 +183,14 @@ static void time_travel_ext_update_request(unsigned long long time)
 	    time == time_travel_ext_prev_request)
 		return;
 
+	/*
+	 * if we're running and are allowed to run past the request
+	 * then we don't need to update it either
+	 */
+	if (!time_travel_ext_waiting && time_travel_ext_free_until_valid &&
+	    time < time_travel_ext_free_until)
+		return;
+
 	time_travel_ext_prev_request = time;
 	time_travel_ext_prev_request_valid = true;
 	time_travel_ext_req(UM_TIMETRAVEL_REQUEST, time);
@@ -217,6 +225,7 @@ static void time_travel_ext_wait(bool idle)
 	};
 
 	time_travel_ext_prev_request_valid = false;
+	time_travel_ext_free_until_valid = false;
 	time_travel_ext_waiting++;
 
 	time_travel_ext_req(UM_TIMETRAVEL_WAIT, -1);
@@ -639,7 +648,8 @@ static u64 timer_read(struct clocksource *cs)
 		 * "what do I do next" and onstack event we use to know when
 		 * to return from time_travel_update_time().
 		 */
-		if (!irqs_disabled() && !in_interrupt() && !in_softirq())
+		if (!irqs_disabled() && !in_interrupt() && !in_softirq() &&
+		    !time_travel_ext_waiting)
 			time_travel_update_time(time_travel_time +
 						TIMER_MULTIPLIER,
 						false);
-- 
2.26.2


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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] um: time-travel: actually apply "free-until" optimisation
  2020-12-03 14:36 [PATCH] um: time-travel: actually apply "free-until" optimisation Johannes Berg
@ 2020-12-10 22:28 ` Richard Weinberger
  2020-12-11  7:54   ` Johannes Berg
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Weinberger @ 2020-12-10 22:28 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-um, Johannes Berg

On Thu, Dec 3, 2020 at 3:37 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> From: Johannes Berg <johannes.berg@intel.com>
>
> Due a bug - we never checked the time_travel_ext_free_until value - we
> were always requesting time for every single scheduling. This adds up
> since we make reading time cost 256ns, and it's a fairly common call.
> Fix this.
>
> While at it, also make reading time only cost something when we're not
> currently waiting for our scheduling turn - otherwise things get mixed
> up in a very confusing way. We should never get here, since we're not
> actually running, but it's possible if you stick printk() or such into
> the virtio code that must handle the external interrupts.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  arch/um/kernel/time.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
> index 303565ce8c64..5ac084deac7e 100644
> --- a/arch/um/kernel/time.c
> +++ b/arch/um/kernel/time.c
> @@ -183,6 +183,14 @@ static void time_travel_ext_update_request(unsigned long long time)
>             time == time_travel_ext_prev_request)
>                 return;
>
> +       /*
> +        * if we're running and are allowed to run past the request
> +        * then we don't need to update it either
> +        */
> +       if (!time_travel_ext_waiting && time_travel_ext_free_until_valid &&
> +           time < time_travel_ext_free_until)
> +               return;
> +
>         time_travel_ext_prev_request = time;
>         time_travel_ext_prev_request_valid = true;
>         time_travel_ext_req(UM_TIMETRAVEL_REQUEST, time);
> @@ -217,6 +225,7 @@ static void time_travel_ext_wait(bool idle)
>         };
>
>         time_travel_ext_prev_request_valid = false;
> +       time_travel_ext_free_until_valid = false;
>         time_travel_ext_waiting++;
>
>         time_travel_ext_req(UM_TIMETRAVEL_WAIT, -1);
> @@ -639,7 +648,8 @@ static u64 timer_read(struct clocksource *cs)
>                  * "what do I do next" and onstack event we use to know when
>                  * to return from time_travel_update_time().
>                  */
> -               if (!irqs_disabled() && !in_interrupt() && !in_softirq())
> +               if (!irqs_disabled() && !in_interrupt() && !in_softirq() &&
> +                   !time_travel_ext_waiting)

This will not build with timetravel support disabled.

>                         time_travel_update_time(time_travel_time +
>                                                 TIMER_MULTIPLIER,
>                                                 false);
> --
> 2.26.2
>
>
> _______________________________________________
> linux-um mailing list
> linux-um@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-um



-- 
Thanks,
//richard

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] um: time-travel: actually apply "free-until" optimisation
  2020-12-10 22:28 ` Richard Weinberger
@ 2020-12-11  7:54   ` Johannes Berg
  0 siblings, 0 replies; 3+ messages in thread
From: Johannes Berg @ 2020-12-11  7:54 UTC (permalink / raw)
  To: Richard Weinberger; +Cc: linux-um

On Thu, 2020-12-10 at 23:28 +0100, Richard Weinberger wrote:
> 
> > -               if (!irqs_disabled() && !in_interrupt() && !in_softirq())
> > +               if (!irqs_disabled() && !in_interrupt() && !in_softirq() &&
> > +                   !time_travel_ext_waiting)
> 
> This will not build with timetravel support disabled.

Ahrg, sorry, will fix.

johannes


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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-12-11  7:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03 14:36 [PATCH] um: time-travel: actually apply "free-until" optimisation Johannes Berg
2020-12-10 22:28 ` Richard Weinberger
2020-12-11  7:54   ` Johannes Berg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.