Date: Mon, 26 Sep 2011 15:59:16 +0200 From: Attilio Rao <attilio@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r225372 - head/sys/kern Message-ID: <CAJ-FndD%2BfWPW_XFAChG5UbOR3iAvU5i7Lo35=8J_1gdRWmGcLg@mail.gmail.com> In-Reply-To: <20110905023251.C832@besplex.bde.org> References: <201109041307.p84D72GY092462@svn.freebsd.org> <CAJ-FndDa=xmvrcn9CdgMvDZ_vG3pjUdFqLH=Q%2BVK%2BSWjvGFO9g@mail.gmail.com> <20110905023251.C832@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
2011/9/4 Bruce Evans <brde@optusnet.com.au>:
> On Sun, 4 Sep 2011, Attilio Rao wrote:
>
>> Also please notice that intr enable/disable happens in the wrong way
>> as it is done via the MD (x86 specific likely) interface. This is
>> wrong for 2 reasons:
>
> No, intr_disable() is MI. It is also used by witness. disable_intr()
> is the corresponding x86 interface that you may be thinking of. The MI
> interface intr_disable() was introduced to avoid the MD'ness of
> intr_disable().
I was a bit surprised to verify that you are right but
spinlock_enter() has the big difference besides disable_intr() of also
explicitly disabling preemption via critical_enter() which some
codepath can trigger without even noticing it.
This means it is more safer in presence of PREEMPTION option on and
thus should be preferred to the normal intr_disable(), in particular
for convoluted codepaths.
>> 1) There may be some codepaths leading to explicit preemption
>> 2) It should really use an MI interface
>>
>> The right way to do this should be via spinlock_enter().
>
> spinlock_enter() is MI, but has wrong semantics. In my version of i386,
> spinlocks don't disable any h/w interrupt, as is needed for fast interrupt
> handlers to actually work. I believe sparc64 is similar, except its
> spinlock_enter() disables most h/w interrupts and this includes fast
> interrupt handlers. I don't understand sparc64, but it looks like its
> spinlock_enter() disables all interrupts visible in C code, but not
> all interrupts:
Can you please explain more about the 'h/w interrupts not disabled' in X86?
Are you speaking about NMIs? For those the only way to effectively
mask them would be to reprogram the LAPIC entry, but I don't really
think we may want that.
> from cpufunc.h:
> % static __inline register_t
> % intr_disable(void)
> % {
> % register_t s;
> % % s = rdpr(pstate);
> % wrpr(pstate, s & ~PSTATE_IE, 0);
> % return (s);
> % }
>
> This seems to mask all interrupts, as required.
>
> (The interface here is slightly broken (non-MI). It returns register_t.
> This assumes that the interrupt state can be represented in a single
> register. The type critical_t exists to avoid the same bug in an
> old version of critical_enter(). Now this type is just bogus.
> critical_enter() no longer returns it. Instead, spinlock_enter() uses
> a non-reentrant interface which stores what used to be the return value
> of critical_enter() in a per-thread MD data structure (md_saved_pil
> in the above). Most or all arches use register_t for this. This
> leaves critical_t as pure garbage -- the only remaining references to
> it are for its definition.)
I mostly agree, I think we should have an MD specified type to replace
register_t for this (it could alias it, if it is suitable, but this
interface smells a lot like x86-centric).
Thanks,
Attilio
--
Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndD%2BfWPW_XFAChG5UbOR3iAvU5i7Lo35=8J_1gdRWmGcLg>