Date: Tue, 12 Jun 2012 19:21:05 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Hans Petter Selasky <hselasky@c2i.net> Cc: "svn-src-head@freebsd.org" <svn-src-head@FreeBSD.org>, "svn-src-all@freebsd.org" <svn-src-all@FreeBSD.org>, "src-committers@freebsd.org" <src-committers@FreeBSD.org>, Pawel Jakub Dawidek <pjd@FreeBSD.org>, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r236909 - head/sbin/hastd Message-ID: <20120612183547.C2680@besplex.bde.org> In-Reply-To: <201206120951.38492.hselasky@c2i.net> References: <201206112359.41892.hselasky@c2i.net> <20120612130912.O1895@besplex.bde.org> <201206120951.38492.hselasky@c2i.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 12 Jun 2012, Hans Petter Selasky wrote: > On Tuesday 12 June 2012 05:49:33 Bruce Evans wrote: >> On Mon, 11 Jun 2012, Hans Petter Selasky wrote: >>> On Monday 11 June 2012 22:21:51 Hans Petter Selasky wrote: >>>> On Monday 11 June 2012 22:05:07 Pawel Jakub Dawidek wrote: >>>>> On Mon, Jun 11, 2012 at 07:21:00PM +0000, Hans Petter Selasky wrote: >>>>>> Author: hselasky >>>>>> Date: Mon Jun 11 19:20:59 2012 >>>>>> New Revision: 236909 >>>>>> URL: http://svn.freebsd.org/changeset/base/236909 > >> The point is in 2038, on systems with 32-bit signed time_t's. None >> should exist then. Even if time_t is 32 bits, it can be unsigned, and >> never become negative, and work until 2106. Changing time_t from >> signed to unsigned would break mainly times before the Epoch, which >> are invalid for current times anyway, and expose broken code which >> assumes that time_t is signed. > > Lets assume you need to reboot the system at some point and that solves the > problem. Yes, that solves it for >= 86 years after rebooting, provided the clock id is CLOCK_MONOTONIC and the time that this clock is relative to is the boot time. >>> functionality stop working then, because pthread_cond_timedwait() has a >>> check for negative time? >> >> This check is just to prevent invalid times. It does prevents hacks like >> the kernel treating negative times as large unsigned ones so that the >> result is much the same as changing time_t to unsigned, without actually >> changing time_t. >> >>> Or is hastd wrong, that it can compute a timeout offset which is outside >>> the valid range, because it uses a simple: >>> >>> tv_sec += timeout? >>> tv_sec %= 1000000000; /* Is this perhaps missing in hastd and other >>> drivers ???? */ >>> >>> What is the modulus which should be used for tv_sec? >> >> `tv_sec %= ANY' makes no sense. >> >> With CLOCK_MONOTONIC, signed 32-bit time_t's work for 86 years after the >> unspecified point in the past that CLOCK_MONOTONIC is relative to. This >> should be enough for anyone, provided the unspecified point is the boot >> time. hastd also uses mostly-relative, mostly-monotonic, often 32-bit >> signed in timeouts internally. E.g., in the function that seems to be >> causing problems: > > When CLOCK_REALTIME finally goes negative, then pthread_cond_timedwait() will > simply return an error code, due to the negative tv_sec check in there! I see > other clients like sendmail, using even simpler formats like: Are you going to wait around for 86 years after rebooting for that? :-). > tv_nsec = 0; > tv_sec = time(NULL); > > If this piece of code stops working at a given data, regardless of uptime, > shouldn't that be fixed now? Only if you urgently need current systems 32-bit signed time_t, which are rebooted tomorrow with this fix, to not need another reboot for 86 years. Actually `tv_sec = time(NULL);' will overflow on only 26 years on such systems, but time() use not usable together with CLOCK_MONOTONIC. 32-bit signed time_t's are a more general problem. > ... > A final question on the matter: > > I tried to use CLOCK_MONOTONIC_FAST when setting up the condition variable. > That did not appear to be support in 9-stable. Is there a list of supported > clocks anywhere? Don't know, but the list in clock_gettime(2) for -current seems to have them all except CLOCK_THREAD_CPUTIME_ID. That one is also badly named: - verbosely named - has an _ID suffix, unlike all the others Since it is like CLOCK_PROF (the differences are that it is for threads instead of processes, and interrupt times are broken in a different way for it), it should be named more like CLOCK_PROF. Of course the kernel source code has the correct list. Most of these clock ids are only supported by clock_gettime() and clock_getres(). Most of them arent't writeable, so clock_settime() doesn't apply to them. clock_settime(2) is identical with clock_gettime(2), and doesn't say anything about which clocks are read-only -- you have to try to set them to see which, or you can read the POSIX spec to see that CLOCK_MONOTONIC must be read-only. clock_settime(2) also fails to document the (bad) errno for failure with CLOCK_MONOTONIC. It only says "the following error codes _may_ be set", with EINVAL meaning that the clock id was invalid. But in POSIX, EINVAL for just clock_settime() is grossly overloaded; it means any of: - clock_id for unknown clock (this applies to all the functions; the rest are for clock_settime()) - the tp arg (sic; should be the value pointed to be the tp arg) is out of bounds for the specified clock_id - the tp arg (sic) has an out of bounds nanosecond value - the clock_id arg is CLOCK_MONOTONIC. I don't like the explosion of clock ids in FreeBSD, and support for them outside of clock_gettime() and clock_getres() seems to be nonexistent. In man pages for POSIX realtime timers, only the standard CLOCK_REALTIME and CLOCK_MONOTONIC are documented, and the code seems to agree. FreeBSD aliases like CLOCK_REALTIME_PRECISE will work because they are just spelling errors. FreeBSD extensions like CLOCK_REALTIME_FAST won't work for timers, but timers are already "FAST" (really "!PRECISE"). Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120612183547.C2680>