lkml.org 
[lkml]   [2007]   [Nov]   [16]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [patch] x86: make delay_tsc() preemptible again
    FromPeter Zijlstra <>
    DateFri, 16 Nov 2007 10:39:02 +0100
    On Fri, 2007-11-16 at 09:47 +0100, Ingo Molnar wrote:
    > * Ingo Molnar <mingo@elte.hu> wrote:
    > 
    > > but that should not be needed in this case. Why doesnt the TSC using 
    > > delay loop simply poll the CPU it is on and fix up the TSC?
    > 
    > something like the patch below.
    > 
    > 	Ingo
    > 
    > --------------->
    > Subject: x86: make delay_tsc() preemptible again
    > From: Ingo Molnar <mingo@elte.hu>
    > 
    > make delay_tsc() preemptible again.
    > 
    > Signed-off-by: Ingo Molnar <mingo@elte.hu>
    > ---
    >  arch/x86/lib/delay_32.c |   28 +++++++++++++++++++++++-----
    >  arch/x86/lib/delay_64.c |   30 ++++++++++++++++++++++++------
    >  2 files changed, 47 insertions(+), 11 deletions(-)
    > 
    > Index: linux/arch/x86/lib/delay_32.c
    > ===================================================================
    > --- linux.orig/arch/x86/lib/delay_32.c
    > +++ linux/arch/x86/lib/delay_32.c
    > @@ -38,17 +38,35 @@ static void delay_loop(unsigned long loo
    >  		:"0" (loops));
    >  }
    > 
    > -/* TSC based delay: */
    > +/*
    > + * TSC based delay:
    > + *
    > + * We are careful about preemption as TSC's are per-CPU.
    > + */
    >  static void delay_tsc(unsigned long loops)
    >  {
    > -	unsigned long bclock, now;
    > +	unsigned long prev, now;
    > +	long left = loops;
    > +	int prev_cpu, cpu;
    > 
    > -	preempt_disable();		/* TSC's are per-cpu */
    > -	rdtscl(bclock);
    > +	preempt_disable();
    > +	rdtscl(prev);
    >  	do {
    > +		prev_cpu = smp_processor_id();
    >  		rep_nop();
    > +		preempt_enable();
    
    Why not have the rep_nop() here between the enable, and disable ?
    
    > +
    > +		preempt_disable();
    > +		cpu = smp_processor_id();
    >  		rdtscl(now);
    > -	} while ((now-bclock) < loops);
    > +		/*
    > +		 * If we preempted we skip this small amount of time:
                               ^ migrated, perhaps?
    
    > +		 */
    > +		if (prev_cpu != cpu)
    > +			prev = now;
    > +		left -= now - prev;
    > +		prev = now;
    > +	} while (left > 0);
    >  	preempt_enable();
    >  }
    
    
    Otherwise, looks like a very nice patch :-)
    
    
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at  http://www.tux.org/lkml/
    
    
    \
     
     \ /
      Last update: 2007-11-16 10:41    [from the cache]
    ©2003-2008