Re: [PATCH] fix race in TSC test

Mathieu Desnoyers <[email protected]> Tue, 19 Feb 2008 15:47:19 -0500
Newsgroups gmane.linux.kernel.tracing
Message-ID <20080219204719.GB16701@Krystal>
* Jan Kiszka ([email protected]) wrote:
> There is a nasty (but rare, unless you use QEMU) race in
> test_tsc_synchronization: If the slave CPU is stuck on the final loop in
> test_sync and then gets distracted for a while (NMI watchdog or -that's
> how I caught this- QEMU's SMP scheduler), the master could happen to set
> wait_end_sync and, shortly after that, reset it again in the next main
> loop. We thus end up with a deadlock.
> 
> Patch below fixes this by synchronizing the master on the slave to have
> left test_sync.
> 

Good catch. Will apply, thanks!

> Signed-off-by: Jan Kiszka <[email protected]>
> 
> ---
>  ltt/ltt-test-tsc.c |   14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> Index: b/ltt/ltt-test-tsc.c
> ===================================================================
> --- a/ltt/ltt-test-tsc.c
> +++ b/ltt/ltt-test-tsc.c
> @@ -59,7 +59,8 @@ static void test_sync(void *arg)
>  	per_cpu(wait_end_sync, smp_processor_id())--;
>  	do {
>  		smp_mb();
> -	} while (unlikely(per_cpu(wait_end_sync, wait_cpu) > 0));
> +	} while (unlikely(per_cpu(wait_end_sync, wait_cpu) > 1));
> +	per_cpu(wait_end_sync, smp_processor_id())--;
>  	local_irq_restore(flags);
>  }
>  
> @@ -88,11 +89,18 @@ static int test_tsc_synchronization(void
>  			 */
>  			per_cpu(wait_sync, master) = 2;
>  			per_cpu(wait_sync, cpu) = 2;
> -			per_cpu(wait_end_sync, master) = 1;
> -			per_cpu(wait_end_sync, cpu) = 1;
> +			per_cpu(wait_end_sync, master) = 2;
> +			per_cpu(wait_end_sync, cpu) = 2;
>  			smp_call_function_single(cpu, test_sync,
>  						(void *)master, 1, 0);
>  			test_sync((void *)cpu);
> +			/*
> +			 * Wait until slave is done so that we don't overwrite
> +			 * wait_end_sync prematurely.
> +			 */
> +			while (unlikely(per_cpu(wait_end_sync, cpu) > 0))
> +				cpu_relax();
> +
>  			diff = abs(per_cpu(tsc_count, cpu)
>  				- per_cpu(tsc_count, master));
>  			best_loop = min(best_loop, diff);
> _______________________________________________
> Ltt-dev mailing list
> [email protected]
> http://listserv.shafik.org/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68