Re: [LTP] [PATCH] Fix pass/fail criteria reporting in realtime/func/prio-wake testcase

[Chirag J. <ch...@li...>]

* Ankita Garg <an...@in...> [2008-05-26 11:57:18]:

> Hello,
> 
> We have been seeing failures in the prio-wake testcase for a while now.
> Digged closer into the testcase to find if there was an error lurking in
> the testcase itself. Did hit a few, which this patch aims to fix. But
> the failures persist, indicating that something is amiss in the kernel.
> 
> This patch does the following:
> 
> o Make the output of the test more readable. It is now time ordered.
> 
> o Bases the pass/fail not only on the order in which the waking up
> threads update the 'wakeup.arr' but also on the time they woke up. This
> reduced chances of error. 
I am not convinced that adding a check for wake up time is going to
reduce the chances for error.
Whichever thread wakes up first, takes the lock,updates priority and time
fields.
Hence the time field will always be updated in wakeup order.
So this is not really adding another useful check.
I am ready to be conviced otherwise :)

> o Remove an extra lock and unlock on mutex when updating wakeup.arr
> 
> Signed-off-by: Ankita Garg <an...@in...>
> Signed-off-by: Chandan Kumar B V <cbh...@in...>
> 
> Index: ltp-full-20080430/testcases/realtime/func/prio-wake/prio-wake.c
> ===================================================================
> --- ltp-full-20080430.orig/testcases/realtime/func/prio-wake/prio-wake.c	2008-05-22 11:04:08.000000000 +0530
> +++ ltp-full-20080430/testcases/realtime/func/prio-wake/prio-wake.c	2008-05-23 17:18:22.000000000 +0530
> @@ -95,10 +95,11 @@
>  }
> 
>  struct array {
> -	int *arr;
> +	int *pri;
> +	int *time;
>  	int counter;
>  };
> -struct array wakeup = { NULL , 0 };
> +struct array wakeup = { NULL , NULL , 0 };
> 
>  void *master_thread(void* arg)
>  {
> @@ -111,6 +112,7 @@
> 
>  	start = rt_gettime() - beginrun;
> 
> +	printf("%08lld us: Master thread about to wake the workers\n", start/NS_PER_US);
>  	/* start the children threads */
>  	rc = pthread_mutex_lock(&mutex);
>  	rc = pthread_cond_broadcast(&cond);
> @@ -118,7 +120,6 @@
> 
>  	while (running_threads > 0)
>  		sleep(1);
> -	printf("%08lld us: Master thread about to wake the workers\n", start/NS_PER_US);
>  	return NULL;
>  }
> 
> @@ -141,24 +142,23 @@
>  	}
> 
>  	start = rt_gettime() - beginrun;
> +	printf("%08lld us: RealtimeThread-%03d pri %03d started\n", start/NS_PER_US, j, mypri);
> 
>  	rc = pthread_mutex_lock(&mutex);
>  	running_threads++;
>  	rc = pthread_cond_wait(&cond, &mutex);
> 
>  	wake = rt_gettime() - beginrun;
> -	rc = pthread_mutex_unlock(&mutex);
> -
> -	rc = pthread_mutex_lock(&mutex);
>  	running_threads--;
> -	wakeup.arr[wakeup.counter++] = mypri;
> +	wakeup.pri[wakeup.counter] = mypri;
> +	wakeup.time[wakeup.counter++] = wake;
> +	printf("%08lld us: RealtimeThread-%03d pri %03d awake\n", wake/NS_PER_US, j, mypri);
> +
>  	rc = pthread_mutex_unlock(&mutex);
> 
>  	/* wait for all threads to quit */
>  	while (running_threads > 0)
>  		sleep(1);
> -	printf("%08lld us: RealtimeThread-%03d pri %03d started\n", start/NS_PER_US, j, mypri);
> -	printf("%08lld us: RealtimeThread-%03d pri %03d awake\n", wake/NS_PER_US, j, mypri);
>  	return NULL;
>  }
> 
> @@ -183,7 +183,8 @@
>  		numcpus = sysconf(_SC_NPROCESSORS_ONLN);
>  		rt_threads = numcpus;
>  	}
> -	wakeup.arr = (int *)malloc(rt_threads * sizeof(int));
> +	wakeup.pri = (int *)malloc(rt_threads * sizeof(int));
> +	wakeup.time = (int *)malloc(rt_threads * sizeof(int));
>  	wakeup.counter = 0;
>  	printf("\n-----------------------\n");
>  	printf("Priority Ordered Wakeup\n");
> @@ -211,9 +212,9 @@
> 
>  	printf("\nCriteria: Threads should be woken up in priority order\n");
> 
> -	for (i = 0; i < wakeup.counter; i++) {
> -		if (wakeup.arr[i] < wakeup.arr[i+1]) {
> -			printf("FAIL: Thread %d woken before %d\n", wakeup.arr[i], wakeup.arr[i+1]);
> +	for (i = 0; i < (wakeup.counter-1); i++) {
> +		if (wakeup.pri[i] < wakeup.pri[i+1] && wakeup.time[i] < wakeup.time[i+1]) {
> +			printf("FAIL: Thread %d woken before %d\n", wakeup.pri[i], wakeup.pri[i+1]);
>  			ret++;
>  		}
>  	}
> 
> -- 
> Regards,
> Ankita Garg (an...@in...)
> Linux Technology Center
> IBM India Systems & Technology Labs, 
> Bangalore, India   

-- 
Cheers,
Chirag Jog