Re: [tip:timers/core] posix-timers: Zero settings value in common code

Andrei Vagin
2017-06-09T20:12:36Z

Hello Thomas,

This patch breaks one of our CRIU tests:
https://github.com/xemul/criu/blob/master/test/zdtm/static/posix_timers.c#L145

python /root/git/main/criu/test/zdtm.py run -t zdtm/static/posix_timers --iter 0

====================== Run zdtm/static/posix_timers in h =======================
Start test
./posix_timers --pidfile=posix_timers.pid --outfile=posix_timers.out
Send the 15 signal to 24
Wait for zdtm/static/posix_timers(24) to die for 0.100000
############## Test zdtm/static/posix_timers FAIL at result check ##############
Test output: ================================
22:39:39.573: 24: ( start) boottime 12 boottime-coarse 12 total_sleep_time 0
22:39:39.602: 24: ( end) boottime 12 boottime-coarse 12 total_sleep_time 0
22:39:39.603: 24: FAIL: posix_timers.c:145: REALTIME (oneshot): timer became periodic
22:39:39.603: 24: FAIL: posix_timers.c:145: MONOTONIC (oneshot): timer became periodic
22:39:39.603: 24: FAIL: posix_timers.c:145: BOOTTIME (oneshot): timer became periodic

<<< ================================
=== Run 1/1 ================ zdtm/static/posix_timers
##################################### FAIL #####################################

This test sets a timer and then check that it is set correctly. And with this patch,
we see that timer_settime returns incorrect data.

2447 timer_settime(10, 0, {it_interval={tv_sec=0, tv_nsec=0}, it_value={tv_sec=2147483, tv_nsec=647000000}}, NULL) = 0

....

2519 timer_settime(10, 0, {it_interval={tv_sec=0, tv_nsec=0}, it_value={tv_sec=0, tv_nsec=0}}, <unfinished ...>
2509 <... exit_group resumed>) = ?
2519 <... timer_settime resumed> {it_interval={tv_sec=-1861316067, tv_nsec=18446645490300239488}, it_value={tv_sec=-1852565060, tv_nsec=133}}) = 0

Thanks,
Andrei

On Mon, Jun 05, 2017 at 01:21:46AM -0700, tip-bot for Jacob Shin wrote:
> Commit-ID: eabdec04385376d560078992710104cc7be2ce1b
> Gitweb: http://git.kernel.org/tip/eabdec04385376d560078992710104cc7be2ce1b
> Author: Thomas Gleixner <tglx@linutronix.de>
> AuthorDate: Tue, 30 May 2017 23:15:51 +0200
> Committer: Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Sun, 4 Jun 2017 15:40:28 +0200
>
> posix-timers: Zero settings value in common code
>
> Zero out the settings struct in the common code so the callbacks do not
> have to do it themself.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: John Stultz <john.stultz@linaro.org>
> Link: http://lkml.kernel.org/r/20170530211657.200870713@linutronix.de
> ---
> kernel/time/posix-cpu-timers.c | 5 +----
> kernel/time/posix-timers.c | 3 +--
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
> index 96c833a..cb4a4eb 100644
> --- a/kernel/time/posix-cpu-timers.c
> +++ b/kernel/time/posix-cpu-timers.c
> @@ -719,10 +719,8 @@ static void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp
> */
> itp->it_interval = ns_to_timespec64(timer->it.cpu.incr);
>
> - if (timer->it.cpu.expires == 0) { /* Timer not armed at all. */
> - itp->it_value.tv_sec = itp->it_value.tv_nsec = 0;
> + if (!timer->it.cpu.expires)
> return;
> - }
>
> /*
> * Sample the clock to take the difference with the expiry time.
> @@ -746,7 +744,6 @@ static void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp
> * Call the timer disarmed, nothing else to do.
> */
> timer->it.cpu.expires = 0;
> - itp->it_value = ns_to_timespec64(timer->it.cpu.expires);
> return;
> } else {
> cpu_timer_sample_group(timer->it_clock, p, &now);
> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
> index 48f6c37..0332f7a 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -645,8 +645,6 @@ common_timer_get(struct k_itimer *timr, struct itimerspec64 *cur_setting)
> struct timespec64 ts64;
> bool sig_none;
>
> - memset(cur_setting, 0, sizeof(*cur_setting));
> -
> sig_none = (timr->it_sigev_notify & ~SIGEV_THREAD_ID) != SIGEV_NONE;
> iv = timr->it_interval;
>
> @@ -705,6 +703,7 @@ SYSCALL_DEFINE2(timer_gettime, timer_t, timer_id,
> if (!timr)
> return -EINVAL;
>
> + memset(&cur_setting64, 0, sizeof(cur_setting64));
> kc = timr->kclock;
> if (WARN_ON_ONCE(!kc || !kc->timer_get))
> ret = -EINVAL;