mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] alarmtimer: Fix some non-standard alarm timer behavior
@ 2014-09-10  1:31 Richard Larocque
  2014-09-10  1:31 ` [PATCH 1/3] alarmtimer: Return relative times in timer_gettime Richard Larocque
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Richard Larocque @ 2014-09-10  1:31 UTC (permalink / raw)
  To: tglx, john.stultz; +Cc: linux-kernel, Richard Larocque

I've been working on some changes to posix-timers.c in an attempt to better
support CRIU.  Along the way, I've discovered some issues with the posix alarm
timers that should be fixed independent of any other work.

It seems that there was an older issue with the setting of relative timeouts
with timer_settime().  That was addressed around 3.16 with
16927776ae757d0d132bdbfabbfe2c498342bd59 ("alarmtimer: Fix bug where relative
alarm timers were treated as absolute").  That fixed the setting of times, but
it doesn't fix the retrieval of times.

According to the man pages (and POSIX), timer_gettime() is supposed to return
the time left until the timeout, not the absolute time at which the timeout is
expected to fire.  The non-ALARM clocks correctly show relative times, but
CLOCK_BOOTTIME_ALARM and CLOCK_REALTIME_ALARM do not.

A second issue with these timers is that they call posix_event_timer() without
checking the value of it_sigev_notify.  This is a problem, because
it_sigev_notify could be SIGEV_NONE, in which case the signal should not be
delivered.  The non-alarm timers don't need to check it_sigev_notify in their
timeout handling code path, because they never schedule timeouts for those
kinds of timers in the first place.

See below for a program that demonstrates these behaviors, and some sample output.

The third patch in this stack is the odd one out.  I suspect that there's
a locking issue in the alarm timer code, and this is my attempt to fix it.
It's not quite related to the first two, but it does conflict with one of them,
so I figured I should include it in this patch stack.

PS: This is my first upstream patch, so please excuse any etiquette violations.

Richard Larocque (3):
  alarmtimer: Return relative times in timer_gettime
  alarmtimer: Do not signal SIGEV_NONE timers
  alarmtimer: Lock k_itimer during timer callback

 kernel/time/alarmtimer.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)
--------
/* timer_gettime_test.c: A program to test the behavior of timer_gettime() */

#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <unistd.h>

static void do_show_timer(clockid_t which_clock);

int main() {
        printf("CLOCK_REALTIME\n");
        do_show_timer(CLOCK_REALTIME);
        printf("\n");

        printf("CLOCK_BOOTTIME\n");
        do_show_timer(CLOCK_BOOTTIME);
        printf("\n");

        printf("CLOCK_REALTIME_ALARM\n");
        do_show_timer(CLOCK_REALTIME_ALARM);
        printf("\n");

        printf("CLOCK_BOOTTIME_ALARM\n");
        do_show_timer(CLOCK_BOOTTIME_ALARM);
        printf("\n");
}

static void do_show_timer(clockid_t which_clock) {
        struct sigevent sevp;
        timer_t timerid;
        struct itimerspec in, out;
        struct timespec ts;
        int i;

        /*
         * SIGEV_NONE is supposed to prevent signal delivery, but it doesn't.
         * Set signo to SIGSTOP to make the received signal obvious but
         * harmless.
         */
        sevp.sigev_notify = SIGEV_NONE;
        sevp.sigev_signo = SIGSTOP;

        if (timer_create(which_clock, &sevp, &timerid) != 0) {
                perror("timer_create");
                exit(EXIT_FAILURE);
        }

        if (timer_gettime(timerid, &out) != 0) {
                perror("timer_gettime");
                exit(EXIT_FAILURE);
        }
        printf("before timer start: %ld.%09ld\n",
                        out.it_value.tv_sec, out.it_value.tv_nsec);

        /* Use absolute times to sidestep bug in older kernels. */
        clock_gettime(which_clock, &ts);

        in.it_value.tv_sec = ts.tv_sec + 3600;
        in.it_value.tv_nsec = ts.tv_nsec;
        in.it_interval.tv_sec = 0;
        in.it_interval.tv_nsec = 0;
        if (timer_settime(timerid, TIMER_ABSTIME, &in, NULL)) {
                perror("timer_settime");
                exit(EXIT_FAILURE);
        }

        for (i = 0; i < 3; ++i) {
                if (timer_gettime(timerid, &out) != 0) {
                        perror("timer_gettime");
                        exit(EXIT_FAILURE);
                }
                printf("step %d: %ld.%09ld\n",
                                i, out.it_value.tv_sec, out.it_value.tv_nsec);
                sleep(1);
        }

        in.it_value.tv_sec = ts.tv_sec;
        in.it_value.tv_nsec = ts.tv_nsec;
        in.it_interval.tv_sec = 0;
        in.it_interval.tv_nsec = 0;
        if (timer_settime(timerid, TIMER_ABSTIME, &in, NULL)) {
                perror("timer_settime");
                exit(EXIT_FAILURE);
        }

        if (timer_gettime(timerid, &out) != 0) {
                perror("timer_gettime");
                exit(EXIT_FAILURE);
        }
        printf("after expiry: %ld.%09ld\n",
                        out.it_value.tv_sec, out.it_value.tv_nsec);

        timer_delete(timerid);
}
--------
ihrt16:~# ./timer_gettime_test 
CLOCK_REALTIME
before timer start: 0.000000000
step 0: 3599.999987035
step 1: 3598.999824068
step 2: 3597.999588143
after expiry: 0.000000000

CLOCK_BOOTTIME
before timer start: 0.000000000
step 0: 3599.999996257
step 1: 3598.999871706
step 2: 3597.999753939
after expiry: 0.000000000

CLOCK_REALTIME_ALARM
before timer start: 0.000000000
step 0: 1410314715.145776868
step 1: 1410314715.145776868
step 2: 1410314715.145776868

[1]+  Stopped                 ./timer_gettime_test
ihrt16:~# fg
./timer_gettime_test
after expiry: 1410311115.145776868

CLOCK_BOOTTIME_ALARM
before timer start: 0.000000000
step 0: 3923.018128712
step 1: 3923.018128712
step 2: 3923.018128712

[1]+  Stopped                 ./timer_gettime_test
ihrt16:~# fg
./timer_gettime_test
after expiry: 323.018128712

ihrt16:~# 
-- 
2.1.0.rc2.206.gedb03e5


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-10 17:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-10  1:31 [PATCH 0/3] alarmtimer: Fix some non-standard alarm timer behavior Richard Larocque
2014-09-10  1:31 ` [PATCH 1/3] alarmtimer: Return relative times in timer_gettime Richard Larocque
2014-09-10  1:31 ` [PATCH 2/3] alarmtimer: Do not signal SIGEV_NONE timers Richard Larocque
2014-09-10  1:31 ` [PATCH 3/3] alarmtimer: Lock k_itimer during timer callback Richard Larocque
2014-09-10  3:33 ` [PATCH 0/3] alarmtimer: Fix some non-standard alarm timer behavior John Stultz
2014-09-10 17:40   ` Richard Larocque

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome