lkml.org 
[lkml]   [2007]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: Tesing of / bugs in new timerfd API
From
Hi Davide,

Davide Libenzi wrote:
> On Thu, 13 Dec 2007, Michael Kerrisk wrote:
>
>>>> BUG 2:
>>>> The last sentence does not match the implementation.
>>>> (Nor is it consistent with the behavior of POSIX timers.
>>>> And I *think* things did work correctly in the original
>>>> timerfd() implementation, but I have not gone back to check.)
>>>>
>>>> Suppose that we set an absolute timer to expire 100 seconds
>>>> in the future. Then according to this sentence of the man
>>>> page then each subsequent call to timerfd_gettime() should
>>>> retrun an itimerspec structure whose it_value steadily
>>>> decreases from 100 to 0 (when the timer expires). (This
>>>> is the behavior in the analogous situation with POSIX timers
>>>> and with setitimer()/getitimer().)
>>>>
>>>> However, the implementation of timerfd_gettime() always
>>>> returns the "time when the timer would next expire", and
>>>> this value depends on whether TFD_TIMER_ABSTIME was specified
>>>> when setting the timer.
>>> This is been like that from the beginning of the new API. So no, the
>>> previous was behaving exactly the same WRT this feature.
>>> Is this something really needed?
>> Three reasons that I think of off the top of my head (and there might
>> well be more reasosn) why this should change:
>>
>> a) consistency with the other two timer APIs (POSIX timers
>> (timer_create(), etc.), and setitimer()/getitimer()).

You made no comment on this, which is perhaps the most important of the
reasons to make the change.

>> b) Returning the amount of time until the next expiration is more
>> useful to userland: I'd say the most common case is for userland to
>> want to know how long until the next expiration occurs, or to adjust
>> that time by adding/subtracting some value to the existing setting.
>> That is difficult to with the current implementation: the userland app
>> must use timer_gettime(), call clock_gettime(), and calculate the
>> difference between the two, in order to know how much time remains
>> until the next timer expiration.
>
> Bah, I don't want to argue with that because otherwise it starts going
> towards the "typical" use cases listing, that can be found the same exact
> reasons to have one or the other way. And we end up wasting lots of time.
> We'd just have to move another thing that userspace could do, inside the
> kernel (subtract the current value returned by hrtimer_forward() in
> ->expires with "now").

Okay -- I still think this reason matters, but let's leave it for the
moment. I think the other reasons are strong enough anyway...

>> c) Currently, the information returned differs depending on whether
>> TFD_TIMER_ABSTIME is specified -- this is not the case for the
>> analogous situation for POSIX timers. For POSIX timers, the returned
>> setting is always the amount of time until the timer next expires.
>> This inconsistency is messy for applications -- the application may
>> not (be able to) know whether or not the timer it is examining was set
>> using TFD_TIMER_ABSTIME (the timerfd might have been created by a
>> library, for example).
>
> Hmm... the time returned is always the next absolute time when the next
> timer tick will go off. It does not depend on TFD_TIMER_ABSTIME. I return
> the ->expires field of the timer, and hrtimer_forward() sets it to the
> next absolute time.

You snipped my example that demonstrated the problem. Both of the
following runs create a timer that expires 10 seconds from "now", but
observe the difference in the value returned by timerfd_gettime():

$ ./timerfd_test 10 # does not use TFD_TIMER_ABSTIME
Initial setting for settime: value=10.000, interval=0.000
./timerfd_test> g
(elapsed time= 1)
Current value: value=346.448, interval=0.000

$ ./timerfd_test -a 10 # uses TFD_TIMER_ABSTIME
Initial setting for settime: value=1197630855.254, interval=0.000
./timerfd_test> g
(elapsed time= 1)
Current value: value=1197630855.254, interval=0.000

