On Tue, Jan 26, 2010 at 08:10:25AM +0100, Attilio Rao wrote: > 2010/1/26 Rob Farmer <rfar...@predatorlabs.net>: > > On Sat, Jan 23, 2010 at 7:54 AM, Attilio Rao <atti...@freebsd.org> wrote: > >> Author: attilio > >> Date: Sat Jan 23 15:54:21 2010 > >> New Revision: 202889 > >> URL: http://svn.freebsd.org/changeset/base/202889 > >> > >> Log: > >> ??- Fix a race in sched_switch() of sched_4bsd. > >> ?? ??In the case of the thread being on a sleepqueue or a turnstile, the > >> ?? ??sched_lock was acquired (without the aid of the td_lock interface) and > >> ?? ??the td_lock was dropped. This was going to break locking rules on > >> other > >> ?? ??threads willing to access to the thread (via the td_lock interface) > >> and > >> ?? ??modify his flags (allowed as long as the container lock was different > >> ?? ??by the one used in sched_switch). > >> ?? ??In order to prevent this situation, while sched_lock is acquired there > >> ?? ??the td_lock gets blocked. [0] > >> ??- Merge the ULE's internal function thread_block_switch() into the global > >> ?? ??thread_lock_block() and make the former semantic as the default for > >> ?? ??thread_lock_block(). This means that thread_lock_block() will not > >> ?? ??disable interrupts when called (and consequently thread_unlock_block() > >> ?? ??will not re-enabled them when called). This should be done manually > >> ?? ??when necessary. > >> ?? ??Note, however, that ULE's thread_unblock_switch() is not reaped > >> ?? ??because it does reflect a difference in semantic due in ULE (the > >> ?? ??td_lock may not be necessarilly still blocked_lock when calling this). > >> ?? ??While asymmetric, it does describe a remarkable difference in semantic > >> ?? ??that is good to keep in mind. > >> > >> ??[0] Reported by: ?? ?? ??Kohji Okuno > >> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??<okuno dot kohji at jp dot panasonic > >> dot com> > >> ??Tested by: ?? ?? ?? ?? ?? ??Giovanni Trematerra > >> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??<giovanni dot trematerra at gmail dot > >> com> > >> ??MFC: ?? ?? ?? ?? ?? ?? ?? ?? ??2 weeks > >> > >> Modified: > >> ??head/sys/kern/kern_mutex.c > >> ??head/sys/kern/sched_4bsd.c > >> ??head/sys/kern/sched_ule.c > > > > Hi, > > > > This commit seems to be causing me a kernel panic on sparc64 - details > > are in PR 143215. Could you take a look before MFCing this? > > I think that the bug may be in cpu_switch() where the mutex parameter > for sched_4bsd is not handled correctly. > Does sparc64 support ULE? I don't think it does and I think that it > simply ignores the third argument of cpu_switch() which is vital now > for for sched_4bsd too (what needs to happen is to take the passed > mutex and to set the TD_LOCK of old thread to be the third argument). > Unluckilly, I can't do that in sparc64 asm right now, but it should > not be too difficult to cope with it. >
The following patch adds handling of the mutex parameter to the sparc64 cpu_switch(): http://people.freebsd.org/~marius/sparc64_cpu_switch_mtx.diff This patch works fine with r202888. With r202889 it allows the machine to boot again, however putting some load on the machine causes it to issue a reset without a chance to debug. I've also tried with some variations like duplicating the old cpu_switch() for cpu_throw() so the altered cpu_switch() doesn't need to distinguish between the to cases and only assigning old->td_lock right before return but nothing made a difference. Given that this leaves little room for a bug in the cpu_switch() changes I suspect r202889 also breaks additional assumptions. For example the sparc64 pmap code used sched_lock, does that need to change to be td_lock now maybe? Is there anything else that comes to your mind in this regard? Marius _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
