struct timex and Linux adjtimex()

[Cy Schubert]

In message <X8l7bjf2aEPFRdYj at kib.kiev.ua>, Konstantin Belousov writes:
> On Thu, Dec 03, 2020 at 02:58:52PM -0800, Cy Schubert wrote:
> > In message <25989.1607033614 at critter.freebsd.dk>, "Poul-Henning Kamp" 
> > writes:
> > > --------
> > > Cy Schubert writes:
> > >
> > > > I will go back 
> > > > with my initial proposal of a timespec add/subtract syscall takes a 
> > > > timespec as input increments or decrements the clock by the timespec an
> d 
> > > > returns a timespec with the time.
> > >
> > > I would be tempted by the clock_settime(2) "clock_id" argument.
> > >
> > > The functionality required has a LOT more commonality with
> > > clock_settime(2) than with ntp_adjtime(2), and absconding with a
> > > couple of the top bits of clock_id for "CLOCK_ADD_ADJUSTMENT" and
> > > "CLOCK_SUB_ADJUSTMENT" would be be a pretty clean solution.
> > 
> > Correct. My initial proposal was:
> > 
> > +.Fn clock_updtime "clockid_t clock_id" "const struct timespec *itp"
> > "struct timespec *otp"
> > 
> > Briefly it does this:
> > 
> > +int
> > +kern_clock_updtime(struct thread *td, clockid_t clock_id,
> > +         const struct timespec *its, struct timespec *ots)
> Note that phk suggested using specific clock id with clock_settime(),
> and I believe that you only need one such clock id.

Correct. This is from work I stashed in my git repo from Sunday. I haven't 
updated it yet with phk's suggestions.

>
> > +{
> > + struct timespec ats;
> > + int error;
> > +
> > + if ((error = kern_clock_gettime(td, clock_id, &ats)) != 0)
> > +         return (error);
> > +
> > + timespecadd(its, &ats, &ats);
> > +
> > + if ((error = kern_clock_settime(td, clock_id, &ats)) != 0)
> > +         return (error);
> > +
> > + return(kern_clock_gettime(td, clock_id, ots));
> > +}
> This is awful, it must not be done this way.
>
> Look how tc_setclock() is implemented.  It is careful to adjust time
> with interrupts and preemption disabled, and does it by adjusting the
> source of truth, not by fetching through several layers and then hoping
> that we did not get delayed too much when pushing back.

Thanks. I'll look there.

>
> I think you need to refactor tc_setclock() somewhat to allow to specify
> offset instead of absolute value and use it as a helper.

I'll do that. I'll add phk and you as reviewers.

>
> > 
> > I can prepare a review if you want. I haven't touched the man page nor any 
> > tests yet.
> > 
> > It's affected by kib@'s https://reviews.freebsd.org/D27471, as conflicts 
> > will result. I'll wait until that's committed before continuing work on it,
>  
> > assuming this is the direction we want to go.
> This change does not affect *setclock() work above.

Thanks.


-- 
Cheers,
Cy Schubert <Cy.Schubert at cschubert.com>
FreeBSD UNIX:  <cy at FreeBSD.org>   Web:  https://FreeBSD.org
NTP:           <cy at nwtime.org>    Web:  https://nwtime.org

	The need of the many outweighs the greed of the few.