From owner-svn-src-head@FreeBSD.ORG Thu Feb 21 14:52:41 2013 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 4940E560 for ; Thu, 21 Feb 2013 14:52:41 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from mail-ia0-x22a.google.com (mail-ia0-x22a.google.com [IPv6:2607:f8b0:4001:c02::22a]) by mx1.freebsd.org (Postfix) with ESMTP id 19985A1D for ; Thu, 21 Feb 2013 14:52:41 +0000 (UTC) Received: by mail-ia0-f170.google.com with SMTP id k20so8391693iak.29 for ; Thu, 21 Feb 2013 06:52:40 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:sender:subject:mime-version:content-type:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to:x-mailer:x-gm-message-state; bh=g8Pvyj8imXOdnI6TJ4w7cqJQotWVQGxPg5h9kv/opUU=; b=iL9evf7twfXs2e2VHQaYFAERotKl7H3iOnge6aVbL2lzqdvXRqPPkzUghDhUgyg5iO Wc7dP2nXXk1gqTGn3bnHt0uJHXIByk0V3NjZPptYitu8XrenwUSTe+z6T9bgVh8eWJ3C sC1BvU+mXkf+udRy2XRMKKAtaJ4Dfn2CcMgDeB13LkV+e0bzLekmNX+u9vanXZzSJFmq 34pyqnP/jIVAHXQvRsNv29LYmCA5zUXnQA5kSSd2M6nqOBDCeQJ8kr1TveJma5SEGw70 nE8smOfRqULmVlPwERxrMm1FzdfP5smUeQOWOU7t+rNp6JuWS9JSABSyMu2kjhBlEfIM Ap7g== X-Received: by 10.50.76.168 with SMTP id l8mr12997368igw.97.1361458360660; Thu, 21 Feb 2013 06:52:40 -0800 (PST) Received: from 53.imp.bsdimp.com (50-78-194-198-static.hfc.comcastbusiness.net. [50.78.194.198]) by mx.google.com with ESMTPS id vb15sm18777441igb.9.2013.02.21.06.52.38 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 21 Feb 2013 06:52:39 -0800 (PST) Sender: Warner Losh Subject: Re: svn commit: r247068 - head/sys/x86/isa Mime-Version: 1.0 (Apple Message framework v1085) Content-Type: text/plain; charset=us-ascii From: Warner Losh In-Reply-To: <20130221225538.J1447@besplex.bde.org> Date: Thu, 21 Feb 2013 07:52:37 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: <9BB56E45-A272-46D7-AF3F-833424017F2D@bsdimp.com> References: <201302210040.r1L0e80m095140@svn.freebsd.org> <20130221225538.J1447@besplex.bde.org> To: Bruce Evans X-Mailer: Apple Mail (2.1085) X-Gm-Message-State: ALoCoQmP5GSwj3muFWqEDB2bkvoB5WAaKsT1c6qVPY42iLplZy2jqNpFfPZTHVTzvIr44a/nUc+4 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Warner Losh X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Feb 2013 14:52:41 -0000 On Feb 21, 2013, at 5:31 AM, Bruce Evans wrote: > On Thu, 21 Feb 2013, Warner Losh wrote: >=20 >> Log: >> Fix broken usage of splhigh() by removing it. >=20 > This is more broken than before. The splhigh() served to indicate > missing locking. Depends on what you mean by more :) It is less depessimized after the = nopification of splhigh(). >> Modified: head/sys/x86/isa/atrtc.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=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> --- head/sys/x86/isa/atrtc.c Thu Feb 21 00:36:12 2013 = (r247067) >> +++ head/sys/x86/isa/atrtc.c Thu Feb 21 00:40:08 2013 = (r247068) >> @@ -328,7 +328,6 @@ static int >> atrtc_gettime(device_t dev, struct timespec *ts) >> { >> struct clocktime ct; >> - int s; >>=20 >> /* Look if we have a RTC present and the time is valid */ >> if (!(rtcin(RTC_STATUSD) & RTCSD_PWR)) { >> @@ -338,11 +337,8 @@ atrtc_gettime(device_t dev, struct times >>=20 >> /* wait for time update to complete */ >> /* If RTCSA_TUP is zero, we have at least 244us before next = update */ >=20 > As the comment says, this is time-critical code. It needs to do = something > to prevent it being preempted for more than 244 usec That's a long time... >> - s =3D splhigh(); >=20 > It used to do something... >=20 >> - while (rtcin(RTC_STATUSA) & RTCSA_TUP) { >> - splx(s); >> - s =3D splhigh(); >> - } >> + while (rtcin(RTC_STATUSA) & RTCSA_TUP) >> + continue; >=20 > You should probably have changed this to a critical section like you = did > in ppc. Disabling hardware interrupts would be even better. I'll replace with a critical section. > There is a problem with the "show rtc" command in ddb. It was born > broken (racy), and the races were turned into deadlocks by adding > locking in rtcin(). So if you trace through this code, then > "show rtc" while holding the lock in rtcin() will deadlock. It is a > bug for ddb to call any code that might try to acquire a mutex, but > "show rtc" always calls rtcin() and rtcin() always tries to aquire a > mutex. Similar deadlocks on the i8254 lock in DELAY() are worked > around by not trying to acquire the lock in kdb mode. kbd_active is what I need to check, right? I'll fix that while here. >> ct.nsec =3D 0; >> ct.sec =3D readrtc(RTC_SEC); >> ct.min =3D readrtc(RTC_MIN); >>=20 >=20 > There are 8 or 9 readrtc()'s altogether. These must be atomic, and = all > within the 244 usec limit. There is considerable danger of exceeding = the > limit without even being preempted. Each readrtc() does 1 or 4 isa = bus > accesses. I've seen a single isa bus access taking 139 usec (delayed = by > PCI DMA). >=20 > Another way of doing this without any locking against preemption or > timing magic is to read the time before and after. If it took more = than > 244 usec from before seeing RTCSA_TUP deasserted to after the last > readrtc(), then retry. By computing a time difference, or by checking RTCSA_TUP? > My version still depends on splhigh() working, but it does more > sophisticated waiting that gives a full second to read the registers > without getting more than a stale time. The above reads an RTC value > that may be stale by almost 1 second. My version busy-waits until > just after after the counter rolls over. This is only suitable for > boot=3Dtime initialization. Debugging printfs show that this never = got > preempted over the last few years -- the whole operation always > completed within about 40 usec of seeing the rollover. So this isn't a problem on fast machines? > Linux does even more sophisticated waiting for writing the registers, > so as not to be off by another second in the write operation. Yea, that would be nice, I may have to look into that, once I'm done = fetching these stones. Warner