From owner-freebsd-arch@freebsd.org Fri Dec 11 17:16:56 2020 Return-Path: Delivered-To: freebsd-arch@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 9893B4B59C6 for ; Fri, 11 Dec 2020 17:16:56 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from smtp-out-so.shaw.ca (smtp-out-so.shaw.ca [64.59.136.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CsyBc23WKz4dkM; Fri, 11 Dec 2020 17:16:55 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from spqr.komquats.com ([70.67.229.168]) by shaw.ca with ESMTPA id nm2FkkXGwbYg3nm2HkPExc; Fri, 11 Dec 2020 10:16:54 -0700 X-Authority-Analysis: v=2.4 cv=Q4RsX66a c=1 sm=1 tr=0 ts=5fd3a986 a=7AlCcx2GqMg+lh9P3BclKA==:117 a=7AlCcx2GqMg+lh9P3BclKA==:17 a=xqWC_Br6kY4A:10 a=kj9zAlcOel0A:10 a=zTNgK-yGK50A:10 a=6I5d2MoRAAAA:8 a=YxBL1-UpAAAA:8 a=EkcXrb_YAAAA:8 a=Ob6Gysnb11nFkN38hccA:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 a=Ia-lj3WSrqcvXOmTRaiG:22 a=LK5xJRSDVpKd5WXXoEvA:22 Received: from slippy.cwsent.com (slippy [IPv6:fc00:1:1:1::5b]) by spqr.komquats.com (Postfix) with ESMTPS id AF748A34; Fri, 11 Dec 2020 09:16:50 -0800 (PST) Received: from slippy (localhost [127.0.0.1]) by slippy.cwsent.com (8.16.1/8.16.1) with ESMTP id 0BBHGo3v069667; Fri, 11 Dec 2020 09:16:50 -0800 (PST) (envelope-from Cy.Schubert@cschubert.com) Message-Id: <202012111716.0BBHGo3v069667@slippy.cwsent.com> X-Mailer: exmh version 2.9.0 11/07/2018 with nmh-1.7.1 Reply-to: Cy Schubert From: Cy Schubert X-os: FreeBSD X-Sender: cy@cwsent.com X-URL: http://www.cschubert.com/ To: Konstantin Belousov cc: Cy Schubert , Poul-Henning Kamp , Ian Lepore , freebsd-arch@freebsd.org Subject: Re: struct timex and Linux adjtimex() In-reply-to: References: <202012030523.0B35NsG7003810@slippy.cwsent.com> <4086.1606982335@critter.freebsd.dk> <5e0db735b29f1ece02521871b2cd392c3467101d.camel@freebsd.org> <25487.1607029223@critter.freebsd.dk> <202012032203.0B3M3VJx004269@slippy.cwsent.com> <25989.1607033614@critter.freebsd.dk> <202012032258.0B3MwqVQ004875@slippy.cwsent.com> <202012040114.0B41EuFc006408@slippy.cwsent.com> Comments: In-reply-to Konstantin Belousov message dated "Fri, 11 Dec 2020 17:15:23 +0200." Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Fri, 11 Dec 2020 09:16:49 -0800 X-CMAE-Envelope: MS4xfO/t65nYGZI13jZChNDBBp8+/Rg1VsFqZCt9PwVCpeGd+EqgHGiIunb3yiafLrZYMoGGKED3IiVdi6rtuSgMPkZA2oIsJzvK14MXC7lyNdlhTtclfBwd YxlCyR7CXS3E/EsUvYv3f9jeAd++Z41zoY1vWZpFp/mUyTxOjzdBW9zZ6zI3dEUvM2pkK1BOMSjWdJNuuwVjxnoYNb5+Gv5qcE22UKmr2fMTMMqQxrfshYrz 57crZLLIJzAhP+0RQRTfmTF9YSJY/VEphsae0t2T+kP2i1KFGvwMJZVMjLdYnB9t X-Rspamd-Queue-Id: 4CsyBc23WKz4dkM X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Dec 2020 17:16:56 -0000 In message , Konstantin Belousov writes: > On Thu, Dec 03, 2020 at 05:14:56PM -0800, Cy Schubert wrote: > > In message , Konstantin Belousov writes: > > > On Thu, Dec 03, 2020 at 02:58:52PM -0800, Cy Schubert wrote: > > > > In message <25989.1607033614@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 timespe > c 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 conflict > s > > > > 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. > > Ok, I went ahead and wrote https://reviews.freebsd.org/D27571 . > This should handle all notes from the conversation. Thanks. I just finished mine and tested it last night. BTW, they want it to return clock_gettime() output atomically. I suppose I could add that later. -- Cheers, Cy Schubert FreeBSD UNIX: Web: https://FreeBSD.org NTP: Web: https://nwtime.org The need of the many outweighs the greed of the few.