Re: Stop scheduler on panic

2011/12/7 Andriy Gapon <avg_at_freebsd.org>:
> on 07/12/2011 00:11 Attilio Rao said the following:
>> I'd just change this check on panicstr:
>> _at__at_ -606,9 +603,13 _at__at_ kdb_trap(int type, int code, struct trapframe *tf)
>>       intr = intr_disable();
>>
>>  #ifdef SMP
>> -     other_cpus = all_cpus;
>> -     CPU_CLR(PCPU_GET(cpuid), &other_cpus);
>> -     stop_cpus_hard(other_cpus);
>> +     if (panicstr == NULL) {
>> +             other_cpus = all_cpus;
>> +             CPU_CLR(PCPU_GET(cpuid), &other_cpus);
>> +             stop_cpus_hard(other_cpus);
>> +             did_stop_cpus = 1;
>> +     } else
>> +             did_stop_cpus = 0;
>>
>> to be SCHEDULER_STOPPED().
>
> Makes sense.  I will do this.
>
>> If you agree I can fix the kern_mutex, kern_sx and kern_rwlock parts
>> and it should be done.
>
> Since I am not very familiar with the details of that code, I can not be against
> such a proposal :-)  What Kostik did seemed quite reasonable to me, but if that
> can be further improved, then I am all for it.

The following patch is a further add-on on Kostik's:
http://www.freebsd.org/~attilio/scheduler_stopped.patch

- Rework of mutex, rwlock and sxlock for a correct dealing of hard and
fast paths
- Protection of LOCK_PROFILING bits (missed also in my review)
- Protection of WITNESS_SAVE/RESTORE because of Giant handling (missed
also in my review)
- Removal of gratuitous whitelines
- Usage of SCHEDULER_STOPPED() in kdb check

What do you think about it?
I just test-compiled it with several combinations of LOCK_PROFILING
and LOCK_DEBUG, but I didn't change the bulk of it thus it should be
perfectly fine.

If you like it I'd say to go for the commit asap.
I wonder if someone tried to simulate a livelock and panic and thus
verify that stoppcbs is correctly populated as expected (to be honest,
this is one of the best features I'm interested into for this patch).

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein