Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Jan 2010 08:10:25 +0100
From:      Attilio Rao <attilio@freebsd.org>
To:        Rob Farmer <rfarmer@predatorlabs.net>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Marius Strobl <marius@freebsd.org>
Subject:   Re: svn commit: r202889 - head/sys/kern
Message-ID:  <3bbf2fe11001252310r408a6be4j9bc42618394b3e3d@mail.gmail.com>
In-Reply-To: <b025ceb71001252225r56d4b0c8qe4c6affe338e6f9f@mail.gmail.com>
References:  <201001231554.o0NFsMbx049837@svn.freebsd.org> <b025ceb71001252225r56d4b0c8qe4c6affe338e6f9f@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
2010/1/26 Rob Farmer <rfarmer@predatorlabs.net>:
> On Sat, Jan 23, 2010 at 7:54 AM, Attilio Rao <attilio@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:
>> =C2=A0- Fix a race in sched_switch() of sched_4bsd.
>> =C2=A0 =C2=A0In the case of the thread being on a sleepqueue or a turnst=
ile, the
>> =C2=A0 =C2=A0sched_lock was acquired (without the aid of the td_lock int=
erface) and
>> =C2=A0 =C2=A0the td_lock was dropped. This was going to break locking ru=
les on other
>> =C2=A0 =C2=A0threads willing to access to the thread (via the td_lock in=
terface) and
>> =C2=A0 =C2=A0modify his flags (allowed as long as the container lock was=
 different
>> =C2=A0 =C2=A0by the one used in sched_switch).
>> =C2=A0 =C2=A0In order to prevent this situation, while sched_lock is acq=
uired there
>> =C2=A0 =C2=A0the td_lock gets blocked. [0]
>> =C2=A0- Merge the ULE's internal function thread_block_switch() into the=
 global
>> =C2=A0 =C2=A0thread_lock_block() and make the former semantic as the def=
ault for
>> =C2=A0 =C2=A0thread_lock_block(). This means that thread_lock_block() wi=
ll not
>> =C2=A0 =C2=A0disable interrupts when called (and consequently thread_unl=
ock_block()
>> =C2=A0 =C2=A0will not re-enabled them when called). This should be done =
manually
>> =C2=A0 =C2=A0when necessary.
>> =C2=A0 =C2=A0Note, however, that ULE's thread_unblock_switch() is not re=
aped
>> =C2=A0 =C2=A0because it does reflect a difference in semantic due in ULE=
 (the
>> =C2=A0 =C2=A0td_lock may not be necessarilly still blocked_lock when cal=
ling this).
>> =C2=A0 =C2=A0While asymmetric, it does describe a remarkable difference =
in semantic
>> =C2=A0 =C2=A0that is good to keep in mind.
>>
>> =C2=A0[0] Reported by: =C2=A0 =C2=A0 =C2=A0Kohji Okuno
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0<okuno dot kohji at jp dot panasonic dot com>
>> =C2=A0Tested by: =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Giovanni Trema=
terra
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0<giovanni dot trematerra at gmail dot com>
>> =C2=A0MFC: =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A02 weeks
>>
>> Modified:
>> =C2=A0head/sys/kern/kern_mutex.c
>> =C2=A0head/sys/kern/sched_4bsd.c
>> =C2=A0head/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.

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?3bbf2fe11001252310r408a6be4j9bc42618394b3e3d>