Re: [PATCHv1] timekeeping: Update multiplier when NTP frequency is set directly
On Tue, May 29, 2018 at 10:42:05AM -0700, John Stultz wrote: > On Tue, May 29, 2018 at 3:53 AM, Miroslav Lichvar wrote: > > -void update_wall_time(void) > > +static void timekeeping_advance(bool force_update) > > This is kind of a nit, but mind switching out bool for an enum? Using > something like TK_ADV_NORMAL and TK_ADV_FORCE? > > > +void update_wall_time(void) > > +{ > > + timekeeping_advance(false); > > +} > > The enum makes usage like timekeeping_advance(false) a little less > opaque to the reader ("Wait, don't advance? Let me go look at the > function"). > > We got bitten by this earlier when we had the old > "timekeeping_update(tk, true, false, true)" usage. Ok. That make sense. I'll send a v2. Thanks, -- Miroslav Lichvar
Re: [PATCHv1] timekeeping: Update multiplier when NTP frequency is set directly
On Tue, May 29, 2018 at 10:42:05AM -0700, John Stultz wrote: > On Tue, May 29, 2018 at 3:53 AM, Miroslav Lichvar wrote: > > -void update_wall_time(void) > > +static void timekeeping_advance(bool force_update) > > This is kind of a nit, but mind switching out bool for an enum? Using > something like TK_ADV_NORMAL and TK_ADV_FORCE? > > > +void update_wall_time(void) > > +{ > > + timekeeping_advance(false); > > +} > > The enum makes usage like timekeeping_advance(false) a little less > opaque to the reader ("Wait, don't advance? Let me go look at the > function"). > > We got bitten by this earlier when we had the old > "timekeeping_update(tk, true, false, true)" usage. Ok. That make sense. I'll send a v2. Thanks, -- Miroslav Lichvar
Re: [PATCHv1] timekeeping: Update multiplier when NTP frequency is set directly
On Tue, May 29, 2018 at 3:53 AM, Miroslav Lichvar wrote: > When the NTP frequency is set directly from userspace using the > ADJ_FREQUENCY or ADJ_TICK timex mode, immediately update the > timekeeper's multiplier instead of waiting for the next tick. > > This removes a hidden non-deterministic delay in setting of the > frequency and allows an extremely tight control of the system clock > with update rates close to or even exceeding the kernel HZ. > > Cc: Thomas Gleixner > Cc: John Stultz > Cc: Richard Cochran > Cc: Prarit Bhargava > Signed-off-by: Miroslav Lichvar > --- > > Notes: > RFC->v1: > - added a new parameter to force the update of the timekeeper to the > current > NTP tick length only from adjtimex() > - added timekeeping_advance() to keep the parameter local to timekeeping.c > > kernel/time/timekeeping.c | 23 ++- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > index 49cbceef5deb..5524c07d43e3 100644 > --- a/kernel/time/timekeeping.c > +++ b/kernel/time/timekeeping.c > @@ -2021,11 +2021,11 @@ static u64 logarithmic_accumulation(struct timekeeper > *tk, u64 offset, > return offset; > } > > -/** > - * update_wall_time - Uses the current clocksource to increment the wall time > - * > +/* > + * timekeeping_advance - Updates the timekeeper to the current time and > + * current NTP tick length > */ > -void update_wall_time(void) > +static void timekeeping_advance(bool force_update) This is kind of a nit, but mind switching out bool for an enum? Using something like TK_ADV_NORMAL and TK_ADV_FORCE? > +void update_wall_time(void) > +{ > + timekeeping_advance(false); > +} The enum makes usage like timekeeping_advance(false) a little less opaque to the reader ("Wait, don't advance? Let me go look at the function"). We got bitten by this earlier when we had the old "timekeeping_update(tk, true, false, true)" usage. thanks -john
Re: [PATCHv1] timekeeping: Update multiplier when NTP frequency is set directly
On Tue, May 29, 2018 at 3:53 AM, Miroslav Lichvar wrote: > When the NTP frequency is set directly from userspace using the > ADJ_FREQUENCY or ADJ_TICK timex mode, immediately update the > timekeeper's multiplier instead of waiting for the next tick. > > This removes a hidden non-deterministic delay in setting of the > frequency and allows an extremely tight control of the system clock > with update rates close to or even exceeding the kernel HZ. > > Cc: Thomas Gleixner > Cc: John Stultz > Cc: Richard Cochran > Cc: Prarit Bhargava > Signed-off-by: Miroslav Lichvar > --- > > Notes: > RFC->v1: > - added a new parameter to force the update of the timekeeper to the > current > NTP tick length only from adjtimex() > - added timekeeping_advance() to keep the parameter local to timekeeping.c > > kernel/time/timekeeping.c | 23 ++- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > index 49cbceef5deb..5524c07d43e3 100644 > --- a/kernel/time/timekeeping.c > +++ b/kernel/time/timekeeping.c > @@ -2021,11 +2021,11 @@ static u64 logarithmic_accumulation(struct timekeeper > *tk, u64 offset, > return offset; > } > > -/** > - * update_wall_time - Uses the current clocksource to increment the wall time > - * > +/* > + * timekeeping_advance - Updates the timekeeper to the current time and > + * current NTP tick length > */ > -void update_wall_time(void) > +static void timekeeping_advance(bool force_update) This is kind of a nit, but mind switching out bool for an enum? Using something like TK_ADV_NORMAL and TK_ADV_FORCE? > +void update_wall_time(void) > +{ > + timekeeping_advance(false); > +} The enum makes usage like timekeeping_advance(false) a little less opaque to the reader ("Wait, don't advance? Let me go look at the function"). We got bitten by this earlier when we had the old "timekeeping_update(tk, true, false, true)" usage. thanks -john
[PATCHv1] timekeeping: Update multiplier when NTP frequency is set directly
When the NTP frequency is set directly from userspace using the ADJ_FREQUENCY or ADJ_TICK timex mode, immediately update the timekeeper's multiplier instead of waiting for the next tick. This removes a hidden non-deterministic delay in setting of the frequency and allows an extremely tight control of the system clock with update rates close to or even exceeding the kernel HZ. Cc: Thomas Gleixner Cc: John Stultz Cc: Richard Cochran Cc: Prarit Bhargava Signed-off-by: Miroslav Lichvar --- Notes: RFC->v1: - added a new parameter to force the update of the timekeeper to the current NTP tick length only from adjtimex() - added timekeeping_advance() to keep the parameter local to timekeeping.c kernel/time/timekeeping.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 49cbceef5deb..5524c07d43e3 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -2021,11 +2021,11 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset, return offset; } -/** - * update_wall_time - Uses the current clocksource to increment the wall time - * +/* + * timekeeping_advance - Updates the timekeeper to the current time and + * current NTP tick length */ -void update_wall_time(void) +static void timekeeping_advance(bool force_update) { struct timekeeper *real_tk = _core.timekeeper; struct timekeeper *tk = _timekeeper; @@ -2048,7 +2048,7 @@ void update_wall_time(void) #endif /* Check if there's really nothing to do */ - if (offset < real_tk->cycle_interval) + if (offset < real_tk->cycle_interval && !force_update) goto out; /* Do some additional sanity checking */ @@ -2105,6 +2105,15 @@ void update_wall_time(void) clock_was_set_delayed(); } +/** + * update_wall_time - Uses the current clocksource to increment the wall time + * + */ +void update_wall_time(void) +{ + timekeeping_advance(false); +} + /** * getboottime64 - Return the real time of system boot. * @ts:pointer to the timespec64 to be set @@ -2332,6 +2341,10 @@ int do_adjtimex(struct timex *txc) write_seqcount_end(_core.seq); raw_spin_unlock_irqrestore(_lock, flags); + /* Update the multiplier immediately if frequency was set directly */ + if (txc->modes & (ADJ_FREQUENCY | ADJ_TICK)) + timekeeping_advance(true); + if (tai != orig_tai) clock_was_set(); -- 2.14.3
[PATCHv1] timekeeping: Update multiplier when NTP frequency is set directly
When the NTP frequency is set directly from userspace using the ADJ_FREQUENCY or ADJ_TICK timex mode, immediately update the timekeeper's multiplier instead of waiting for the next tick. This removes a hidden non-deterministic delay in setting of the frequency and allows an extremely tight control of the system clock with update rates close to or even exceeding the kernel HZ. Cc: Thomas Gleixner Cc: John Stultz Cc: Richard Cochran Cc: Prarit Bhargava Signed-off-by: Miroslav Lichvar --- Notes: RFC->v1: - added a new parameter to force the update of the timekeeper to the current NTP tick length only from adjtimex() - added timekeeping_advance() to keep the parameter local to timekeeping.c kernel/time/timekeeping.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 49cbceef5deb..5524c07d43e3 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -2021,11 +2021,11 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset, return offset; } -/** - * update_wall_time - Uses the current clocksource to increment the wall time - * +/* + * timekeeping_advance - Updates the timekeeper to the current time and + * current NTP tick length */ -void update_wall_time(void) +static void timekeeping_advance(bool force_update) { struct timekeeper *real_tk = _core.timekeeper; struct timekeeper *tk = _timekeeper; @@ -2048,7 +2048,7 @@ void update_wall_time(void) #endif /* Check if there's really nothing to do */ - if (offset < real_tk->cycle_interval) + if (offset < real_tk->cycle_interval && !force_update) goto out; /* Do some additional sanity checking */ @@ -2105,6 +2105,15 @@ void update_wall_time(void) clock_was_set_delayed(); } +/** + * update_wall_time - Uses the current clocksource to increment the wall time + * + */ +void update_wall_time(void) +{ + timekeeping_advance(false); +} + /** * getboottime64 - Return the real time of system boot. * @ts:pointer to the timespec64 to be set @@ -2332,6 +2341,10 @@ int do_adjtimex(struct timex *txc) write_seqcount_end(_core.seq); raw_spin_unlock_irqrestore(_lock, flags); + /* Update the multiplier immediately if frequency was set directly */ + if (txc->modes & (ADJ_FREQUENCY | ADJ_TICK)) + timekeeping_advance(true); + if (tai != orig_tai) clock_was_set(); -- 2.14.3