diff mbox series

[rt-tests,v5,01/13] cyclictest: Move thread data to struct thread_param

Message ID 20210210175118.19709-2-dwagner@suse.de
State New
Headers show
Series Generate machine-readable output | expand

Commit Message

Daniel Wagner Feb. 10, 2021, 5:51 p.m. UTC
Group thread realated data such as thread ID to struct thread_param.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 src/cyclictest/cyclictest.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

Comments

John Kacur Feb. 17, 2021, 3:52 a.m. UTC | #1
On Wed, 10 Feb 2021, Daniel Wagner wrote:

> Group thread realated data such as thread ID to struct thread_param.

> 

> Signed-off-by: Daniel Wagner <dwagner@suse.de>

> ---

>  src/cyclictest/cyclictest.c | 34 +++++++++++++++++-----------------

>  1 file changed, 17 insertions(+), 17 deletions(-)

> 

> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c

> index c4b2369bee6b..7c45732c1553 100644

> --- a/src/cyclictest/cyclictest.c

> +++ b/src/cyclictest/cyclictest.c

> @@ -113,6 +113,9 @@ static char *policyname(int policy);

>  

>  /* Struct to transfer parameters to the thread */

>  struct thread_param {

> +	pthread_t thread;

> +	int threadstarted;

> +	int tid;

>  	int prio;

>  	int policy;

>  	int mode;

> @@ -141,9 +144,6 @@ struct thread_stat {

>  	long *smis;

>  	long *hist_array;

>  	long *outliers;

> -	pthread_t thread;

> -	int threadstarted;

> -	int tid;

>  	long reduce;

>  	long redmax;

>  	long cycleofmax;

> @@ -530,7 +530,7 @@ static void *timerthread(void *param)

>  	interval.tv_sec = par->interval / USEC_PER_SEC;

>  	interval.tv_nsec = (par->interval % USEC_PER_SEC) * 1000;

>  

> -	stat->tid = gettid();

> +	par->tid = gettid();

>  

>  	sigemptyset(&sigset);

>  	sigaddset(&sigset, par->signal);

> @@ -539,7 +539,7 @@ static void *timerthread(void *param)

>  	if (par->mode == MODE_CYCLIC) {

>  		sigev.sigev_notify = SIGEV_THREAD_ID | SIGEV_SIGNAL;

>  		sigev.sigev_signo = par->signal;

> -		sigev.sigev_notify_thread_id = stat->tid;

> +		sigev.sigev_notify_thread_id = par->tid;

>  		timer_create(par->clock, &sigev, &timer);

>  		tspec.it_interval = interval;

>  	}

> @@ -613,7 +613,7 @@ static void *timerthread(void *param)

>  		setitimer(ITIMER_REAL, &itimer, NULL);

>  	}

>  

> -	stat->threadstarted++;

> +	par->threadstarted++;

>  

>  	while (!shutdown) {

>  

> @@ -719,7 +719,7 @@ static void *timerthread(void *param)

>  			shutdown++;

>  			pthread_mutex_lock(&break_thread_id_lock);

>  			if (break_thread_id == 0) {

> -				break_thread_id = stat->tid;

> +				break_thread_id = par->tid;

>  				tracemark("hit latency threshold (%llu > %d)",

>  					  (unsigned long long) diff, tracelimit);

>  				break_thread_value = diff;

> @@ -795,7 +795,7 @@ static void *timerthread(void *param)

>  	/* switch to normal */

>  	schedp.sched_priority = 0;

>  	sched_setscheduler(0, SCHED_OTHER, &schedp);

> -	stat->threadstarted = -1;

> +	par->threadstarted = -1;

>  

>  	return NULL;

>  }

> @@ -1293,7 +1293,7 @@ static void print_tids(struct thread_param *par[], int nthreads)

>  

>  	printf("# Thread Ids:");

>  	for (i = 0; i < nthreads; i++)

> -		printf(" %05d", par[i]->stats->tid);

> +		printf(" %05d", par[i]->tid);

>  	printf("\n");

>  }

>  

> @@ -1407,7 +1407,7 @@ static void print_stat(FILE *fp, struct thread_param *par, int index, int verbos

>  				fmt = "T:%2d (%5d) P:%2d I:%ld C:%7lu "

>  				        "Min:%7ld Act:%5ld Avg:%5ld Max:%8ld";

>  

> -			fprintf(fp, fmt, index, stat->tid, par->prio,

> +			fprintf(fp, fmt, index, par->tid, par->prio,

>  				par->interval, stat->cycles, stat->min,

>  				stat->act, stat->cycles ?

>  				(long)(stat->avg/stat->cycles) : 0, stat->max);

> @@ -1463,7 +1463,7 @@ static void rstat_print_stat(struct thread_param *par, int index, int verbose, i

>  				fmt = "T:%2d (%5d) P:%2d I:%ld C:%7lu "

>  				        "Min:%7ld Act:%5ld Avg:%5ld Max:%8ld";

>  

> -			dprintf(fd, fmt, index, stat->tid, par->prio,

> +			dprintf(fd, fmt, index, par->tid, par->prio,

>  				par->interval, stat->cycles, stat->min,

>  				stat->act, stat->cycles ?

>  				(long)(stat->avg/stat->cycles) : 0, stat->max);

> @@ -1966,9 +1966,9 @@ int main(int argc, char **argv)

>  		stat->min = 1000000;

>  		stat->max = 0;

>  		stat->avg = 0.0;

> -		stat->threadstarted = 1;

>  		stat->smi_count = 0;

> -		status = pthread_create(&stat->thread, &attr, timerthread, par);

> +		par->threadstarted = 1;

> +		status = pthread_create(&par->thread, &attr, timerthread, par);

>  		if (status)

>  			fatal("failed to create thread %d: %s\n", i, strerror(status));

>  

> @@ -2038,10 +2038,10 @@ int main(int argc, char **argv)

>  	if (quiet)

>  		quiet = 2;

>  	for (i = 0; i < num_threads; i++) {

> -		if (statistics[i]->threadstarted > 0)

> -			pthread_kill(statistics[i]->thread, SIGTERM);

> -		if (statistics[i]->threadstarted) {

> -			pthread_join(statistics[i]->thread, NULL);

> +		if (parameters[i]->threadstarted > 0)

> +			pthread_kill(parameters[i]->thread, SIGTERM);

> +		if (parameters[i]->threadstarted) {

> +			pthread_join(parameters[i]->thread, NULL);

>  			if (quiet && !histogram)

>  				print_stat(stdout, parameters[i], i, 0, 0);

>  		}

> -- 

> 2.30.0

> 

> 


Why? I don't see any advantage to this, and according to the comments at 
the top of the struct, thread_param is to transfer params to a thread
and thread_stat is for statistics. This is unnecessary churn, unless
you can convince me otherwise. I was worried that your JSON changes
would rely on this being changed, but as far as I can see, they do not!
diff mbox series

Patch

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index c4b2369bee6b..7c45732c1553 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -113,6 +113,9 @@  static char *policyname(int policy);
 
 /* Struct to transfer parameters to the thread */
 struct thread_param {
+	pthread_t thread;
+	int threadstarted;
+	int tid;
 	int prio;
 	int policy;
 	int mode;
@@ -141,9 +144,6 @@  struct thread_stat {
 	long *smis;
 	long *hist_array;
 	long *outliers;
-	pthread_t thread;
-	int threadstarted;
-	int tid;
 	long reduce;
 	long redmax;
 	long cycleofmax;
@@ -530,7 +530,7 @@  static void *timerthread(void *param)
 	interval.tv_sec = par->interval / USEC_PER_SEC;
 	interval.tv_nsec = (par->interval % USEC_PER_SEC) * 1000;
 
-	stat->tid = gettid();
+	par->tid = gettid();
 
 	sigemptyset(&sigset);
 	sigaddset(&sigset, par->signal);
@@ -539,7 +539,7 @@  static void *timerthread(void *param)
 	if (par->mode == MODE_CYCLIC) {
 		sigev.sigev_notify = SIGEV_THREAD_ID | SIGEV_SIGNAL;
 		sigev.sigev_signo = par->signal;
-		sigev.sigev_notify_thread_id = stat->tid;
+		sigev.sigev_notify_thread_id = par->tid;
 		timer_create(par->clock, &sigev, &timer);
 		tspec.it_interval = interval;
 	}
@@ -613,7 +613,7 @@  static void *timerthread(void *param)
 		setitimer(ITIMER_REAL, &itimer, NULL);
 	}
 
-	stat->threadstarted++;
+	par->threadstarted++;
 
 	while (!shutdown) {
 
@@ -719,7 +719,7 @@  static void *timerthread(void *param)
 			shutdown++;
 			pthread_mutex_lock(&break_thread_id_lock);
 			if (break_thread_id == 0) {
-				break_thread_id = stat->tid;
+				break_thread_id = par->tid;
 				tracemark("hit latency threshold (%llu > %d)",
 					  (unsigned long long) diff, tracelimit);
 				break_thread_value = diff;
@@ -795,7 +795,7 @@  static void *timerthread(void *param)
 	/* switch to normal */
 	schedp.sched_priority = 0;
 	sched_setscheduler(0, SCHED_OTHER, &schedp);
-	stat->threadstarted = -1;
+	par->threadstarted = -1;
 
 	return NULL;
 }
@@ -1293,7 +1293,7 @@  static void print_tids(struct thread_param *par[], int nthreads)
 
 	printf("# Thread Ids:");
 	for (i = 0; i < nthreads; i++)
-		printf(" %05d", par[i]->stats->tid);
+		printf(" %05d", par[i]->tid);
 	printf("\n");
 }
 
@@ -1407,7 +1407,7 @@  static void print_stat(FILE *fp, struct thread_param *par, int index, int verbos
 				fmt = "T:%2d (%5d) P:%2d I:%ld C:%7lu "
 				        "Min:%7ld Act:%5ld Avg:%5ld Max:%8ld";
 
-			fprintf(fp, fmt, index, stat->tid, par->prio,
+			fprintf(fp, fmt, index, par->tid, par->prio,
 				par->interval, stat->cycles, stat->min,
 				stat->act, stat->cycles ?
 				(long)(stat->avg/stat->cycles) : 0, stat->max);
@@ -1463,7 +1463,7 @@  static void rstat_print_stat(struct thread_param *par, int index, int verbose, i
 				fmt = "T:%2d (%5d) P:%2d I:%ld C:%7lu "
 				        "Min:%7ld Act:%5ld Avg:%5ld Max:%8ld";
 
-			dprintf(fd, fmt, index, stat->tid, par->prio,
+			dprintf(fd, fmt, index, par->tid, par->prio,
 				par->interval, stat->cycles, stat->min,
 				stat->act, stat->cycles ?
 				(long)(stat->avg/stat->cycles) : 0, stat->max);
@@ -1966,9 +1966,9 @@  int main(int argc, char **argv)
 		stat->min = 1000000;
 		stat->max = 0;
 		stat->avg = 0.0;
-		stat->threadstarted = 1;
 		stat->smi_count = 0;
-		status = pthread_create(&stat->thread, &attr, timerthread, par);
+		par->threadstarted = 1;
+		status = pthread_create(&par->thread, &attr, timerthread, par);
 		if (status)
 			fatal("failed to create thread %d: %s\n", i, strerror(status));
 
@@ -2038,10 +2038,10 @@  int main(int argc, char **argv)
 	if (quiet)
 		quiet = 2;
 	for (i = 0; i < num_threads; i++) {
-		if (statistics[i]->threadstarted > 0)
-			pthread_kill(statistics[i]->thread, SIGTERM);
-		if (statistics[i]->threadstarted) {
-			pthread_join(statistics[i]->thread, NULL);
+		if (parameters[i]->threadstarted > 0)
+			pthread_kill(parameters[i]->thread, SIGTERM);
+		if (parameters[i]->threadstarted) {
+			pthread_join(parameters[i]->thread, NULL);
 			if (quiet && !histogram)
 				print_stat(stdout, parameters[i], i, 0, 0);
 		}