LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Stephane Eranian <eranian@hpl.hp.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: linux-kernel@vger.kernel.org, venkatesh.pallipadi@intel.com,
	suresh.b.siddha@intel.com, kenneth.w.chen@intel.com,
	tony.luck@intel.com, Andrew Morton <akpm@osdl.org>
Subject: Re: [patch] sched: improve sched_clock() on i686
Date: Wed, 17 Jan 2007 03:05:22 -0800	[thread overview]
Message-ID: <20070117110522.GD18499@frankl.hpl.hp.com> (raw)
In-Reply-To: <20061222121920.GA3809@elte.hu>

Ingo,

I see that this patch made it into -mm, so i am hoping it will
also show up in mainline fairly soon.

Thanks.


On Fri, Dec 22, 2006 at 01:19:20PM +0100, Ingo Molnar wrote:
> 
> * Stephane Eranian <eranian@hpl.hp.com> wrote:
> 
> > The perfmon subsystems needs to compute per-CPU duration. It is using 
> > sched_clock() to provide this information. However, it seems they are 
> > big variations in the way sched_clock() is implemented for each 
> > architectures, especially in the accuracy of the returned value (going 
> > from TSC to jiffies).
> > 
> > Looking at the i386 implementation, it is not so clear to me what the 
> > actual goal of the function is. I was under the impression that this 
> > function was meant to compute per-CPU time deltas. This is how the 
> > scheduler seems to use it.
> > 
> > The x86-64 and i386 implementations are quite different. The i386 
> > comment about NUMA seems to contradict the initial goal of the 
> > function. Why is that?
> 
> it's purely historic - the i686 sched_clock() implementation predates 
> the scheduler's ability to deal with non-synchronous per-CPU clocks. I 
> tried to fix that (a year ago) and it didnt work out - but i've reviewed 
> my old patch and now realize what the mistake was - the patch below 
> should work better.
> 
> 	Ingo
> 
> ---------------------->
> Subject: [patch] sched: improve sched_clock() on i686
> From: Ingo Molnar <mingo@elte.hu>
> 
> this patch cleans up sched_clock() on i686: it will use the TSC if 
> available and falls back to jiffies only if the user asked for it to be 
> disabled via notsc or the CPU calibration code didnt figure out the 
> right cpu_khz.
> 
> this generally makes the scheduler timestamps more finegrained, on all 
> hardware. (the current scheduler is pretty resistant against 
> asynchronous sched_clock() values on different CPUs, it will allow at 
> most up to a jiffy of jitter.)
> 
> also simplify sched_clock()'s check for TSC availability: propagate the 
> desire and ability to use the TSC into the tsc_disable flag, previously 
> this flag only indicated whether the notsc option was passed. This makes 
> the rare low-res sched_clock() codepath a single branch off a 
> read-mostly flag.
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/i386/kernel/tsc.c |   22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> Index: linux/arch/i386/kernel/tsc.c
> ===================================================================
> --- linux.orig/arch/i386/kernel/tsc.c
> +++ linux/arch/i386/kernel/tsc.c
> @@ -108,13 +108,10 @@ unsigned long long sched_clock(void)
>  	unsigned long long this_offset;
>  
>  	/*
> -	 * in the NUMA case we dont use the TSC as they are not
> -	 * synchronized across all CPUs.
> +	 * Fall back to jiffies if there's no TSC available:
>  	 */
> -#ifndef CONFIG_NUMA
> -	if (!cpu_khz || check_tsc_unstable())
> -#endif
> -		/* no locking but a rare wrong value is not a big deal */
> +	if (unlikely(tsc_disable))
> +		/* No locking but a rare wrong value is not a big deal: */
>  		return (jiffies_64 - INITIAL_JIFFIES) * (1000000000 / HZ);
>  
>  	/* read the Time Stamp Counter: */
> @@ -194,13 +191,13 @@ EXPORT_SYMBOL(recalibrate_cpu_khz);
>  void __init tsc_init(void)
>  {
>  	if (!cpu_has_tsc || tsc_disable)
> -		return;
> +		goto out_no_tsc;
>  
>  	cpu_khz = calculate_cpu_khz();
>  	tsc_khz = cpu_khz;
>  
>  	if (!cpu_khz)
> -		return;
> +		goto out_no_tsc;
>  
>  	printk("Detected %lu.%03lu MHz processor.\n",
>  				(unsigned long)cpu_khz / 1000,
> @@ -208,6 +205,15 @@ void __init tsc_init(void)
>  
>  	set_cyc2ns_scale(cpu_khz);
>  	use_tsc_delay();
> +	return;
> +
> +out_no_tsc:
> +	/*
> +	 * Set the tsc_disable flag if there's no TSC support, this
> +	 * makes it a fast flag for the kernel to see whether it
> +	 * should be using the TSC.
> +	 */
> +	tsc_disable = 1;
>  }
>  
>  #ifdef CONFIG_CPU_FREQ

-- 

-Stephane

  parent reply	other threads:[~2007-01-17 11:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-22 10:43 sched_clock() on i386 Stephane Eranian
2006-12-22 12:19 ` [patch] sched: improve sched_clock() on i686 Ingo Molnar
2006-12-23 10:27   ` Ingo Molnar
2007-01-17 11:05   ` Stephane Eranian [this message]
2006-12-23 15:53 ` sched_clock() on i386 Daniel Walker
2007-01-03 14:36   ` Stephane Eranian

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=20070117110522.GD18499@frankl.hpl.hp.com \
    --to=eranian@hpl.hp.com \
    --cc=akpm@osdl.org \
    --cc=kenneth.w.chen@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=suresh.b.siddha@intel.com \
    --cc=tony.luck@intel.com \
    --cc=venkatesh.pallipadi@intel.com \
    --subject='Re: [patch] sched: improve sched_clock() on i686' \
    /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

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 NNTP newsgroup(s).