All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Jeff Dike <jdike@addtoit.com>
Cc: linux-kernel@vger.kernel.org,
	user-mode-linux-devel@lists.sourceforge.net, miklos@szeredi.hu
Subject: Re: [uml-devel] [PATCH] UML - Fix !NO_HZ busy-loop
Date: Tue, 27 Nov 2007 15:01:08 -0800	[thread overview]
Message-ID: <20071127150108.fb94aaca.akpm@linux-foundation.org> (raw)
In-Reply-To: <20071127200347.GA8847@c2.user-mode-linux.org>

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);
>  }

-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: Jeff Dike <jdike@addtoit.com>
Cc: linux-kernel@vger.kernel.org,
	user-mode-linux-devel@lists.sourceforge.net, miklos@szeredi.hu
Subject: Re: [PATCH] UML - Fix !NO_HZ busy-loop
Date: Tue, 27 Nov 2007 15:01:08 -0800	[thread overview]
Message-ID: <20071127150108.fb94aaca.akpm@linux-foundation.org> (raw)
In-Reply-To: <20071127200347.GA8847@c2.user-mode-linux.org>

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);
>  }

  reply	other threads:[~2007-11-27 23:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-27 20:03 [uml-devel] [PATCH] UML - Fix !NO_HZ busy-loop Jeff Dike
2007-11-27 20:03 ` Jeff Dike
2007-11-27 23:01 ` Andrew Morton [this message]
2007-11-27 23:01   ` Andrew Morton
2007-11-28  0:09   ` [uml-devel] " Jeff Dike
2007-11-28  0:09     ` Jeff Dike

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20071127150108.fb94aaca.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=jdike@addtoit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=user-mode-linux-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.