Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 2 Jan 2002 17:12:49 -0800 (PST)
From:      Matthew Dillon <dillon@apollo.backplane.com>
To:        John Baldwin <jhb@FreeBSD.ORG>
Cc:        arch@FreeBSD.ORG, Bernd Walter <ticso@cicely8.cicely.de>, Mike Smith <msmith@FreeBSD.ORG>, Bruce Evans <bde@zeta.org.au>, Michal Mertl <mime@traveller.cz>, Peter Jeremy <peter.jeremy@alcatel.com.au>
Subject:   Re: When to use atomic_ functions? (was: 64 bit counters)
Message-ID:  <200201030112.g031CnN61108@apollo.backplane.com>
References:   <XFMail.020102164253.jhb@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
:>     expensive.  Couldn't we just have a per-cpu critical section count
:>     and defer the interrupt?  (e.g. like the deferred mechanism we used for
:>     spl()s).  Then we would have an incredibly cheap mechanism for accessing
:>     per-cpu caches (like per-cpu mbuf freelists, for example) which could
:>     further be adapted for use by zalloc[i]() and malloc().
:
:Err, it does maintain a count right now and only does the actual change of
:interrupt state when entering and exiting critical sections at the top-level.
:sti and cli aren't very expensive though.  Critical sections are more expensive
:on other archs though.  For example, it's a PAL call on alpha.  Although, spl's
:were PAL calls on alpha as well.  On ia64 it's a single instruction but one that
:requires a stop.

    Most of the critical section code in the system is going to be at
    the top level.  sti and cli *ARE* expensive, that's why the original
    spl code went to such great lengths to avoid calling them.  I 
    believe one or both instructions synchronizes the cpu pipeline,
    interrupting instruction flow.  It is nasty stuff.

: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.

:In theory, most other critical sections need only tweak their per-thread
:nesting count (which doesn't need a lock).  This has been sitting in the back
:of my mind for a while but I want to figure out how to do this cleanly. 

    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.  

    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();
	    }
	}
    }

:One idea is to allow spinlocks to use slightly different versions of the
: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.

: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.

    I'm willing to do this, if you don't have your fingers in this particular
    section of code.

					-Matt
					Matthew Dillon 
					<dillon@backplane.com>

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?200201030112.g031CnN61108>