lkml.org 
[lkml]   [2007]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] Fix discrepancy between VDSO based gettimeofday() and sys_gettimeofday().
From
Date
On Fri, 2007-08-31 at 13:41 +1000, Tony Breeds wrote:
> On platforms that copy sys_tz into the vdso (currently only x86_64, soon
> to include powerpc), it is possible for the vdso to get out of sync if a
> user calls (admittedly unusual) settimeofday(NULL, ptr).
>
> This patch adds a hook for architectures that set CONFIG_GENERIC_TIME_VSYSCALL
> to ensure when sys_tz is updated they can also updater their copy in the vdso.

Ah. Yea, so after a sys_tz update, until the next timer tick, its
possible to get the stale value. Good catch.

One little comment below.

> Signed-off-by: Tony Breeds <tony@bakeyournoodle.com>
> Cc: Andi Kleen <andi@firstfloor.org>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: John Stultz <johnstul@us.ibm.com>
> ---
>
> Only compile tested for IA64
> Testcase at: http://bakeyournoodle.com/~tony/settimeofday-0.0/
> I guess this is aimed at 2.6.24
>
> arch/ia64/kernel/time.c | 5 +++++
> arch/x86_64/kernel/vsyscall.c | 11 ++++++++++-
> include/linux/clocksource.h | 5 +++++
> kernel/time.c | 2 ++
> 4 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
> index 98cfc90..2bb8421 100644
> --- a/arch/ia64/kernel/time.c
> +++ b/arch/ia64/kernel/time.c
> @@ -371,6 +371,11 @@ ia64_setup_printk_clock(void)
> ia64_printk_clock = ia64_itc_printk_clock;
> }
>
> +/* IA64 doesn't cache the timezone */
> +void update_vsyscall_tz(void)
> +{
> +}
> +
> void update_vsyscall(struct timespec *wall, struct clocksource *c)
> {
> unsigned long flags;
> diff --git a/arch/x86_64/kernel/vsyscall.c b/arch/x86_64/kernel/vsyscall.c
> index 06c3494..4ff78b0 100644
> --- a/arch/x86_64/kernel/vsyscall.c
> +++ b/arch/x86_64/kernel/vsyscall.c
> @@ -66,6 +66,16 @@ struct vsyscall_gtod_data __vsyscall_gtod_data __section_vsyscall_gtod_data =
> .sysctl_enabled = 1,
> };
>
> +void update_vsyscall_tz(void)
> +{
> + unsigned long flags;
> +
> + write_seqlock_irqsave(&vsyscall_gtod_data.lock, flags);
> + /* sys_tz has changed */
> + vsyscall_gtod_data.sys_tz = sys_tz;
> + write_sequnlock_irqrestore(&vsyscall_gtod_data.lock, flags);
> +}
> +
> void update_vsyscall(struct timespec *wall_time, struct clocksource *clock)
> {
> unsigned long flags;
> @@ -79,7 +89,6 @@ void update_vsyscall(struct timespec *wall_time, struct clocksource *clock)
> vsyscall_gtod_data.clock.shift = clock->shift;
> vsyscall_gtod_data.wall_time_sec = wall_time->tv_sec;
> vsyscall_gtod_data.wall_time_nsec = wall_time->tv_nsec;
> - vsyscall_gtod_data.sys_tz = sys_tz;
> vsyscall_gtod_data.wall_time_nsec = wall_time->tv_nsec;
> vsyscall_gtod_data.wall_to_monotonic = wall_to_monotonic;
> write_sequnlock_irqrestore(&vsyscall_gtod_data.lock, flags);


Hmm.. I might just extend update_vsyscall() so it takes a timezone and
then it only updates bits that have non-null pointers. Then call it from
sys_settimeofday(). This cuts down the functions each arch has to
implement, and also makes sure we get the full update each tick.

Alternatively update_vsyscall_tz() could live in timekeeping.c, grab the
appropriate locks and static data and call update_vsyscall(), also
minimizing the per-arch logic.

But your way is correct, and fixes a bug, so I won't stand in the way of
it.

-john

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-08-31 22:49    [W:0.063 / U:0.632 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site