Date: Wed, 7 Dec 2011 18:04:23 +0100 From: Attilio Rao <attilio@freebsd.org> To: Andriy Gapon <avg@freebsd.org> Cc: Kostik Belousov <kostikbel@gmail.com>, arch@freebsd.org, current@freebsd.org Subject: Re: Stop scheduler on panic Message-ID: <CAJ-FndC=oy3W5sxGhwws9FaAxALJxvwZnY_mwXr4tqiCeq27yw@mail.gmail.com> In-Reply-To: <4EDF18AA.2070509@FreeBSD.org> References: <20111113083215.GV50300@deviant.kiev.zoral.com.ua> <CAJ-FndCz0wrFojK4_FOrcoQWk6Wd%2Btq%2Br%2BT9PXT%2BLn%2B7hvKB8A@mail.gmail.com> <4EDE8931.1080506@FreeBSD.org> <CAJ-FndA0f5v%2B7Y_HreqUXwxRwWyEqie1ip2c6==VKME4Bp1u5A@mail.gmail.com> <4EDF18AA.2070509@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
2011/12/7 Andriy Gapon <avg@freebsd.org>:
> on 07/12/2011 00:11 Attilio Rao said the following:
>> I'd just change this check on panicstr:
>> @@ -606,9 +603,13 @@ kdb_trap(int type, int code, struct trapframe *tf)
>> =C2=A0 =C2=A0 =C2=A0 intr =3D intr_disable();
>>
>> =C2=A0#ifdef SMP
>> - =C2=A0 =C2=A0 other_cpus =3D all_cpus;
>> - =C2=A0 =C2=A0 CPU_CLR(PCPU_GET(cpuid), &other_cpus);
>> - =C2=A0 =C2=A0 stop_cpus_hard(other_cpus);
>> + =C2=A0 =C2=A0 if (panicstr =3D=3D NULL) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 other_cpus =3D all_cpus;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 CPU_CLR(PCPU_GET(cpuid), &ot=
her_cpus);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 stop_cpus_hard(other_cpus);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 did_stop_cpus =3D 1;
>> + =C2=A0 =C2=A0 } else
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 did_stop_cpus =3D 0;
>>
>> to be SCHEDULER_STOPPED().
>
> Makes sense. =C2=A0I 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 :-) =C2=A0What 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
--=20
Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndC=oy3W5sxGhwws9FaAxALJxvwZnY_mwXr4tqiCeq27yw>