Date: Fri, 9 Jun 2017 13:12:06 -0700
From: Andrei Vagin <avagin@virtuozzo.com>
To: tip-bot for Jacob Shin <tipbot@zytor.com>
Cc: linux-tip-commits@vger.kernel.org, peterz@infradead.org,
        john.stultz@linaro.org, mingo@kernel.org, linux-kernel@vger.kernel.org,
        tglx@linutronix.de, hpa@zytor.com,
        Cyrill Gorcunov <gorcunov@openvz.org>
Subject: Re: [tip:timers/core] posix-timers: Zero settings value in common
 code
Message-ID: <20170609201156.GB21491@outlook.office365.com>
References: <tip-eabdec04385376d560078992710104cc7be2ce1b@git.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
In-Reply-To: <tip-eabdec04385376d560078992710104cc7be2ce1b@git.kernel.org>
User-Agent: Mutt/1.8.0 (2017-02-23)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jun 2017 20:12:26.9400 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB0751
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 4403
Lines: 107

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;