Date: Tue, 13 Mar 2018 06:02:56 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Ian Lepore <ian@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r330780 - in head/sys: amd64/include isa x86/isa Message-ID: <20180313045728.A3796@besplex.bde.org> In-Reply-To: <20180312155617.GF76926@kib.kiev.ua> References: <201803111922.w2BJMwr8043084@repo.freebsd.org> <20180311195800.GC76926@kib.kiev.ua> <1520799639.84937.154.camel@freebsd.org> <20180311212539.GD76926@kib.kiev.ua> <1520868891.84937.177.camel@freebsd.org> <20180312155617.GF76926@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 12 Mar 2018, Konstantin Belousov wrote: > On Mon, Mar 12, 2018 at 09:34:51AM -0600, Ian Lepore wrote: >> On Sun, 2018-03-11 at 23:25 +0200, Konstantin Belousov wrote: >>> On Sun, Mar 11, 2018 at 02:20:39PM -0600, Ian Lepore wrote: >>>> >>>> On Sun, 2018-03-11 at 21:58 +0200, Konstantin Belousov wrote: >>>>> >>>>> On Sun, Mar 11, 2018 at 07:22:58PM +0000, Ian Lepore wrote: >>>>>> >>>>>> >>>>>> [...] >>>> >>>> Unfortunately, this reverts one type of wrong locking back to another. >>>> =9AThe need is to prevent access to the atrtc hardware if efi is >>>> accessing it, and the locking I just reverted to that uses a sleepable >>>> mutex only protects against inittodr()/resettodr() access, but not >>>> against nvram(4) or if the atrtc is being used as an eventtimer (of >>>> course nobody uses it for that, but the driver supports it). >>>> >>>> I have some pending changes that cause the atrtc driver to just not >>>> attach at all if the efirtc driver attached, but they're stacked up >>>> behind some other changes in phab. =9AAnd that still doesn't fix the >>>> nvram(4) part of it. >>> Not attaching atrtc if efirtc is attached sounds reasonable. But then >>> you should also disable efirt attach if atrtc is on. One possible issue >>> is that efirt is typicall loadable, while atrtc is compiled into the >>> kernel, which means that efirt would become virtually unusable. >> >> Even if efirt is loaded (in loader(8)) the efirtc driver will still >> attach instead of atrtc, because it's a direct child of nexus and gets >> the first opportunity to probe and attach. =9ABut you raise a good point= , >> I should make it handle the case where it gets kldload'd when atrtc is >> already attached. >> >>> For nvram(4), you can take the atrtc_time_lock around accesses in addit= ion >>> to the atrtc_lock, instead of providing exclusivity on the level of dri= vers >>> attach. >> >> It occurs to me that an even better fix for all of this would be to >> remove support for atrtc being an eventtimer. =9AThat allows removing th= e >> interrupt filter handler, and then there's no need for a spinlock at >> all, a sleepable mutex works fine for all accesses. Sleep mutexes never work fine for real time clocks. They give unbounded time for clock accesses. >> Does anybody really need an eventtimer that runs only at a fixed >> periodic rate of 32khz in 2018? > > Problem is that atrtc is what old machines use. If we have HPET or > better LAPIC timers, then we do not need atrtc at all, of course. > But e.g. 486 do not have them. I am not even sure about early amd64 > machines. The early AMD development board "Solo 2" has LAPIC. It even has HPET for a timecounter but not for an event timer. My 2006 laptop doesn't have HPET. My version of FreeBSD-5 uses the 1 Hz RTC update interrupt for fixing up the timecounter after stopping in ddb. This should also be used for fixing up the timecounter after suspension. This doesn't need full event timer support, but it needs full interrupt support including spin mutexes. > Definitely any machine that needs EFIRT has both HPET and LAPIC, I like > the idea of not providing eventtimer backed by atrtc if feasible. Might > be, make this a compile or runtime option, unless this over-complicates > the code. It's just more complicated to make it optional. The top-level event timer code supports using "any" timer in the system with very little driver support, at least for drivers/hardware that only support periodic interrupt= s. Everyone with a 486 needs an atrtc that runs at 128 Hz like it used to. 32 kHz wastes a lot of CPU even on newer systems. Just reading 1 status register in rtc_intr() takes at least 1 usec. That is about 3% of the CPU at 32 kHz. rtcin() used to read 4 ISA registers (2 intentional delays which are FUD mainly for 20-30 year old systems). That is about 12% for the CPU at 32 kHz. Probably more like 20%. I optimized rtcin() especially for this case (where the index register is almost constant). SCHED_4BSD only needs 16 Hz, and only profiling needs more than 128 Hz. I used hz =3D stathz =3D lapic_timer_hz =3D 100 (all periodic) for some tim= e before event timers existed. This doesn't support fine-grained timeouts, but I don't want them. It gives much too synchronization between hardclock and statclock interrupts. but not much more than with the the old default lapic_timer_hz of 2000. Bruce From owner-svn-src-all@freebsd.org Mon Mar 12 19:03:32 2018 Return-Path: <owner-svn-src-all@freebsd.org> Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id EAD7BF36CEC for <svn-src-all@mailman.ysv.freebsd.org>; Mon, 12 Mar 2018 19:03:31 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from pmta2.delivery6.ore.mailhop.org (pmta2.delivery6.ore.mailhop.org [54.200.129.228]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 704B46C003 for <svn-src-all@freebsd.org>; Mon, 12 Mar 2018 19:03:31 +0000 (UTC) (envelope-from ian@freebsd.org) X-MHO-User: ed7ad2f1-2627-11e8-b951-f99fef315fd9 X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 67.177.211.60 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [67.177.211.60]) by outbound2.ore.mailhop.org (Halon) with ESMTPSA id ed7ad2f1-2627-11e8-b951-f99fef315fd9; Mon, 12 Mar 2018 19:02:37 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id w2CJ3SfN016997; Mon, 12 Mar 2018 13:03:28 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <1520881408.84937.232.camel@freebsd.org> Subject: Re: svn commit: r330780 - in head/sys: amd64/include isa x86/isa From: Ian Lepore <ian@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Date: Mon, 12 Mar 2018 13:03:28 -0600 In-Reply-To: <20180312155617.GF76926@kib.kiev.ua> References: <201803111922.w2BJMwr8043084@repo.freebsd.org> <20180311195800.GC76926@kib.kiev.ua> <1520799639.84937.154.camel@freebsd.org> <20180311212539.GD76926@kib.kiev.ua> <1520868891.84937.177.camel@freebsd.org> <20180312155617.GF76926@kib.kiev.ua> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.18.5.1 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" <svn-src-all.freebsd.org> List-Unsubscribe: <https://lists.freebsd.org/mailman/options/svn-src-all>, <mailto:svn-src-all-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/svn-src-all/>; List-Post: <mailto:svn-src-all@freebsd.org> List-Help: <mailto:svn-src-all-request@freebsd.org?subject=help> List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/svn-src-all>, <mailto:svn-src-all-request@freebsd.org?subject=subscribe> X-List-Received-Date: Mon, 12 Mar 2018 19:03:32 -0000 On Mon, 2018-03-12 at 17:56 +0200, Konstantin Belousov wrote: > On Mon, Mar 12, 2018 at 09:34:51AM -0600, Ian Lepore wrote: > > > > On Sun, 2018-03-11 at 23:25 +0200, Konstantin Belousov wrote: > > > > > > On Sun, Mar 11, 2018 at 02:20:39PM -0600, Ian Lepore wrote: > > > > > > > > > > > > On Sun, 2018-03-11 at 21:58 +0200, Konstantin Belousov wrote: > > > > > > > > > > > > > > > On Sun, Mar 11, 2018 at 07:22:58PM +0000, Ian Lepore wrote: > > > > > > > > > > > > > > > > > > > > > > > > [...] > > > > Unfortunately, this reverts one type of wrong locking back to another. > > > > The need is to prevent access to the atrtc hardware if efi is > > > > accessing it, and the locking I just reverted to that uses a sleepable > > > > mutex only protects against inittodr()/resettodr() access, but not > > > > against nvram(4) or if the atrtc is being used as an eventtimer (of > > > > course nobody uses it for that, but the driver supports it). > > > > > > > > I have some pending changes that cause the atrtc driver to just not > > > > attach at all if the efirtc driver attached, but they're stacked up > > > > behind some other changes in phab. And that still doesn't fix the > > > > nvram(4) part of it. > > > Not attaching atrtc if efirtc is attached sounds reasonable. But then > > > you should also disable efirt attach if atrtc is on. One possible issue > > > is that efirt is typicall loadable, while atrtc is compiled into the > > > kernel, which means that efirt would become virtually unusable. > > > > > Even if efirt is loaded (in loader(8)) the efirtc driver will still > > attach instead of atrtc, because it's a direct child of nexus and gets > > the first opportunity to probe and attach. But you raise a good point, > > I should make it handle the case where it gets kldload'd when atrtc is > > already attached. > > > > > > > > For nvram(4), you can take the atrtc_time_lock around accesses in addition > > > to the atrtc_lock, instead of providing exclusivity on the level of drivers > > > attach. > > It occurs to me that an even better fix for all of this would be to > > remove support for atrtc being an eventtimer. That allows removing the > > interrupt filter handler, and then there's no need for a spinlock at > > all, a sleepable mutex works fine for all accesses. > > > > Does anybody really need an eventtimer that runs only at a fixed > > periodic rate of 32khz in 2018? > Problem is that atrtc is what old machines use. If we have HPET or > better LAPIC timers, then we do not need atrtc at all, of course. > But e.g. 486 do not have them. I am not even sure about early amd64 > machines. I think not unless someone has manually configured it that way. The i8254 timer should be used as both timecounter and eventtimer if there is no lapic or hpet. i8254 has an ET priority of 100 compared to atrtc's 0. That's by code inspection... I might still have some industrial SBCs around here old enough to not have hpet or lapic, I'll see if I can get one running. -- Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180313045728.A3796>