Re: [patch 14/18] time: export time information for KVM pvclock

[John Stultz <johnstul@us.ibm.com>]

On 10/24/2012 06:13 AM, Marcelo Tosatti wrote:
> As suggested by John, export time data similarly to how its
> done by vsyscall support. This allows KVM to retrieve necessary
> information to implement vsyscall support in KVM guests.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Thanks Marcelo, I like this much better then what you were proposing 
privately earlier!

Fairly minor nit below.

> Index: vsyscall/kernel/time/timekeeping.c
> ===================================================================
> --- vsyscall.orig/kernel/time/timekeeping.c
> +++ vsyscall/kernel/time/timekeeping.c
> @@ -21,6 +21,7 @@
>   #include <linux/time.h>
>   #include <linux/tick.h>
>   #include <linux/stop_machine.h>
> +#include <linux/pvclock_gtod.h>
>
>
>   static struct timekeeper timekeeper;
> @@ -180,6 +181,79 @@ static inline s64 timekeeping_get_ns_raw
>   	return nsec + arch_gettimeoffset();
>   }
>
> +static RAW_NOTIFIER_HEAD(pvclock_gtod_chain);
> +
> +/**
> + * pvclock_gtod_register_notifier - register a pvclock timedata update listener
> + *
> + * Must hold write on timekeeper.lock
> + */
> +int pvclock_gtod_register_notifier(struct notifier_block *nb)
> +{
> +	struct timekeeper *tk = &timekeeper;
> +	unsigned long flags;
> +	int ret;
> +
> +	write_seqlock_irqsave(&tk->lock, flags);
> +	ret = raw_notifier_chain_register(&pvclock_gtod_chain, nb);
> +	write_sequnlock_irqrestore(&tk->lock, flags);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pvclock_gtod_register_notifier);
> +
> +/**
> + * pvclock_gtod_unregister_notifier - unregister a pvclock
> + * timedata update listener
> + *
> + * Must hold write on timekeeper.lock
> + */
> +int pvclock_gtod_unregister_notifier(struct notifier_block *nb)
> +{
> +	struct timekeeper *tk = &timekeeper;
> +	unsigned long flags;
> +	int ret;
> +
> +	write_seqlock_irqsave(&tk->lock, flags);
> +	ret = raw_notifier_chain_unregister(&pvclock_gtod_chain, nb);
> +	write_sequnlock_irqrestore(&tk->lock, flags);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pvclock_gtod_unregister_notifier);
> +
> +struct pvclock_gtod_data pvclock_gtod_data;
> +EXPORT_SYMBOL_GPL(pvclock_gtod_data);
> +
> +static void update_pvclock_gtod(struct timekeeper *tk)
> +{
> +	struct pvclock_gtod_data *vdata = &pvclock_gtod_data;
> +
> +	write_seqcount_begin(&vdata->seq);
> +
> +	/* copy pvclock gtod data */
> +	vdata->clock.vclock_mode	= tk->clock->archdata.vclock_mode;
> +	vdata->clock.cycle_last		= tk->clock->cycle_last;
> +	vdata->clock.mask		= tk->clock->mask;
> +	vdata->clock.mult		= tk->mult;
> +	vdata->clock.shift		= tk->shift;
> +
> +	vdata->monotonic_time_sec	= tk->xtime_sec
> +					+ tk->wall_to_monotonic.tv_sec;
> +	vdata->monotonic_time_snsec	= tk->xtime_nsec
> +					+ (tk->wall_to_monotonic.tv_nsec
> +						<< tk->shift);
> +	while (vdata->monotonic_time_snsec >=
> +					(((u64)NSEC_PER_SEC) << tk->shift)) {
> +		vdata->monotonic_time_snsec -=
> +					((u64)NSEC_PER_SEC) << tk->shift;
> +		vdata->monotonic_time_sec++;
> +	}
> +
> +	write_seqcount_end(&vdata->seq);
> +	raw_notifier_call_chain(&pvclock_gtod_chain, 0, NULL);
> +}
> +

My only request is could the update_pvclock_gtod() be implemented 
similarly to the update_vsyscall, where the update function lives in the 
pvclock code (maybe using a weak symbol or something) so we don't have 
to have all these pvclock details in the timekeeping core?

thanks
-john