Date: Sun, 10 Feb 2013 12:41:45 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Ian Lepore <ian@FreeBSD.org> Cc: "freebsd-hackers@freebsd.org" <freebsd-hackers@FreeBSD.org> Subject: Re: Request for review, time_pps_fetch() enhancement Message-ID: <20130210104145.GJ2522@kib.kiev.ua> In-Reply-To: <1360365220.4545.42.camel@revolution.hippie.lan> References: <1360125698.93359.566.camel@revolution.hippie.lan> <20130206155830.GX2522@kib.kiev.ua> <1360365220.4545.42.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
--+gGPykIdtTMJQYdC
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Feb 08, 2013 at 04:13:40PM -0700, Ian Lepore wrote:
> On Wed, 2013-02-06 at 17:58 +0200, Konstantin Belousov wrote:
> > 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 retu=
rn
> > > 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
> >=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 w=
ere
> > > + * most recently captured. If timeout seconds is -1, that's a requ=
est
> > > + * 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 i=
nto
> > 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.
> >=20
> > I understand the desire to avoid lock, esp. in the pps_event() called
> > from the arbitrary driver context. But the race is also real.
> >=20
>=20
> What race? A user of the pps interface understands that there is one
> event per second, and understands that if you ask to block until the
> next event at approximately the time that event is expected to occur,
> then it is ambiguous whether the call completes almost-immediately or in
> about 1 second.
>=20
> Looking at it another way, if a blocking call is made right around the
> time of the PPS, the thread could get preempted before getting to
> pps_fetch() function and not get control again until after the PPS has
> occurred. In that case it's going to block for about a full second,
> even though the call was made before top-of-second. That situation is
> exactly the same with or without locking, so what extra functionality is
> gained with locking? What guarantee does locking let us make to the
> caller that the lockless code doesn't?
No guarantees, but I noted in the original reply that this is about the
quality of the implementation and not about correctness.
As I said there as well, I am not sure that any locking can be useful
for the situation at all.
--+gGPykIdtTMJQYdC
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)
iQIcBAEBAgAGBQJRF3loAAoJEJDCuSvBvK1BdfkQAJ23vyNMtY8Hsue3Vvf1uHd8
pJFQr/6NBIeaCIH057H6G8aWzmEUStoY7Gve8mzFr0oBo0zkb6ttTjINkbvaNI0T
48htcdEUx+elnFs8bIzlPstEsluJGg4TcJG3NbVBJzu1Hmjyt6MqsgbI9kOyJpmB
X1JjwMDNT0TiPL8gSKJcNMFT2FYc2186Ol4vVyeBS3oQCnzFHSfUpvF350felDo5
8sS0A50E1owRdcNje8FWtShwFmVHIVgwSbuSMaf5Ga4ks2eupc4HUtCYQAxumOzY
P+NLlOfEWSmzQfDbkXk1527CGn6lKai+SZVXOp9OY081XcwEbuLZjja84JK9WyQ3
hb+RMOPY6tEMxYg4VrkeuskMV3S0H95EHP09/3neHOjxRDqFPgk+plLe3r4zkQLw
up72SO7VEMqvsN8R8vXtN89uetUaC50LbcV15iLdplZRtcQcIgkq6oAV/9131IUK
EkZeRGBJMGeM78wRdSnPilB7zpxLcBe0U7WXkvmbB3n0nAfsQRj3QVi6YGzDZ6Cq
0SZm1oQUUFyPjja+afdjOmIy69G6WbrA1AY2YqSHV7/65sYqg7bQEXwvEDfbGgik
xfsYqBe/DQHnluSpjgRyRK2pXIQmu5T6NP7PFJBcl2e6ASWQhJZA3rCSSWiH8Gjf
T8VOLEKByW58VUFEZc1+
=uLon
-----END PGP SIGNATURE-----
--+gGPykIdtTMJQYdC--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130210104145.GJ2522>