From owner-svn-src-all@FreeBSD.ORG Tue Jun 12 07:52:13 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E7FD11065672; Tue, 12 Jun 2012 07:52:12 +0000 (UTC) (envelope-from hselasky@c2i.net) Received: from swip.net (mailfe02.c2i.net [212.247.154.34]) by mx1.freebsd.org (Postfix) with ESMTP id BF6DE8FC0C; Tue, 12 Jun 2012 07:52:11 +0000 (UTC) X-T2-Spam-Status: No, hits=-0.2 required=5.0 tests=ALL_TRUSTED, BAYES_50 Received: from [176.74.212.201] (account mc467741@c2i.net HELO laptop015.hselasky.homeunix.org) by mailfe02.swip.net (CommuniGate Pro SMTP 5.4.4) with ESMTPA id 285995802; Tue, 12 Jun 2012 09:52:09 +0200 From: Hans Petter Selasky To: Bruce Evans Date: Tue, 12 Jun 2012 09:51:38 +0200 User-Agent: KMail/1.13.7 (FreeBSD/9.0-STABLE; KDE/4.7.4; amd64; ; ) References: <201206112359.41892.hselasky@c2i.net> <20120612130912.O1895@besplex.bde.org> In-Reply-To: <20120612130912.O1895@besplex.bde.org> X-Face: 'mmZ:T{)),Oru^0c+/}w'`gU1$ubmG?lp!=R4Wy\ELYo2)@'UZ24N@d2+AyewRX}mAm; Yp |U[@, _z/([?1bCfM{_"B<.J>mICJCHAzzGHI{y7{%JVz%R~yJHIji`y>Y}k1C4TfysrsUI -%GU9V5]iUZF&nRn9mJ'?&>O MIME-Version: 1.0 Content-Type: Text/Plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Message-Id: <201206120951.38492.hselasky@c2i.net> Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" , Pawel Jakub Dawidek Subject: Re: svn commit: r236909 - head/sbin/hastd X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jun 2012 07:52:13 -0000 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