From owner-freebsd-hackers@FreeBSD.ORG Wed Feb 6 15:58:35 2013 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 91410607; Wed, 6 Feb 2013 15:58:35 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) by mx1.freebsd.org (Postfix) with ESMTP id 05C4C633; Wed, 6 Feb 2013 15:58:34 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.6/8.14.6) with ESMTP id r16FwUu3014881; Wed, 6 Feb 2013 17:58:30 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.7.4 kib.kiev.ua r16FwUu3014881 Received: (from kostik@localhost) by tom.home (8.14.6/8.14.6/Submit) id r16FwUVK014880; Wed, 6 Feb 2013 17:58:30 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 6 Feb 2013 17:58:30 +0200 From: Konstantin Belousov To: Ian Lepore Subject: Re: Request for review, time_pps_fetch() enhancement Message-ID: <20130206155830.GX2522@kib.kiev.ua> References: <1360125698.93359.566.camel@revolution.hippie.lan> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tcpvmIcflXmDwk6w" Content-Disposition: inline In-Reply-To: <1360125698.93359.566.camel@revolution.hippie.lan> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on tom.home Cc: "freebsd-hackers@freebsd.org" X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Feb 2013 15:58:35 -0000 --tcpvmIcflXmDwk6w Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Feb 05, 2013 at 09:41:38PM -0700, Ian Lepore wrote: > I'd like feedback on the attached patch, which adds support to our > time_pps_fetch() implementation for the blocking behaviors described in > section 3.4.3 of RFC 2783. The existing implementation can only return > the most recently captured data without blocking. These changes add the > ability to block (forever or with timeout) until a new event occurs. >=20 > -- Ian >=20 > Index: sys/kern/kern_tc.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/kern/kern_tc.c (revision 246337) > +++ sys/kern/kern_tc.c (working copy) > @@ -1446,6 +1446,50 @@ > * RFC 2783 PPS-API implementation. > */ > =20 > +static int > +pps_fetch(struct pps_fetch_args *fapi, struct pps_state *pps) > +{ > + int err, timo; > + pps_seq_t aseq, cseq; > + struct timeval tv; > + > + if (fapi->tsformat && fapi->tsformat !=3D PPS_TSFMT_TSPEC) > + return (EINVAL); > + > + /* > + * If no timeout is requested, immediately return whatever values were > + * most recently captured. If timeout seconds is -1, that's a request > + * to block without a timeout. WITNESS won't let us sleep forever > + * without a lock (we really don't need a lock), so just repeatedly > + * sleep a long time. > + */ Regarding no need for the lock, it would just move the implementation into the low quality one, for the case when one timestamp capture is lost and caller of time_pps_fetch() sleeps until next pps event is generated. I understand the desire to avoid lock, esp. in the pps_event() called =66rom the arbitrary driver context. But the race is also real. > + if (fapi->timeout.tv_sec || fapi->timeout.tv_nsec) { > + if (fapi->timeout.tv_sec =3D=3D -1) > + timo =3D 0x7fffffff; > + else { > + tv.tv_sec =3D fapi->timeout.tv_sec; > + tv.tv_usec =3D fapi->timeout.tv_nsec / 1000; > + timo =3D tvtohz(&tv); > + } > + aseq =3D pps->ppsinfo.assert_sequence; > + cseq =3D pps->ppsinfo.clear_sequence; > + while (aseq =3D=3D pps->ppsinfo.assert_sequence && > + cseq =3D=3D pps->ppsinfo.clear_sequence) { Note that compilers are allowed to optimize these accesses even over the sequential point, which is the tsleep() call. Only accesses to volatile objects are forbidden to be rearranged. I suggest to add volatile casts to pps in the loop condition. > + err =3D tsleep(pps, PCATCH, "ppsfch", timo); > + if (err =3D=3D EWOULDBLOCK && fapi->timeout.tv_sec =3D=3D -1) { > + continue; > + } else if (err !=3D 0) { > + return (err); > + } > + } > + } > + > + pps->ppsinfo.current_mode =3D pps->ppsparam.mode; > + fapi->pps_info_buf =3D pps->ppsinfo; > + > + return (0); > +} > + > int > pps_ioctl(u_long cmd, caddr_t data, struct pps_state *pps) > { > @@ -1485,13 +1529,7 @@ > return (0); > case PPS_IOC_FETCH: > fapi =3D (struct pps_fetch_args *)data; > - if (fapi->tsformat && fapi->tsformat !=3D PPS_TSFMT_TSPEC) > - return (EINVAL); > - if (fapi->timeout.tv_sec || fapi->timeout.tv_nsec) > - return (EOPNOTSUPP); > - pps->ppsinfo.current_mode =3D pps->ppsparam.mode; > - fapi->pps_info_buf =3D pps->ppsinfo; > - return (0); > + return (pps_fetch(fapi, pps)); > #ifdef FFCLOCK > case PPS_IOC_FETCH_FFCOUNTER: > fapi_ffc =3D (struct pps_fetch_ffc_args *)data; > @@ -1540,7 +1578,7 @@ > void > pps_init(struct pps_state *pps) > { > - pps->ppscap |=3D PPS_TSFMT_TSPEC; > + pps->ppscap |=3D PPS_TSFMT_TSPEC | PPS_CANWAIT; > if (pps->ppscap & PPS_CAPTUREASSERT) > pps->ppscap |=3D PPS_OFFSETASSERT; > if (pps->ppscap & PPS_CAPTURECLEAR) > @@ -1680,6 +1718,9 @@ > hardpps(tsp, ts.tv_nsec + 1000000000 * ts.tv_sec); > } > #endif > + > + /* Wakeup anyone sleeping in pps_fetch(). */ > + wakeup(pps); > } > =20 > /* > _______________________________________________ > freebsd-hackers@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-hackers > To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org" --tcpvmIcflXmDwk6w Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iQIcBAEBAgAGBQJREn2lAAoJEJDCuSvBvK1Bz7YP/R5+JihMt78WpNfaMSMlPHyE Kl5GEmAZVojEt8kBp0/uGHzR7VyN9U18lTgQlrC/BJhAlJZlC7vJE856+VS+L5zG Cz3D4+Psx+5IeUBeL1Vq52fJ3BcRWz3dN9XRyK66vFxS0/xWqv2+F6VBMN/pWXI3 1vJmcvcanhLF17hdSF7HwLPweqVyVLf3k43SR/MxVCWkX+Yga9hz0RXfWSJAH4hz zClNLKHsTFJfoHeSLAEJkDXoBoC/JbpWywVjhlmecnldNZKjvE+d5l2Egz39uNyn mRus+gUKbYdYEffadBL49WrGveV3MRNnJNN8QnuB+/9Dt+/x8zkqSqDYSBxoA9Ac b2gYhMw8ytn+oMfGVlqzO9rw4/EBfGAQrkUrLDl9orSYj+P4+Fm8p8Kb6pmzWfLN e7za4VZrPE3fTQecDlZH9ZUmY3Gq9HAdM2SyNeuZiOJYoNEghXch/VXAHQOUpJCC n55ipGFTgzRxfaIE810uu9NkSvlx2iAVJJw48qYjABCejvmNbg59WiBL7IyF/5Ka j/wH2ZnuhLfj4OhSfqVXuMzMSizkGLDNB6VtXmEmvehKhSZ+7VDjBmcaLCcbkgsg NdahVIxLoJtPDW4dv1LxmBonMPvji/nZZUz9Lc1MXJTLs9/BnCHrnVir+PDkgyHl sIC4C6JKgoqjq41k/Flm =9TbN -----END PGP SIGNATURE----- --tcpvmIcflXmDwk6w--