From owner-freebsd-hackers@FreeBSD.ORG Sat Feb 9 13:47:09 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 33C69350; Sat, 9 Feb 2013 13:47:09 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) by mx1.freebsd.org (Postfix) with ESMTP id EEEC5ABD; Sat, 9 Feb 2013 13:47:08 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id C00B5358C60; Sat, 9 Feb 2013 14:47:06 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id A86C02848C; Sat, 9 Feb 2013 14:47:06 +0100 (CET) Date: Sat, 9 Feb 2013 14:47:06 +0100 From: Jilles Tjoelker To: Konstantin Belousov Subject: Re: Request for review, time_pps_fetch() enhancement Message-ID: <20130209134706.GB19909@stack.nl> References: <1360125698.93359.566.camel@revolution.hippie.lan> <20130206155830.GX2522@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130206155830.GX2522@kib.kiev.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: "freebsd-hackers@freebsd.org" , Ian Lepore 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: Sat, 09 Feb 2013 13:47:09 -0000 On Wed, Feb 06, 2013 at 05:58:30PM +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 return > > the most recently captured data without blocking. These changes add the > > ability to block (forever or with timeout) until a new event occurs. > > Index: sys/kern/kern_tc.c > > =================================================================== > > --- sys/kern/kern_tc.c (revision 246337) > > +++ sys/kern/kern_tc.c (working copy) > > @@ -1446,6 +1446,50 @@ > > * RFC 2783 PPS-API implementation. > > */ > > > > +static int > > +pps_fetch(struct pps_fetch_args *fapi, struct pps_state *pps) > > +{ > > [snip] > > + aseq = pps->ppsinfo.assert_sequence; > > + cseq = pps->ppsinfo.clear_sequence; > > + while (aseq == pps->ppsinfo.assert_sequence && > > + cseq == 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. The memory pointed to by pps is global (other code may have a pointer to it); therefore, the compiler must assume that the tsleep() call (which invokes code in a different compilation unit) may modify it. Because volatile does not make concurrent access by multiple threads defined either, adding it here only seems to slow down the code (potentially). > > + err = tsleep(pps, PCATCH, "ppsfch", timo); > > + if (err == EWOULDBLOCK && fapi->timeout.tv_sec == -1) { > > + continue; > > + } else if (err != 0) { > > + return (err); > > + } > > + } > > + } -- Jilles Tjoelker