2011/9/4 Bruce Evans <b...@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
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
