Date: Wed, 02 Jan 2002 17:30:03 -0800 (PST) From: John Baldwin <jhb@FreeBSD.org> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: Peter Jeremy <peter.jeremy@alcatel.com.au>, Michal Mertl <mime@traveller.cz>, Bruce Evans <bde@zeta.org.au>, Mike Smith <msmith@FreeBSD.ORG>, Bernd Walter <ticso@cicely8.cicely.de>, arch@FreeBSD.ORG Subject: Re: When to use atomic_ functions? (was: 64 bit counters) Message-ID: <XFMail.020102173003.jhb@FreeBSD.org> In-Reply-To: <200201030112.g031CnN61108@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 03-Jan-02 Matthew Dillon wrote:
>:Actually, we could allow most critical sections to allow interrupts. The
>:only
>:critical section that really needs to disable interrupts are those that need
>:are in spinlocks shared with bottom half code (namely, the locks in the sio
>
> I think a general implementation is better. If you introduce all
> sorts of weird exceptions you only make the code unnecessarily complex
> and developers programming to the API are far more likely to make
> serious mistakes and introduce serious bugs into the system.
Err, no. Only spin locks need actually disable interrupts.
> The critical section code *only* needs to bump/drop a per-thread
> nesting count, which a simple deferred _cpl check when the critical
> section is dropped. If an interrupt occurs on the cpu while the current
> thread is in a critical section, it should be deferred just like we
> did with _cpl in our spl*() code.
Have you even looked at the current implementation? :)
> critical_enter() (inline)
> {
> ++curthread->critical_count;
> }
>
> critical_exit() (inline or inline/procedure hybrid)
> {
> if (--curthread->critical_count == 0) {
> cpl_t cpl;
>
> /*
> * curthread->deferred_cpl is only modified by an interrupt
> * if we are in a critical section, so we can safely modify
> * it when we are not in a critical section.
> *
> * Alternatively one could do something more complex to avoid
> * preemptive races, like do a real sti/cli here.
> *
> * As with our SPL code, this case rarely occurs so we can
> * afford to be a bit more heavy-weight when it does occur.
> */
> if ((cpl = curthread->deferred_cpl) != 0) {
> /*
> * (This part could be in a hybrid procedure, while the
> * above section could be in an inline)
> */
> real_sti();
> curthread->deferred_cpl = 0;
> dispatch_deferred_cpl(cpl);
> real_cli();
> }
> }
> }
This requires the bottom-half code to not use any locks at all which might be
possible. However it means if I have an idle CPU, it can't pick up the
interrupt handler for the interrupt that came in until the CPU that gets its
interrupt leaves the handler. If you want to prevent that, you need to allow
access to the scheduler when the interrupt comes in, which means that when top
half code holds the sched_lock it needs to disable interrupts.
>:critical enter/exit calls that do disable interrutps if needed and enable
>:them
>:again when needed. Right now the current critical_enter/exit code assumes
>:that
>
> I think it is overkill too. Keep it simple and it can be used without
> having to have a manual reference by your desk to figure out what it
> actually does.
Err, it's not near that complicated. This only changes the calls to
critical_enter/exit in one location. I was saying that the separate nesting
count was a bit too complicated.
>:This won't change the critical API for consumers other than spin locks and
>:just
>:needs a different critical_enter() for spin locks. Hmm, OTOH, I could fix
>:that
>:last case by using a separate nesting count (rather than first nesting level)
>:for hte interrupt disabled critical_enter/exits. That's a bit too
>:complicated
>:though I think. Anyways, this can fit into the current API under the covers
>:as
>:an optimization later on.
>:
>:--
>:
>:John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
>
> Yes, definitely too complicated. But I disagree with the 'as an
> optimization later on'. The perceived cost of these functions colors
> everyone's implementation of code around them, and some pretty nasty
> code (due to people trying to avoid perceived-to-be expensive calls)
> can be the result. Something like this needs to be implemented right
> off the bat so people can depend on it.
No. People need to write algorithms and not assume that implementation
specifics about certain functions. There is a manpage (though it needs a bit
of updating) for the critcal section stuff and it still accurately documents
what the API is guaranteed to provide: protection from preemption.
> I'm willing to do this, if you don't have your fingers in this particular
> section of code.
Actually, I have some changes in this code already including halfway
implementing this.
> -Matt
> Matthew Dillon
> <dillon@backplane.com>
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"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.020102173003.jhb>