Hi!
> +#define SAFE_PTHREAD(fn, ...) do { \
> +     int ret = fn(__VA_ARGS__); \
> +     if (ret) { \
> +             TEST_ERRNO = ret; \
> +             tst_brkm(TBROK | TTERRNO, cleanup, #fn); \
                                   ^
                                 We have TRERRNO for the case of pthread
                                 functions.
> +     } \
> +} while (0)
> +
> +char *TCID = "clock_gettime04";
> +int TST_TOTAL = 1;
> +
> +static volatile sig_atomic_t done_testing;
> +static int opt_testtime;
> +static char *opt_testtimestr;
> +static option_t options[] = {
> +     {"T:", &opt_testtime, &opt_testtimestr},

There is no need to use both opt_testtime and opt_testtimestr, you can
do just:

        if (opt_testtimestr) {
                ...
        }

> +     {NULL, NULL, NULL}
> +};
> +static pthread_mutex_t exit_mutex = PTHREAD_MUTEX_INITIALIZER;
> +
> +static void setup(void);
> +static void cleanup(void);
> +
> +static void sigproc(int sig)
> +{
> +     (void) sig;
> +     done_testing = 1;
> +}

Shouldn't we rather do:

        if (!done_testing)
                done_testing = 1;

Because as far as I can see the alarm may stil fire between
one of the threads set done_testing to 2 and the if (done_testing == 1)
message.

> +int64_t get_thread_cpu_time(pthread_t thread)
> +{
> +     clockid_t clockid;
> +     int ret;
> +     struct timespec tp;
> +
> +     SAFE_PTHREAD(pthread_getcpuclockid, thread, &clockid);
> +
> +     ret = clock_gettime(clockid, &tp);
> +     if (ret)
> +             tst_brkm(TBROK | TERRNO, cleanup, "clock_gettime: %d", ret);
> +
> +     return (tp.tv_sec * NANOSECS_PER_SEC) + tp.tv_nsec;
> +}
> +
> +static void *child(void *arg)
> +{
> +     struct timespec tim;
> +     int64_t t, t_last = 0;
> +     int check_own_cpu_time = *(int *)arg;
> +
> +     tim.tv_sec = 0;
> +     tim.tv_nsec = 5000;
> +
> +     while (!done_testing) {
> +             nanosleep(&tim, NULL);
> +
> +             if (!check_own_cpu_time)
> +                     continue;
> +
> +             t = get_thread_cpu_time(pthread_self());
> +             if (t < t_last) {
> +                     tst_resm(TFAIL, "t: %" PRId64 " < t_last: %"
> +                             PRId64, t, t_last);
> +                     done_testing = 2;
> +             } else {
> +                     t_last = t;
> +             }
> +     }
> +
> +     SAFE_PTHREAD(pthread_mutex_lock, &exit_mutex);
> +     SAFE_PTHREAD(pthread_mutex_unlock, &exit_mutex);
> +
> +     return NULL;
> +}
> +
> +static void test(int seconds)
> +{
> +     int i, child_check_cpu_time[] = { 0, 1 };
> +     pthread_t thread[THREAD_COUNT];
> +     clockid_t clockid[THREAD_COUNT];
> +     int64_t thread_time[THREAD_COUNT], cur_time;
> +
> +     done_testing = 0;
> +     alarm(seconds);
> +
> +     /* create children and make every 2nd child check its own
> +      * cpu time as well. */
> +     for (i = 0; i < THREAD_COUNT; i++) {
> +             SAFE_PTHREAD(pthread_create, &thread[i], NULL, child,
> +                     &child_check_cpu_time[i % 2]);
> +             SAFE_PTHREAD(pthread_getcpuclockid, thread[i], &clockid[i]);
> +             tst_resm(TINFO, "thread no.: %d clockid: %d", i, clockid[i]);
> +     }
> +
> +     for (i = 0; i < THREAD_COUNT; i++)
> +             thread_time[i] = get_thread_cpu_time(thread[i]);
> +
> +     SAFE_PTHREAD(pthread_mutex_lock, &exit_mutex);

Hmm, shouldn't this mutex be locked before we create the threads
because otherwise some threads may have exited allready.

> +     /* check periodically that cpu time of each thread
> +      * doesn't go backwards */
> +     while (!done_testing) {
> +             for (i = 0; i < THREAD_COUNT; i++) {
> +                     cur_time = get_thread_cpu_time(thread[i]);
> +                     if (cur_time < thread_time[i]) {
> +                             tst_resm(TFAIL, "cpu clock time went backwards "
> +                                             "thread no. %d, clock id: %d"
> +                                             " previous value: %" PRId64
> +                                             " current value: %" PRId64,
> +                                             i, clockid[i], thread_time[i],
> +                                             cur_time);
> +                             done_testing = 2;
> +                             break;
> +                     }
> +                     thread_time[i] = cur_time;
> +             }
> +     }
> +
> +     /* allow children to exit now */
> +     SAFE_PTHREAD(pthread_mutex_unlock, &exit_mutex);
> +
> +     for (i = 0; i < THREAD_COUNT; i++)
> +             SAFE_PTHREAD(pthread_join, thread[i], NULL);
> +
> +     if (done_testing == 1)
> +             tst_resm(TPASS, "Test completed, cpu times of all "
> +                             "thread clocks were monotonic");
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +     int testtime = DEFAULT_TEST_TIME;
> +     const char *msg = NULL;
> +
> +     msg = parse_opts(argc, argv, options, NULL);
> +     if (msg)
> +             tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
> +
> +     if (opt_testtime) {
> +             testtime = atoi(opt_testtimestr);
> +             if (testtime <= 0) {
> +                     tst_brkm(TBROK, NULL, "Invalid arg for -T: %s",
> +                             opt_testtimestr);
> +             }
> +     }
> +
> +     setup();
> +
> +     test(testtime);

Shouldn't the test(testtime) be still inside the for () loop with
TEST_LOOPING() because otherwise the test will gladly accept and ignore
the standard parameters (-i -I etc).

Otherwise it looks OK to me.

-- 
Cyril Hrubis
chru...@suse.cz

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to