svn commit: r202889 - head/sys/kern
marius at alchemy.franken.de
Wed Jan 27 21:59:06 UTC 2010
On Tue, Jan 26, 2010 at 08:10:25AM +0100, Attilio Rao wrote:
> 2010/1/26 Rob Farmer <rfarmer at predatorlabs.net>:
> > On Sat, Jan 23, 2010 at 7:54 AM, Attilio Rao <attilio at 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. 
> >> Â - 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.
> >> Â  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
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?
More information about the svn-src-all