LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH] UML - Fix !NO_HZ busy-loop
@ 2007-11-27 20:03 Jeff Dike
  2007-11-27 23:01 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Dike @ 2007-11-27 20:03 UTC (permalink / raw
  To: Andrew Morton; +Cc: LKML, uml-devel, Miklos Szeredi

[ This one needs to get into 2.6.24 ]

With NO_HZ disabled, the UML idle loop effectively becomes a busy
loop, as it will sleep for no time.

The cause was forgetting to restart the tick after waking up from
sleep.  It was disabled before sleeping, and the remaining time used
as the interval to sleep.  So, the tick needs to be restarted when
nanosleep finishes.

This is done by introducing after_sleep_interval, which is empty in
the NO_HZ case, but which sets the tick starting in the !NO_HZ case.

Signed-off-by: Jeff Dike <jdike@linux.intel.com>
---
 arch/um/os-Linux/time.c |   54 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 3 deletions(-)

Index: linux-2.6.22/arch/um/os-Linux/time.c
===================================================================
--- linux-2.6.22.orig/arch/um/os-Linux/time.c	2007-11-14 10:33:29.000000000 -0500
+++ linux-2.6.22/arch/um/os-Linux/time.c	2007-11-26 15:50:46.000000000 -0500
@@ -59,7 +59,7 @@ long long disable_timer(void)
 {
 	struct itimerval time = ((struct itimerval) { { 0, 0 }, { 0, 0 } });
 
-	if(setitimer(ITIMER_VIRTUAL, &time, &time) < 0)
+	if (setitimer(ITIMER_VIRTUAL, &time, &time) < 0)
 		printk(UM_KERN_ERR "disable_timer - setitimer failed, "
 		       "errno = %d\n", errno);
 
@@ -74,13 +74,61 @@ long long os_nsecs(void)
 	return timeval_to_ns(&tv);
 }
 
+#ifdef UML_CONFIG_NO_HZ
+static int after_sleep_interval(struct timespec *ts)
+{
+}
+#else
+static inline long long timespec_to_us(const struct timespec *ts)
+{
+	return ((long long) ts->tv_sec * UM_USEC_PER_SEC) +
+		ts->tv_nsec / UM_NSEC_PER_USEC;
+}
+
+static int after_sleep_interval(struct timespec *ts)
+{
+	int usec = UM_USEC_PER_SEC / UM_HZ;
+	long long start_usecs = timespec_to_us(ts);
+	struct timeval tv;
+	struct itimerval interval;
+
+	/*
+	 * It seems that rounding can increase the value returned from
+	 * setitimer to larger than the one passed in.  Over time,
+	 * this will cause the remaining time to be greater than the
+	 * tick interval.  If this happens, then just reduce the first
+	 * tick to the interval value.
+	 */
+	if (start_usecs > usec)
+		start_usecs = usec;
+	tv = ((struct timeval) { .tv_sec  = start_usecs / UM_USEC_PER_SEC,
+				 .tv_usec = start_usecs % UM_USEC_PER_SEC });
+	interval = ((struct itimerval) { { 0, usec }, tv });
+
+	if (setitimer(ITIMER_VIRTUAL, &interval, NULL) == -1)
+		return -errno;
+
+	return 0;
+}
+#endif
+
 extern void alarm_handler(int sig, struct sigcontext *sc);
 
 void idle_sleep(unsigned long long nsecs)
 {
-	struct timespec ts = { .tv_sec	= nsecs / UM_NSEC_PER_SEC,
-			       .tv_nsec = nsecs % UM_NSEC_PER_SEC };
+	struct timespec ts;
+
+	/*
+	 * nsecs can come in as zero, in which case, this starts a
+	 * busy loop.  To prevent this, reset nsecs to the tick
+	 * interval if it is zero.
+	 */
+	if (nsecs == 0)
+		nsecs = UM_NSEC_PER_SEC / UM_HZ;
+	ts = ((struct timespec) { .tv_sec	= nsecs / UM_NSEC_PER_SEC,
+				  .tv_nsec	= nsecs % UM_NSEC_PER_SEC });
 
 	if (nanosleep(&ts, &ts) == 0)
 		alarm_handler(SIGVTALRM, NULL);
+	after_sleep_interval(&ts);
 }

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

* Re: [PATCH] UML - Fix !NO_HZ busy-loop
  2007-11-27 20:03 [PATCH] UML - Fix !NO_HZ busy-loop Jeff Dike
@ 2007-11-27 23:01 ` Andrew Morton
  2007-11-28  0:09   ` Jeff Dike
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2007-11-27 23:01 UTC (permalink / raw
  To: Jeff Dike; +Cc: linux-kernel, user-mode-linux-devel, miklos

On Tue, 27 Nov 2007 15:03:47 -0500
Jeff Dike <jdike@addtoit.com> wrote:

> [ This one needs to get into 2.6.24 ]
> 
> With NO_HZ disabled, the UML idle loop effectively becomes a busy
> loop, as it will sleep for no time.
> 
> The cause was forgetting to restart the tick after waking up from
> sleep.  It was disabled before sleeping, and the remaining time used
> as the interval to sleep.  So, the tick needs to be restarted when
> nanosleep finishes.
> 
> This is done by introducing after_sleep_interval, which is empty in
> the NO_HZ case, but which sets the tick starting in the !NO_HZ case.
> 
> Signed-off-by: Jeff Dike <jdike@linux.intel.com>
> ---
>  arch/um/os-Linux/time.c |   54 +++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 51 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6.22/arch/um/os-Linux/time.c
> ===================================================================
> --- linux-2.6.22.orig/arch/um/os-Linux/time.c	2007-11-14 10:33:29.000000000 -0500
> +++ linux-2.6.22/arch/um/os-Linux/time.c	2007-11-26 15:50:46.000000000 -0500
> @@ -59,7 +59,7 @@ long long disable_timer(void)
>  {
>  	struct itimerval time = ((struct itimerval) { { 0, 0 }, { 0, 0 } });
>  
> -	if(setitimer(ITIMER_VIRTUAL, &time, &time) < 0)
> +	if (setitimer(ITIMER_VIRTUAL, &time, &time) < 0)
>  		printk(UM_KERN_ERR "disable_timer - setitimer failed, "
>  		       "errno = %d\n", errno);
>  
> @@ -74,13 +74,61 @@ long long os_nsecs(void)
>  	return timeval_to_ns(&tv);
>  }
>  
> +#ifdef UML_CONFIG_NO_HZ

Nothing ever defines this?

> +static int after_sleep_interval(struct timespec *ts)
> +{
> +}
> +#else
> +static inline long long timespec_to_us(const struct timespec *ts)
> +{
> +	return ((long long) ts->tv_sec * UM_USEC_PER_SEC) +
> +		ts->tv_nsec / UM_NSEC_PER_USEC;
> +}
> +
> +static int after_sleep_interval(struct timespec *ts)
> +{
> +	int usec = UM_USEC_PER_SEC / UM_HZ;
> +	long long start_usecs = timespec_to_us(ts);
> +	struct timeval tv;
> +	struct itimerval interval;
> +
> +	/*
> +	 * It seems that rounding can increase the value returned from
> +	 * setitimer to larger than the one passed in.  Over time,
> +	 * this will cause the remaining time to be greater than the
> +	 * tick interval.  If this happens, then just reduce the first
> +	 * tick to the interval value.
> +	 */
> +	if (start_usecs > usec)
> +		start_usecs = usec;
> +	tv = ((struct timeval) { .tv_sec  = start_usecs / UM_USEC_PER_SEC,
> +				 .tv_usec = start_usecs % UM_USEC_PER_SEC });
> +	interval = ((struct itimerval) { { 0, usec }, tv });
> +
> +	if (setitimer(ITIMER_VIRTUAL, &interval, NULL) == -1)
> +		return -errno;
> +
> +	return 0;
> +}
> +#endif
> +
>  extern void alarm_handler(int sig, struct sigcontext *sc);
>  
>  void idle_sleep(unsigned long long nsecs)
>  {
> -	struct timespec ts = { .tv_sec	= nsecs / UM_NSEC_PER_SEC,
> -			       .tv_nsec = nsecs % UM_NSEC_PER_SEC };
> +	struct timespec ts;
> +
> +	/*
> +	 * nsecs can come in as zero, in which case, this starts a
> +	 * busy loop.  To prevent this, reset nsecs to the tick
> +	 * interval if it is zero.
> +	 */
> +	if (nsecs == 0)
> +		nsecs = UM_NSEC_PER_SEC / UM_HZ;
> +	ts = ((struct timespec) { .tv_sec	= nsecs / UM_NSEC_PER_SEC,
> +				  .tv_nsec	= nsecs % UM_NSEC_PER_SEC });
>  
>  	if (nanosleep(&ts, &ts) == 0)
>  		alarm_handler(SIGVTALRM, NULL);
> +	after_sleep_interval(&ts);
>  }

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

* Re: [PATCH] UML - Fix !NO_HZ busy-loop
  2007-11-27 23:01 ` Andrew Morton
@ 2007-11-28  0:09   ` Jeff Dike
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Dike @ 2007-11-28  0:09 UTC (permalink / raw
  To: Andrew Morton; +Cc: linux-kernel, user-mode-linux-devel, miklos

On Tue, Nov 27, 2007 at 03:01:08PM -0800, Andrew Morton wrote:
> > +#ifdef UML_CONFIG_NO_HZ
> 
> Nothing ever defines this?

$ grep HZ obj/arch/um/include/uml-config.h 
#define UML_CONFIG_NO_HZ 1

uml-config.h is auto-generated to provide the config definitions to the
userspace side of UML.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

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

end of thread, other threads:[~2007-11-28  0:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-27 20:03 [PATCH] UML - Fix !NO_HZ busy-loop Jeff Dike
2007-11-27 23:01 ` Andrew Morton
2007-11-28  0:09   ` Jeff Dike

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).