Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 23 Mar 2001 18:26:07 -0800 (PST)
From:      John Baldwin <jhb@FreeBSD.org>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        arch@FreeBSD.org
Subject:   Re: Critical Regions Round II
Message-ID:  <XFMail.010323182607.jhb@FreeBSD.org>
In-Reply-To: <Pine.BSF.4.21.0103231706230.17460-100000@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help

On 23-Mar-01 Bruce Evans wrote:
> On Thu, 22 Mar 2001, John Baldwin wrote:
> 
>> Hopefully this will make everyone happy.
> 
> You are an optimist :-).

Apparently, though I'd like to get this done soon, as it is holding up other
work (I have too many interdependent patch sets on my laptop).

>> Prior to SMPng we had the following functions related to interrupts:
>> 
>> disable_intr()  - x86 only and just did a 'cli'
>> enable_intr()   - x86 only and just did a 'sti'
> 
> SMPog actually made these do things like 'cli(); s_lock(&clock_lock)'
> in many cases (most of the non-broken cases).

Yes, for sio/cy, etc.

>> With the initial SMPng commit this was added to to allow nested disabling
>> of interrupts:
> 
> I originally tried hard to avoid nested disabling of interrupts.  The
> x86 functions to support it were intentionally left out of early
> versions of the x86 cpufunc.h.  The principle is that nested disabling
> of interrupts is as evil as recursive locking.  However, it is very
> convenient.  It was necessary to support profiling of code that is
> running with interrupts disabled (even though that code disabled
> interrupts to prevent potentially lengthy interference by things like
> profilers), so support for it was added in rev.1.21 of the x86 cpufunc.h.
> It is necessary to support debugging of code that os running with
> interrupts disabled (even though interference by debuggers screws up
> the timing much worse than interference by profilers).

It's also needed for recursive locking of spinlocks, or for another spinlock to
be acquired while holding some other spinlock.  (We can't allow preemption
while holding a spinlock or we can get into trivial deadlocks.)

>> save_intr()     - return current interrupt state
>> restore_intr()  - revert to previously read interrupt state
> 
> This was really a renaming of the x86-specific interfaces read_eflags()
> and write_eflags().  I object to the x86 code being changed to use the
> new names.  The x86 code that disables interrupts mostly wants precisely
> the MD interfaces, not the MI abstraction of them.  It just happens that
> the MI versions are binary-identical with the MD versions.

Ok, I could go with that.  (/me deletes half of his patch.)

>> Also, this entire API was forced out into MI code.  After the discussion on
>> the mailing list I propose the following that will hopefully make everyone
>> happy:
>> 
>> disable_intr()  - x86 / ia64 only, just clears the IF or psr.i bit, and is
>>                   a MD function
>> enable_intr()   - x86 / ia64 only, just sets the IF or psr.i bit, and is a
>>                   MD function
> 
> I want these to be named cli() and sti() for x86 only (maybe clpsri() and
> stpsri() for ia64 :-).  MD functions should have MD names.

Ok.

>> save_intr()     - gone
>> restore_intr()  - gone
> 
> The MD versions of these may still be needed to implement critical_*().

Yes, in fact they are.  critical_enter() on x86 uses read_eflags/disable_intr,
and critical_exit() uses write_eflags().  On the alpha both functions just call
alpha_pal_swpipl(), and on ia64 it is somewhat similar to x86.

>> critical_enter() - enters a critical region of code that must not be
>>                    preempted
>> critical_exit()  - leaves a critical region of code
>> 
>> The critical_enter/exit functions use an opaque type 'critical_t' for the
>> value that critical_enter() returns that must be passed to critical_exit().
> 
> OK, but see other replies.

Ok, I'll try and separate this from my other patches and put it up for people
to gawk at^H^H^H^H^H^H^Hreview.

> Bruce

-- 

John Baldwin <jhb@FreeBSD.org> -- http://www.FreeBSD.org/~jhb/
PGP Key: http://www.baldwin.cx/~john/pgpkey.asc
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.010323182607.jhb>