linux-kernel - Re: [PATCH 2/2] time: Cleanup offs_real/wall_to_mono and offs_boot/total_sleep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5007FF6C.3060302@redhat.com>
Date:	Thu, 19 Jul 2012 08:37:00 -0400
From:	Prarit Bhargava <prarit@...hat.com>
To:	John Stultz <john.stultz@...aro.org>
CC:	lkml <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Richard Cochran <richardcochran@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 2/2] time: Cleanup offs_real/wall_to_mono and offs_boot/total_sleep_time
 updates



On 07/18/2012 09:19 PM, John Stultz wrote:
> For performance reasons, we maintain ktime_t based duplicates of
> wall_to_monotonic (offs_real) and total_sleep_time (offs_boot).
> 
> Since large problems could occur (such as the resume regression
> on 3.5-rc7, or the leapsecond hrtimer issue) if these value pairs
> were to be inconsistently updated, this patch this cleans up how
> we modify these value pairs to ensure we are always consistent.
> 
> As a side-effect this is also more efficient as we only
> caulculate the duplicate values when they are changed,
> rather then every update_wall_time call.
> 
> This also provides WARN_ONs to detect if future changes break
> the invariants.
> 
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Cc: Richard Cochran <richardcochran@...il.com>
> Cc: Prarit Bhargava <prarit@...hat.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: John Stultz <john.stultz@...aro.org>

<snip>

> @@ -1024,11 +1041,18 @@ static inline void accumulate_nsecs_to_secs(struct timekeeper *tk)
>  
>  		/* Figure out if its a leap sec and apply if needed */
>  		leap = second_overflow(tk->xtime_sec);
> -		tk->xtime_sec += leap;
> -		tk->wall_to_monotonic.tv_sec -= leap;
> -		if (leap)
> -			clock_was_set_delayed();
> +		if (unlikely(leap)) {

I'm likely a bit behind the times with this comment ...

I thought someone did a comparison of the usage of unlikely() within the kernel
and found that it didn't really add that much.  I'm not strongly opposed to it
in anyway, it is just that I'm curious about unlikely()'s continued usage within
the kernel; does it really add anything *other* than code readability at this point?

> +			struct timespec ts;
> +
> +			tk->xtime_sec += leap;
>  
> +			ts.tv_sec = leap;
> +			ts.tv_nsec = 0;

I wonder if this is true or not when the kernel handles the leap second.  I
suppose, in theory it is ... but it might be ahead by a bit.  I guess it is
"close enough" ;)

> +			tk_set_wall_to_mono(tk,
> +				timespec_sub(tk->wall_to_monotonic, ts));
> +
> +			clock_was_set_delayed();
> +		}
>  	}
>  }
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/