Either there's an inconsistency here depending on the use of
TFD_TIMER_ABSTIME, or there is a bug in my understanding or my test program
(but so far I haven't spotted that bug ;-).).

Cheers,

Michael

PS There was a little bug in my test program, do do with the treatment of
nanosecond command-line arguments. It didn't affect any of the tests, but
for completeness, the revised version of the program is below.

/* timerfd_test.c */

/* Link with -lrt */

#define _GNU_SOURCE
#include <sys/syscall.h>
#include <unistd.h>
#include <time.h>
#if defined(__i386__)
#define __NR_timerfd_create 322
#define __NR_timerfd_settime 325
#define __NR_timerfd_gettime 326
#endif

static int
timerfd_create(int clockid, int flags)
{
return syscall(__NR_timerfd_create, clockid, flags);
}

static int
timerfd_settime(int fd, int flags, struct itimerspec *new_value,
struct itimerspec *curr_value)
{
return syscall(__NR_timerfd_settime, fd, flags,
new_value, curr_value);
}

static int
timerfd_gettime(int fd, struct itimerspec *curr_value)
{
return syscall(__NR_timerfd_gettime, fd, curr_value);
}

#define TFD_TIMER_ABSTIME (1 << 0)


#define handle_error(msg) \
do { perror(msg); exit(EXIT_FAILURE); } while (0)

// #include <sys/timerfd.h>
#include <time.h>
#include <sys/times.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <stdint.h> /* Definition of uint64_t */

static void
usage(const char *pname, const char *msg)
{
if (msg != NULL)
printf("%s", msg);
fprintf(stderr, "Usage: %s [options] value-sec [value-nsec "
"[intvl-sec [intvl-nsec]]]\n", pname);
fprintf(stderr, "Options are:\n");
fprintf(stderr, "\t-a Use absolute timer\n");
fprintf(stderr, "\t-m Use CLOCK_MONOTONIC "
"(instead of CLOCK_REALTIME)\n");

exit(EXIT_FAILURE);
} /* usage */

static void
display_help(void)
{
printf("n val-sec val-nsec intvl-sec intvl-nsec\n"
" Reset timer value & interval\n");
printf("g Get timer value\n");
printf("r Read from file descriptor\n");
printf("t Print elapsed time\n");
} /* display_help */


#define MAX_LINE 1024

static void
print_itimerspec(struct itimerspec *its)
{
printf("value=%ld.%03ld, interval=%ld.%03ld",
(long) its->it_value.tv_sec,
(long) its->it_value.tv_nsec / 1000000,
(long) its->it_interval.tv_sec,
(long) its->it_interval.tv_nsec / 1000000);
} /* print_itimerspec */


int
main(int argc, char *argv[])
{
struct itimerspec new_value, curr_value;
int fd, flags;
struct timespec now;
int s, opt, use_abs_timer, use_monotonic;
long arg1, arg2, arg3, arg4;
uint64_t exp;
time_t start;
char line[MAX_LINE];
int num_read, clockid;
char cmd;

use_abs_timer = 0;
use_monotonic = 0;

while ((opt = getopt(argc, argv, "am")) != -1) {
switch (opt) {
case 'a':
use_abs_timer = 1;
break;

case 'm':
use_monotonic = 1;
break;

default:
usage(argv[1], NULL);
break;
} /* switch */
} /* while */

clockid = (use_monotonic ? CLOCK_MONOTONIC : CLOCK_REALTIME);

if (optind + 1 > argc)
usage(argv[0], NULL);

if (clock_gettime(clockid, &now) == -1)
handle_error("clock_gettime");

flags = use_abs_timer ? TFD_TIMER_ABSTIME : 0;

/* Create a timer with initial expiration and interval
as specified in command line */

if (use_abs_timer) {
new_value.it_value.tv_sec = now.tv_sec + atoi(argv[optind]);
new_value.it_value.tv_nsec = (argc > optind + 1) ?
atol(argv[optind + 1]) + now.tv_nsec : now.tv_nsec;
if (new_value.it_value.tv_nsec > 1000000000) {
new_value.it_value.tv_sec +=
new_value.it_value.tv_nsec / 1000000000;
new_value.it_value.tv_nsec %= 1000000000;
} /* if */
} else {
new_value.it_value.tv_sec = atoi(argv[optind]);
new_value.it_value.tv_nsec = (argc > optind + 1) ?
atol(argv[optind + 1]) : 0;
}

new_value.it_interval.tv_sec = (argc > optind + 2) ?
atol(argv[optind + 2]) : 0;
new_value.it_interval.tv_nsec = (argc > optind + 3) ?
atol(argv[optind + 3]) : 0;

fd = timerfd_create(clockid, 0);
if (fd == -1)
handle_error("timerfd_create");

printf("Initial setting for settime: ");
print_itimerspec(&new_value);
printf("\n");

s = timerfd_settime(fd, flags, &new_value, &curr_value);
if (s == -1)
handle_error("timerfd_settime");

start = time(NULL);

for ( ; ; ) {
printf("%s> ", argv[0]);
fflush(stdout);

if (fgets(line, MAX_LINE, stdin) == NULL) /* EOF */
exit(EXIT_SUCCESS);
line[strlen(line) - 1] = '\0'; /* Remove trailing '\n' */

if (*line == '\0')
continue; /* Skip blank lines */

if (line[0] == '?')
display_help();

num_read = sscanf(line, " %c %ld %ld %ld %ld",
&cmd, &arg1, &arg2, &arg3, &arg4);

switch (cmd) {
case 'n':
if (num_read != 5) {
printf("Wrong number of arguments\n");
continue;
}

if (use_abs_timer) {
printf("This is an absolute timer\n");
if (clock_gettime(clockid, &now) == -1)
handle_error("clock_gettime");
printf("Now: ");
printf("value=%ld.%03ld", (long) now.tv_sec,
(long) now.tv_nsec / 1000000);
printf("\n");

new_value.it_value.tv_sec = now.tv_sec + arg1;
new_value.it_value.tv_nsec = now.tv_nsec + arg2;

} else {
printf("This is a relative timer\n");
new_value.it_value.tv_sec = arg1;
new_value.it_value.tv_nsec = arg2;
}

new_value.it_interval.tv_sec = arg3;
new_value.it_interval.tv_nsec = arg4;

printf("New setting for settime: ");
print_itimerspec(&new_value);
printf("\n");

s = timerfd_settime(fd, flags, &new_value, &curr_value);
if (s == -1) {
perror("timerfd_settime");
break;
}
printf("Previous setting from settime: ");
print_itimerspec(&curr_value);
printf("\n");

break;

case 't':
printf("%ld\n", (long) (time(NULL) - start));
break;

case 'g':
s = timerfd_gettime(fd, &curr_value);
if (s == -1)
handle_error("timerfd_gettime");
printf("(elapsed time=%3ld)\n", (long) (time(NULL) - start));
printf("Current value: ");
print_itimerspec(&curr_value);
printf("\n");
break;

case 'r':
s = read(fd, &exp, sizeof(uint64_t));
if (s != sizeof(uint64_t))
handle_error("read");
printf("Read: %lld\n", exp);
break;
} /* switch */
} /* for */

exit(EXIT_SUCCESS);
}



\
 
 \ /
  Last update: 2007-12-14 12:27    [W:0.075 / U:0.644 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site