* [rt-tests][PATCH] align thread wakeup times
@ 2013-09-09 7:29 Nicholas Mc Guire
2013-10-04 13:22 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 6+ messages in thread
From: Nicholas Mc Guire @ 2013-09-09 7:29 UTC (permalink / raw)
To: williams; +Cc: linux-rt-users, C.Emde, tglx, andi
Hi !
This patch provides and additional -A/--align flag to cyclictest to align
thread wakeup times of all threads as closly defined as possible.
When running multiple threads in cyclictest (-S or -t # option) the threads
are launched in an unsynchronized manner. Basically the creation order and
time for thread creation determines the start time. For provoking a maximum
congestion situation (e.g. cache evictions) and to improve reproducibility
or run conditions the start time should be defined distances appart. The
well defined distance is implemented as a offset parameter to -A/--align
and will offset each threads start time by the parameter * the sequentially
assigned thread number (par->tnum), together with the -d0 (distance in the
intervals of the individual threads) this alignment option allows to get
the thread wakeup times as closely synchronized as possible.
The method to sync is simply that the thread with par->tnum == 0 is chosen
to set a globally shared timestamp, and all other threads use this timestamp
as their starting time rather than each calling clock_gettime() at startup.
To ensure synchronization of the thread startup the setting of the global
time is guarded by pthread_barriers.
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
Reviewed-by: Andreas Platschek <andreas.platschek@opentech.at>
---
src/cyclictest/cyclictest.c | 36 ++++++++++++++++++++++++++++++++++--
1 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index abf3e8b..96edda5 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -138,6 +138,7 @@ struct thread_param {
unsigned long interval;
int cpu;
int node;
+ int tnum;
};
/* Struct for statistics */
@@ -178,6 +179,8 @@ static int force_sched_other;
static int priospread = 0;
static int check_clock_resolution;
static int ct_debug;
+static int aligned = 0;
+static int disaligned = 0;
static pthread_cond_t refresh_on_max_cond = PTHREAD_COND_INITIALIZER;
static pthread_mutex_t refresh_on_max_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -186,6 +189,10 @@ static pthread_mutex_t break_thread_id_lock = PTHREAD_MUTEX_INITIALIZER;
static pid_t break_thread_id = 0;
static uint64_t break_thread_value = 0;
+static pthread_barrier_t align_barr;
+static pthread_barrier_t globalt_barr;
+static struct timespec globalt;
+
/* Backup of kernel variables that we modify */
static struct kvars {
char name[KVARNAMELEN];
@@ -758,7 +765,20 @@ void *timerthread(void *param)
fatal("timerthread%d: failed to set priority to %d\n", par->cpu, par->prio);
/* Get current time */
- clock_gettime(par->clock, &now);
+ if(aligned){
+ pthread_barrier_wait(&globalt_barr);
+ if(par->tnum==0){
+ clock_gettime(par->clock, &globalt);
+ }
+ pthread_barrier_wait(&align_barr);
+ now = globalt;
+ if(disaligned){
+ now.tv_nsec += disaligned * par->tnum;
+ tsnorm(&now);
+ }
+ } else {
+ clock_gettime(par->clock, &now);
+ }
next = now;
next.tv_sec += interval.tv_sec;
@@ -951,6 +971,7 @@ static void display_help(int error)
"cyclictest <options>\n\n"
"-a [NUM] --affinity run thread #N on processor #N, if possible\n"
" with NUM pin all threads to the processor NUM\n"
+ "-A USEC --aligned=USEC allign threads wakeup to a specific offset"
"-b USEC --breaktrace=USEC send break trace command when latency > USEC\n"
"-B --preemptirqs both preempt and irqsoff tracing (used with -b)\n"
"-c CLOCK --clock=CLOCK select clock\n"
@@ -1093,6 +1114,7 @@ static void process_options (int argc, char *argv[])
*/
static struct option long_options[] = {
{"affinity", optional_argument, NULL, 'a'},
+ {"aligned", optional_argument, NULL, 'A'},
{"breaktrace", required_argument, NULL, 'b'},
{"preemptirqs", no_argument, NULL, 'B'},
{"clock", required_argument, NULL, 'c'},
@@ -1133,7 +1155,7 @@ static void process_options (int argc, char *argv[])
{"help", no_argument, NULL, '?'},
{NULL, 0, NULL, 0}
};
- int c = getopt_long(argc, argv, "a::b:Bc:Cd:D:e:Efh:H:i:Il:MnNo:O:p:PmqQrRsSt::uUvD:wWXT:y:",
+ int c = getopt_long(argc, argv, "a::A:b:Bc:Cd:D:e:Efh:H:i:Il:MnNo:O:p:PmqQrRsSt::uUvD:wWXT:y:",
long_options, &option_index);
if (c == -1)
break;
@@ -1152,6 +1174,10 @@ static void process_options (int argc, char *argv[])
setaffinity = AFFINITY_USEALL;
}
break;
+ case 'A': aligned=1;
+ if (optarg != NULL)
+ disaligned = atoi(optarg);
+ break;
case 'b': tracelimit = atoi(optarg); break;
case 'B': tracetype = PREEMPTIRQSOFF; break;
case 'c': clocksel = atoi(optarg); break;
@@ -1318,6 +1344,11 @@ static void process_options (int argc, char *argv[])
if (num_threads < 1)
error = 1;
+ if (aligned){
+ pthread_barrier_init (&globalt_barr, NULL, num_threads);
+ pthread_barrier_init (&align_barr, NULL, num_threads);
+ }
+
if (error)
display_help(1);
}
@@ -1744,6 +1775,7 @@ int main(int argc, char **argv)
par->max_cycles = max_cycles;
par->stats = stat;
par->node = node;
+ par->tnum = i;
switch (setaffinity) {
case AFFINITY_UNSPECIFIED: par->cpu = -1; break;
case AFFINITY_SPECIFIED: par->cpu = affinity; break;
--
1.7.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [rt-tests][PATCH] align thread wakeup times
2013-09-09 7:29 [rt-tests][PATCH] align thread wakeup times Nicholas Mc Guire
@ 2013-10-04 13:22 ` Sebastian Andrzej Siewior
2013-10-04 13:33 ` Nicholas Mc Guire
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-04 13:22 UTC (permalink / raw)
To: Nicholas Mc Guire; +Cc: williams, linux-rt-users, C.Emde, tglx, andi
* Nicholas Mc Guire | 2013-09-09 09:29:48 [+0200]:
>Hi !
Hi Nicholas,
> This patch provides and additional -A/--align flag to cyclictest to align
> thread wakeup times of all threads as closly defined as possible.
>
> When running multiple threads in cyclictest (-S or -t # option) the threads
> are launched in an unsynchronized manner. Basically the creation order and
> time for thread creation determines the start time. For provoking a maximum
> congestion situation (e.g. cache evictions) and to improve reproducibility
> or run conditions the start time should be defined distances appart. The
> well defined distance is implemented as a offset parameter to -A/--align
> and will offset each threads start time by the parameter * the sequentially
> assigned thread number (par->tnum), together with the -d0 (distance in the
> intervals of the individual threads) this alignment option allows to get
> the thread wakeup times as closely synchronized as possible.
>
> The method to sync is simply that the thread with par->tnum == 0 is chosen
> to set a globally shared timestamp, and all other threads use this timestamp
> as their starting time rather than each calling clock_gettime() at startup.
> To ensure synchronization of the thread startup the setting of the global
> time is guarded by pthread_barriers.
I would rather fix current behaviour instead introducing yet another
option. By using -d0 I assume that all threads wakeup at the same time.
According to your patch this does not happen due to the thread creating
/ starting overhead.
Is there is a reason to keep this "faulty" behavior? If not I would vote
to make this what you suggest the default.
Sebastian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rt-tests][PATCH] align thread wakeup times
2013-10-04 13:22 ` Sebastian Andrzej Siewior
@ 2013-10-04 13:33 ` Nicholas Mc Guire
2013-10-04 15:01 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 6+ messages in thread
From: Nicholas Mc Guire @ 2013-10-04 13:33 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: williams, linux-rt-users, C.Emde, tglx, andi
On Fri, 04 Oct 2013, Sebastian Andrzej Siewior wrote:
> * Nicholas Mc Guire | 2013-09-09 09:29:48 [+0200]:
>
> >Hi !
> Hi Nicholas,
>
> > This patch provides and additional -A/--align flag to cyclictest to align
> > thread wakeup times of all threads as closly defined as possible.
> >
> > When running multiple threads in cyclictest (-S or -t # option) the threads
> > are launched in an unsynchronized manner. Basically the creation order and
> > time for thread creation determines the start time. For provoking a maximum
> > congestion situation (e.g. cache evictions) and to improve reproducibility
> > or run conditions the start time should be defined distances appart. The
> > well defined distance is implemented as a offset parameter to -A/--align
> > and will offset each threads start time by the parameter * the sequentially
> > assigned thread number (par->tnum), together with the -d0 (distance in the
> > intervals of the individual threads) this alignment option allows to get
> > the thread wakeup times as closely synchronized as possible.
> >
> > The method to sync is simply that the thread with par->tnum == 0 is chosen
> > to set a globally shared timestamp, and all other threads use this timestamp
> > as their starting time rather than each calling clock_gettime() at startup.
> > To ensure synchronization of the thread startup the setting of the global
> > time is guarded by pthread_barriers.
>
> I would rather fix current behaviour instead introducing yet another
> option. By using -d0 I assume that all threads wakeup at the same time.
Nop they do not -d0 just says that they all use
the same period rather than some offset with multiple threads (e.g. -S)
> According to your patch this does not happen due to the thread creating
> / starting overhead.
> Is there is a reason to keep this "faulty" behavior? If not I would vote
> to make this what you suggest the default.
>
its not faulty behavior - its a different case
in fact we need both.
same period + "random" start time
same period + synced start time
it makes a difference on some boxes that is significant.
thx!
hofrat
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rt-tests][PATCH] align thread wakeup times
2013-10-04 13:33 ` Nicholas Mc Guire
@ 2013-10-04 15:01 ` Sebastian Andrzej Siewior
2013-10-04 16:21 ` Nicholas Mc Guire
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-04 15:01 UTC (permalink / raw)
To: Nicholas Mc Guire; +Cc: williams, linux-rt-users, C.Emde, tglx, andi
On 10/04/2013 03:33 PM, Nicholas Mc Guire wrote:
>> I would rather fix current behaviour instead introducing yet another
>> option. By using -d0 I assume that all threads wakeup at the same time.
>
> Nop they do not -d0 just says that they all use
> the same period rather than some offset with multiple threads (e.g. -S)
>
>> According to your patch this does not happen due to the thread creating
>> / starting overhead.
>> Is there is a reason to keep this "faulty" behavior? If not I would vote
>> to make this what you suggest the default.
>>
>
> its not faulty behavior - its a different case
> in fact we need both.
>
> same period + "random" start time
> same period + synced start time
>
> it makes a difference on some boxes that is significant.
So you say with -d0, where d is documented as "distance of thread
intervals in us", I should not expect that all threads share the exact
same wakeup time (because their distance is 0)?
> thx!
> hofrat
>
Sebastian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rt-tests][PATCH] align thread wakeup times
2013-10-04 15:01 ` Sebastian Andrzej Siewior
@ 2013-10-04 16:21 ` Nicholas Mc Guire
2013-10-04 16:35 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 6+ messages in thread
From: Nicholas Mc Guire @ 2013-10-04 16:21 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: williams, linux-rt-users, C.Emde, tglx, andi
On Fri, 04 Oct 2013, Sebastian Andrzej Siewior wrote:
> On 10/04/2013 03:33 PM, Nicholas Mc Guire wrote:
> >> I would rather fix current behaviour instead introducing yet another
> >> option. By using -d0 I assume that all threads wakeup at the same time.
> >
> > Nop they do not -d0 just says that they all use
> > the same period rather than some offset with multiple threads (e.g. -S)
> >
> >> According to your patch this does not happen due to the thread creating
> >> / starting overhead.
> >> Is there is a reason to keep this "faulty" behavior? If not I would vote
> >> to make this what you suggest the default.
> >>
> >
> > its not faulty behavior - its a different case
> > in fact we need both.
> >
> > same period + "random" start time
> > same period + synced start time
> >
> > it makes a difference on some boxes that is significant.
>
> So you say with -d0, where d is documented as "distance of thread
> intervals in us", I should not expect that all threads share the exact
> same wakeup time (because their distance is 0)?
>
no why should you - you might want
-i 100 -d 50 -A 20
T1 0 100 200 300
T2 20 170
T3 40 240
T4 60 310
Which is quite a lot different than just stating
that i_1=100 i_2=150, etc... but initial distance "undefined" or random
so in the sense of schedulability tests these are really two seperate
parameters and thus should be configurable independently.
That the current documentation might give the impression that they are
in sync is a bug in the documentation not in the -d implementation.
hofrat
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [rt-tests][PATCH] align thread wakeup times
2013-10-04 16:21 ` Nicholas Mc Guire
@ 2013-10-04 16:35 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-04 16:35 UTC (permalink / raw)
To: Nicholas Mc Guire; +Cc: williams, linux-rt-users, C.Emde, tglx, andi
On 10/04/2013 06:21 PM, Nicholas Mc Guire wrote:
> no why should you - you might want
>
> -i 100 -d 50 -A 20
>
> T1 0 100 200 300
> T2 20 170
> T3 40 240
> T4 60 310
>
> Which is quite a lot different than just stating
>
> that i_1=100 i_2=150, etc... but initial distance "undefined" or random
>
> so in the sense of schedulability tests these are really two seperate
> parameters and thus should be configurable independently.
>
> That the current documentation might give the impression that they are
> in sync is a bug in the documentation not in the -d implementation.
Now, I get it. Okay, so this makes sense.
> hofrat
>
>
Sebastian
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-04 16:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-09 7:29 [rt-tests][PATCH] align thread wakeup times Nicholas Mc Guire
2013-10-04 13:22 ` Sebastian Andrzej Siewior
2013-10-04 13:33 ` Nicholas Mc Guire
2013-10-04 15:01 ` Sebastian Andrzej Siewior
2013-10-04 16:21 ` Nicholas Mc Guire
2013-10-04 16:35 ` Sebastian Andrzej Siewior
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).