public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Bill Davidsen <davidsen@tmr.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: davidsen@posidon.tmr.com,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ian Kent <raven@themaw.net>, Chuck Ebbert <cebbert@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [patch] CFS scheduler, -v19
Date: Thu, 19 Jul 2007 13:06:54 -0400	[thread overview]
Message-ID: <469F9A2E.5020107@tmr.com> (raw)
In-Reply-To: <20070719143214.GA6387@elte.hu>

Ingo Molnar wrote:
> * Bill Davidsen <davidsen@tmr.com> wrote:
> 
>>> Does the patch below help?

Doesn't seem to apply against 2.6.22.1, I'm trying 2.6.22.6 as soon as I 
recreate it.

>> Spectacularly no! With this patch the "glitch1" script with multiple 
>> scrolling windows has all xterms and glxgears stop totally dead for 
>> ~200ms once per second. I didn't properly test anything else after 
>> that.
> 
> Bill, could you try the patch below - does it fix the automount problem, 
> without introducing new problems?
> 
> 	Ingo
> 
> --------------->
> Subject: time: introduce xtime_seconds
> From: Ingo Molnar <mingo@elte.hu>
> 
> introduce the xtime_seconds optimization. This is a read-mostly 
> low-resolution time source available to sys_time() and kernel-internal 
> use. This variable is kept uptodate atomically, and it's monotically 
> increased, every time some time interface constructs an xtime-alike time 
> result that overflows the seconds value. (it's updated from the timer 
> interrupt as well)
> 
> this way high-resolution time results update their seconds component at 
> the same time sys_time() does it:
> 
>  1184858832999989000
>  1184858832000000000
>  1184858832999992000
>  1184858832000000000
>  1184858832999996000
>  1184858832000000000
>  1184858832999999000
>  1184858832000000000
>  1184858833000003000
>  1184858833000000000
>  1184858833000006000
>  1184858833000000000
>  1184858833000009000
>  1184858833000000000
> 
>  [ these are nsec time results from alternating calls to sys_time() and 
>    sys_gettimeofday(), recorded at the seconds boundary. ]
> 
> instead of the previous (non-coherent) behavior:
> 
>  1184848950999987000
>  1184848950000000000
>  1184848950999990000
>  1184848950000000000
>  1184848950999994000
>  1184848950000000000
>  1184848950999997000
>  1184848950000000000
>  1184848951000001000
>  1184848950000000000
>  1184848951000005000
>  1184848950000000000
>  1184848951000008000
>  1184848950000000000
>  1184848951000011000
>  1184848950000000000
>  1184848951000015000
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  include/linux/time.h      |   13 +++++++++++--
>  kernel/time.c             |   25 ++++++-------------------
>  kernel/time/timekeeping.c |   28 ++++++++++++++++++++++++----
>  3 files changed, 41 insertions(+), 25 deletions(-)
> 
> Index: linux/include/linux/time.h
> ===================================================================
> --- linux.orig/include/linux/time.h
> +++ linux/include/linux/time.h
> @@ -91,19 +91,28 @@ static inline struct timespec timespec_s
>  extern struct timespec xtime;
>  extern struct timespec wall_to_monotonic;
>  extern seqlock_t xtime_lock __attribute__((weak));
> +extern unsigned long xtime_seconds;
>  
>  extern unsigned long read_persistent_clock(void);
>  void timekeeping_init(void);
>  
> +extern void __update_xtime_seconds(unsigned long new_xtime_seconds);
> +
> +static inline void update_xtime_seconds(unsigned long new_xtime_seconds)
> +{
> +	if (unlikely((long)(new_xtime_seconds - xtime_seconds) > 0))
> +		__update_xtime_seconds(new_xtime_seconds);
> +}
> +
>  static inline unsigned long get_seconds(void)
>  {
> -	return xtime.tv_sec;
> +	return xtime_seconds;
>  }
>  
>  struct timespec current_kernel_time(void);
>  
>  #define CURRENT_TIME		(current_kernel_time())
> -#define CURRENT_TIME_SEC	((struct timespec) { xtime.tv_sec, 0 })
> +#define CURRENT_TIME_SEC	((struct timespec) { xtime_seconds, 0 })
>  
>  extern void do_gettimeofday(struct timeval *tv);
>  extern int do_settimeofday(struct timespec *tv);
> Index: linux/kernel/time.c
> ===================================================================
> --- linux.orig/kernel/time.c
> +++ linux/kernel/time.c
> @@ -58,11 +58,10 @@ EXPORT_SYMBOL(sys_tz);
>  asmlinkage long sys_time(time_t __user * tloc)
>  {
>  	/*
> -	 * We read xtime.tv_sec atomically - it's updated
> -	 * atomically by update_wall_time(), so no need to
> -	 * even read-lock the xtime seqlock:
> +	 * We read xtime_seconds atomically - it's updated
> +	 * atomically by update_xtime_seconds():
>  	 */
> -	time_t i = xtime.tv_sec;
> +	time_t i = xtime_seconds;
>  
>  	smp_rmb(); /* sys_time() results are coherent */
>  
> @@ -226,11 +225,11 @@ inline struct timespec current_kernel_ti
>  
>  	do {
>  		seq = read_seqbegin(&xtime_lock);
> -		
> +
>  		now = xtime;
>  	} while (read_seqretry(&xtime_lock, seq));
>  
> -	return now; 
> +	return now;
>  }
>  
>  EXPORT_SYMBOL(current_kernel_time);
> @@ -377,19 +376,7 @@ void do_gettimeofday (struct timeval *tv
>  	tv->tv_sec = sec;
>  	tv->tv_usec = usec;
>  
> -	/*
> -	 * Make sure xtime.tv_sec [returned by sys_time()] always
> -	 * follows the gettimeofday() result precisely. This
> -	 * condition is extremely unlikely, it can hit at most
> -	 * once per second:
> -	 */
> -	if (unlikely(xtime.tv_sec != tv->tv_sec)) {
> -		unsigned long flags;
> -
> -		write_seqlock_irqsave(&xtime_lock, flags);
> -		update_wall_time();
> -		write_sequnlock_irqrestore(&xtime_lock, flags);
> -	}
> +	update_xtime_seconds(sec);
>  }
>  EXPORT_SYMBOL(do_gettimeofday);
>  
> Index: linux/kernel/time/timekeeping.c
> ===================================================================
> --- linux.orig/kernel/time/timekeeping.c
> +++ linux/kernel/time/timekeeping.c
> @@ -45,14 +45,27 @@ EXPORT_SYMBOL(xtime_lock);
>   * used instead.
>   */
>  struct timespec xtime __attribute__ ((aligned (16)));
> -struct timespec wall_to_monotonic __attribute__ ((aligned (16)));
> -static unsigned long total_sleep_time;		/* seconds */
> -
>  EXPORT_SYMBOL(xtime);
>  
> +struct timespec wall_to_monotonic __attribute__ ((aligned (16))) __read_mostly;
> +static unsigned long total_sleep_time __read_mostly;	/* seconds */
> +
> +unsigned long xtime_seconds __read_mostly;
> +
> +/* pointer to current clocksource: */
> +static struct clocksource *clock __read_mostly;
>  
> -static struct clocksource *clock; /* pointer to current clocksource */
> +/*
> + * Called when either xtime or any xtime-alike result back to
> + * user-space overflows the xtime_seconds field:
> + */
> +void __update_xtime_seconds(unsigned long new_xtime_seconds)
> +{
> +	unsigned long old_xtime_seconds = xtime_seconds;
>  
> +	if ((long)(new_xtime_seconds - old_xtime_seconds) > 0)
> +		cmpxchg(&xtime_seconds, old_xtime_seconds, new_xtime_seconds);
> +}
>  
>  #ifdef CONFIG_GENERIC_TIME
>  /**
> @@ -100,6 +113,8 @@ static inline void __get_realtime_clock_
>  	} while (read_seqretry(&xtime_lock, seq));
>  
>  	timespec_add_ns(ts, nsecs);
> +
> +	update_xtime_seconds(ts->tv_sec);
>  }
>  
>  /**
> @@ -256,6 +271,8 @@ void __init timekeeping_init(void)
>  	clock->cycle_last = clocksource_read(clock);
>  
>  	xtime.tv_sec = sec;
> +	update_xtime_seconds(sec);
> +
>  	xtime.tv_nsec = 0;
>  	set_normalized_timespec(&wall_to_monotonic,
>  		-xtime.tv_sec, -xtime.tv_nsec);
> @@ -290,6 +307,8 @@ static int timekeeping_resume(struct sys
>  		unsigned long sleep_length = now - timekeeping_suspend_time;
>  
>  		xtime.tv_sec += sleep_length;
> +		update_xtime_seconds(xtime.tv_sec);
> +
>  		wall_to_monotonic.tv_sec -= sleep_length;
>  		total_sleep_time += sleep_length;
>  	}
> @@ -464,6 +483,7 @@ void update_wall_time(void)
>  			clock->xtime_nsec -= (u64)NSEC_PER_SEC << clock->shift;
>  			xtime.tv_sec++;
>  			second_overflow();
> +			update_xtime_seconds(xtime.tv_sec);
>  		}
>  
>  		/* interpolator bits */
> 


  reply	other threads:[~2007-07-19 17:09 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-06 17:33 [patch] CFS scheduler, -v19 Ingo Molnar
2007-07-08 17:46 ` Willy Tarreau
2007-07-09 22:39   ` Ingo Molnar
2007-07-17 21:44     ` Willy Tarreau
2007-07-10  8:08 ` Mike Galbraith
2007-07-11 17:26   ` Bill Davidsen
2007-07-11 20:55     ` Ingo Molnar
2007-07-12 12:41       ` Bill Davidsen
2007-07-13 21:19       ` Bill Davidsen
2007-07-16 21:34         ` Chuck Ebbert
2007-07-16 21:55           ` Ingo Molnar
2007-07-17  4:22             ` Bill Davidsen
2007-07-17  5:01             ` Ian Kent
2007-07-17  7:45               ` Ingo Molnar
2007-07-17 11:17                 ` Ian Kent
2007-07-17 17:16                   ` Ingo Molnar
2007-07-18  1:24                     ` Bill Davidsen
2007-07-18  6:19                       ` Ian Kent
2007-07-17 16:30                 ` Chuck Ebbert
2007-07-17 21:16                 ` David Schwartz
2007-07-18  5:59                   ` Ian Kent
2007-07-18  7:54                     ` Ingo Molnar
2007-07-18 13:50                       ` Bill Davidsen
2007-07-18 17:23                       ` Linus Torvalds
2007-07-18 16:03                 ` Linus Torvalds
2007-07-18 17:31                   ` Ian Kent
2007-07-18 21:37                   ` Bill Davidsen
2007-07-19  8:53                     ` Ingo Molnar
2007-07-19 14:32                     ` Ingo Molnar
2007-07-19 17:06                       ` Bill Davidsen [this message]
2007-07-19 17:10                         ` Ingo Molnar
2007-07-19 17:17                         ` Ingo Molnar
2007-07-19 17:26                         ` Bill Davidsen
2007-07-19 17:42                           ` Ingo Molnar
2007-07-20  2:32                       ` Bill Davidsen
2007-07-19  8:16                   ` Ingo Molnar
2007-07-14 11:34 ` Markus
2007-07-14 15:11   ` Markus
2007-07-16  9:41     ` Ingo Molnar
2007-07-16 17:59       ` Markus
2007-07-17  7:37         ` Ingo Molnar
2007-07-17 13:06           ` Markus
2007-07-17 17:06             ` Ingo Molnar
2007-07-17 17:13               ` Ingo Molnar
2007-07-17 19:42               ` Markus
2007-07-17 20:09                 ` Ingo Molnar
2007-07-17 20:37                   ` Linus Torvalds
2007-07-17 20:43                     ` Ingo Molnar
2007-07-17 22:03                       ` Markus
2007-07-20 22:26                       ` Markus
2007-07-22 11:59                         ` konqueror suddenly vanishing, "konqueror: Fatal IO error: client killed" Ingo Molnar
2007-07-22 14:26                           ` Markus
2007-08-09 17:34                       ` [patch] CFS scheduler, -v19 Markus
2007-08-10  7:46                         ` Ingo Molnar
2007-08-14 17:15                           ` Markus
2007-10-17  0:02                       ` Markus
2007-07-14 17:19 ` Ed Tomlinson
2007-07-15  5:25   ` Mike Galbraith
2007-07-15 12:53     ` Markus
2007-07-15 19:46       ` Mike Galbraith
2007-07-15 21:11         ` Markus
2007-07-16  6:42           ` Mike Galbraith
2007-07-16  8:00     ` Ingo Molnar
2007-07-16  9:17   ` Ingo Molnar
2007-07-16 11:10     ` Ed Tomlinson
  -- strict thread matches above, loose matches on Subject: below --
2007-07-08 20:51 Al Boldi

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=469F9A2E.5020107@tmr.com \
    --to=davidsen@tmr.com \
    --cc=cebbert@redhat.com \
    --cc=davidsen@posidon.tmr.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=raven@themaw.net \
    --cc=torvalds@linux-foundation.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox