[PATCH v7 1/5] um: simplify os_idle_sleep() and sleep longer

[Anton Ivanov]

On 02/12/2020 19:58, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg at intel.com>
> 
> There really is no reason to pass the amount of time we should
> sleep, especially since it's just hard-coded to one second.
> 
> Additionally, one second isn't really all that long, and as we
> are expecting to be woken up by a signal, we can sleep longer
> and avoid doing some work every second, so replace the current
> clock_nanosleep() with just an empty select() that can _only_
> be woken up by a signal.
> 
> We can also remove the deliver_alarm() since we don't need to
> do that when we got e.g. SIGIO that woke us up, and if we got
> SIGALRM the signal handler will actually (have) run, so it's
> just unnecessary extra work.
> 
> Similarly, in time-travel mode, just program the wakeup event
> from idle to be S64_MAX, which is basically the most you could
> ever simulate to. Of course, you should already have an event
> in the list that's earlier and will cause a wakeup, normally
> that's the regular timer interrupt, though in suspend it may
> (later) also be an RTC event. Since actually getting to this
> point would be a bug and you can't ever get out again, panic()
> on it in the time control code.
> 
> Signed-off-by: Johannes Berg <johannes.berg at intel.com>
> ---
> v7:
>   - use pause()
> ---
>   arch/um/include/linux/time-internal.h |  4 ++--
>   arch/um/include/shared/os.h           |  2 +-
>   arch/um/kernel/process.c              | 11 ++++-------
>   arch/um/kernel/time.c                 | 12 ++++++++++--
>   arch/um/os-Linux/time.c               | 17 ++++-------------
>   5 files changed, 21 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/um/include/linux/time-internal.h b/arch/um/include/linux/time-internal.h
> index f3b03d39a854..68e45e950137 100644
> --- a/arch/um/include/linux/time-internal.h
> +++ b/arch/um/include/linux/time-internal.h
> @@ -28,7 +28,7 @@ struct time_travel_event {
>   
>   extern enum time_travel_mode time_travel_mode;
>   
> -void time_travel_sleep(unsigned long long duration);
> +void time_travel_sleep(void);
>   
>   static inline void
>   time_travel_set_event_fn(struct time_travel_event *e,
> @@ -60,7 +60,7 @@ struct time_travel_event {
>   
>   #define time_travel_mode TT_MODE_OFF
>   
> -static inline void time_travel_sleep(unsigned long long duration)
> +static inline void time_travel_sleep(void)
>   {
>   }
>   
> diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
> index e2bb7e488d59..0f7fb8bad728 100644
> --- a/arch/um/include/shared/os.h
> +++ b/arch/um/include/shared/os.h
> @@ -256,7 +256,7 @@ extern void os_warn(const char *fmt, ...)
>   	__attribute__ ((format (printf, 1, 2)));
>   
>   /* time.c */
> -extern void os_idle_sleep(unsigned long long nsecs);
> +extern void os_idle_sleep(void);
>   extern int os_timer_create(void);
>   extern int os_timer_set_interval(unsigned long long nsecs);
>   extern int os_timer_one_shot(unsigned long long nsecs);
> diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c
> index 3bed09538dd9..0686fabba576 100644
> --- a/arch/um/kernel/process.c
> +++ b/arch/um/kernel/process.c
> @@ -204,13 +204,10 @@ void initial_thread_cb(void (*proc)(void *), void *arg)
>   
>   static void um_idle_sleep(void)
>   {
> -	unsigned long long duration = UM_NSEC_PER_SEC;
> -
> -	if (time_travel_mode != TT_MODE_OFF) {
> -		time_travel_sleep(duration);
> -	} else {
> -		os_idle_sleep(duration);
> -	}
> +	if (time_travel_mode != TT_MODE_OFF)
> +		time_travel_sleep();
> +	else
> +		os_idle_sleep();
>   }
>   
>   void arch_cpu_idle(void)
> diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
> index 8dafc3f2add4..8e8eb8ba04a4 100644
> --- a/arch/um/kernel/time.c
> +++ b/arch/um/kernel/time.c
> @@ -46,6 +46,9 @@ static void time_travel_set_time(unsigned long long ns)
>   	if (unlikely(ns < time_travel_time))
>   		panic("time-travel: time goes backwards %lld -> %lld\n",
>   		      time_travel_time, ns);
> +	else if (unlikely(ns >= S64_MAX))
> +		panic("The system was going to sleep forever, aborting");
> +
>   	time_travel_time = ns;
>   }
>   
> @@ -399,9 +402,14 @@ static void time_travel_oneshot_timer(struct time_travel_event *e)
>   	deliver_alarm();
>   }
>   
> -void time_travel_sleep(unsigned long long duration)
> +void time_travel_sleep(void)
>   {
> -	unsigned long long next = time_travel_time + duration;
> +	/*
> +	 * Wait "forever" (using S64_MAX because there are some potential
> +	 * wrapping issues, especially with the current TT_MODE_EXTERNAL
> +	 * controller application.
> +	 */
> +	unsigned long long next = S64_MAX;
>   
>   	if (time_travel_mode == TT_MODE_BASIC)
>   		os_timer_disable();
> diff --git a/arch/um/os-Linux/time.c b/arch/um/os-Linux/time.c
> index 90f6de224c70..a61cbf73a179 100644
> --- a/arch/um/os-Linux/time.c
> +++ b/arch/um/os-Linux/time.c
> @@ -7,6 +7,7 @@
>    */
>   
>   #include <stddef.h>
> +#include <unistd.h>
>   #include <errno.h>
>   #include <signal.h>
>   #include <time.h>
> @@ -99,19 +100,9 @@ long long os_nsecs(void)
>   }
>   
>   /**
> - * os_idle_sleep() - sleep for a given time of nsecs
> - * @nsecs: nanoseconds to sleep
> + * os_idle_sleep() - sleep until interrupted
>    */
> -void os_idle_sleep(unsigned long long nsecs)
> +void os_idle_sleep(void)
>   {
> -	struct timespec ts = {
> -		.tv_sec  = nsecs / UM_NSEC_PER_SEC,
> -		.tv_nsec = nsecs % UM_NSEC_PER_SEC
> -	};
> -
> -	/*
> -	 * Relay the signal if clock_nanosleep is interrupted.
> -	 */
> -	if (clock_nanosleep(CLOCK_MONOTONIC, 0, &ts, NULL))
> -		deliver_alarm();
> +	pause();
>   }
> 

Acked-By: Anton Ivanov <anton.ivanov at cambridgegreys.com>

-- 
Anton R. Ivanov
https://www.kot-begemot.co.uk/