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
> >>>>
>
Hi,
> 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.
>
> > 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:
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?
> % static __inline bool
> % cv_timedwait(pthread_cond_t *cv, pthread_mutex_t *lock, int timeout)
> % {
> % struct timespec ts;
> % int error;
> %
> % if (timeout == 0) {
> % cv_wait(cv, lock);
> % return (false);
> % }
> %
> % error = clock_gettime(CLOCK_MONOTONIC, &ts);
>
> Style bug (corrupt tab in Oct 22 2011 version).
I think I fixed this style bug when reverting.
>
> % PJDLOG_ASSERT(error == 0);
> % ts.tv_sec += timeout;
>
> This converts to absolute monotonic time. Even a 16-bit signed int is
> probably enough for `timeout'.
>
> % error = pthread_cond_timedwait(cv, lock, &ts);
> % PJDLOG_ASSERT(error == 0 || error == ETIMEDOUT);
> % return (error == ETIMEDOUT);
> % }
>
> I don't see any bugs here.
Thanks for feedback Bruce.
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?
--HPS
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
