:interrupts; hard interrupts are rarely masked.  The queueing is written
:in assmebler and runs outside of the kernel so it is fast.  Traps in
:general are fast because they don't touch memory until the trapframe
:is written out, so I don't see much point in changing this do the masking
:in software and avoid the soft interrupt.

    I have no idea what you are talking about Jake.  Could you supply
    some context?

:In regards to the patch I think that making critical_enter/exit MD in
:certain cases is a mistake.  cpu_critical_enter/exit are already MD
:and it looks you could hide this behind them with some minor changes.
:Like in the critical_enter case, make cpu_critical_enter take a thread
:as its argument.  It could be a null macro in the i386 case, which would
:optimize out the test.
:if (td->td_critnest == 0)
:       cpu_critical_enter(td);
:Likewise with cpu_critical_exit, make it take the thread as its argument.
:If (td->td_critnest == 1) {
:       td->td_critnest = 0;
:       cpu_critical_exit(td);
:} else
:       td->td_critnest--;
:(This is equivalent to if (--td->td_critnest == 0), its a matter of taste.)

    I'm afraid I have to strongly disagree.  I believe that 
    critical_enter()/exit is *ALREADY* being severely misused in current.
    For example, fork_exit() makes assumptions about the underlying hardware
    in order to initialize td_critnest and the scheduler mutex safely.  This
    is just plain broken.  fork_exit() should not make any such assumptions.
    It is the clear responsibility of either the trampoline code or
    cpu_fork() to properly set up td_critnest.  One can make an argument
    that the sched_lock fixup is in fork_exit()'s domain but you cannot
    make the argument that the initialization of critnest (and any associated
    hardware state) is MI.  It just isn't.

    Additionally, critical_enter()/exit themselves are certainly much
    more MD then MI.  It makes little sense to abstract out an MI interface
    that forces unnecessary overhead when all you have to do is define an
    MD API that has a few MI requirements, like td_critnest being a critical
    nesting count that the MI code can count on.  But as MI code the existing
    critical_enter()/exit impose additional MD fields and do not give the
    MD code the option of not using them (except in a degenerate fashion).
    Specifically, the use of the td_savecrit field is under the partial
    contorl of the MI code when it shouldn't be, not just in fork_exit()
    but also in critical_enter() and critical_exit() themselves.

    The existing critical_enter/exit code is far too abstracted for its
    low-level purpose and I *WILL* be moving them into MD sections of the
    system where they belong.  These are really MD routines, not MI routines.

:I notice the you are using cpu_critical_exit for the purpose of disabling
:interrupts only, like cli, sti.  I think that you should use api that tmm
:wrote for sparc64 for this so that it can do more.  i386 does not really
:have cli/sti equivalents otherwise.
:s = intr_disable()     disables interrupts and returns the old state.
:intr_restore(s)                restores the state s.
:I don't really like this change in general because it complicates things
:more than I'd like, but I also don't have a bearing on how expensive cli/sti
:are.  It would seem to me that it just takes a few more clock cycles, which
:isn't that important.  I understand that it increases latency for the fast
:part of the interrupt handler, but they are not able to run in critical
:sections anyway due to the software masking.

    Removing cli and sti from the critical functions saves us 300 ns on
    every single system call, interrupt, and most traps.  And that's with the 
    sysctl instrumentation and without them being inlined.  With the sysctl
    instrumentation removed and with critical_enter() and critical_exit()
    inlined my guess is that we will save on the order of 500 ns.  This 
    is not insignificant on a P3, and it will probably save even more on
    a P4.

    With critical_enter() and critical_exit() moved into MD sections, we
    can do away with cpu_critical_enter() and cpu_critical_exit() (that is,
    make them specific to the MD implementation of critical_enter() and
    critical_exit() on those platforms that want to keep the two-layer
    abstraction).  But for I386 I suppose I could just rename them 
    to intr_disable() and intr_restore().  I don't understand your
    comment about functionality, they work just fine.  It's just the name
    that needs changing.

                                        Matthew Dillon 
                                        <[EMAIL PROTECTED]>

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to