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@xxxxxxx>
> ---
> 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!
