critical_exit()

[Julian Elischer]

Why would the following:
void
critical_exit(void)
{
        struct thread *td;

        td = curthread;
        KASSERT(td->td_critnest != 0,
            ("critical_exit: td_critnest == 0"));

        if (td->td_critnest == 1) {
                td->td_critnest = 0;
                if (td->td_owepreempt) {
                        td->td_critnest = 1;
                        thread_lock(td);
                        td->td_critnest--;
                        SCHED_STAT_INC(switch_owepreempt);
                        mi_switch(SW_INVOL|SW_PREEMPT, NULL);
                        thread_unlock(td);
                }
        } else
                td->td_critnest--;

        CTR4(KTR_CRITICAL, "critical_exit by thread %p (%ld, %s) to %d", td,
            (long)td->td_proc->p_pid, td->td_name, td->td_critnest);
}


not be expressed:

void
critical_exit(void)
{
        struct thread *td;

        td = curthread;
        KASSERT(td->td_critnest != 0,
            ("critical_exit: td_critnest == 0"));

        if (td->td_critnest == 1) {
                if (td->td_owepreempt) {
                        thread_lock(td);
                        td->td_critnest = 0;
                        SCHED_STAT_INC(switch_owepreempt);
                        mi_switch(SW_INVOL|SW_PREEMPT, NULL);
                        thread_unlock(td);
                } else {
                        td_critnest =  0;
                }
        } else
                td->td_critnest--;

        CTR4(KTR_CRITICAL, "critical_exit by thread %p (%ld, %s) to %d", td,
            (long)td->td_proc->p_pid, td->td_name, td->td_critnest);
}

It seems to me there is a race in the current version, where the
critical count is temporarily 0, where the thread could be pre-empted
when it shouldn't be..

(prompted by a comment by jeffr that made me go look at this code)..