Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 22 Jan 2010 17:05:13 +0900 (JST)
From:      Kohji Okuno <okuno.kohji@jp.panasonic.com>
To:        attilio@freebsd.org
Cc:        okuno.kohji@jp.panasonic.com, freebsd-current@freebsd.org, jroberson@jroberson.net
Subject:   Re: Bug about sched_4bsd?
Message-ID:  <20100122.170513.468378203492549701.okuno.kohji@jp.panasonic.com>
In-Reply-To: <3bbf2fe11001211532i1cc4eaa8n3d56df04f337298@mail.gmail.com>
References:  <3bbf2fe11001190152k15c24f70k876762817bf522c1@mail.gmail.com> <20100120.115218.999284356098982813.okuno.kohji@jp.panasonic.com> <3bbf2fe11001211532i1cc4eaa8n3d56df04f337298@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Hello, Attilio,

>>>>> I think setpriority() can set priority to sleeping threads.
>>>>> Is it really safe?
>>>>
>>>> I agree, in this code path maybe_resched is not properly locking c=
urthread.
>>>> =A0curthread will be sched_lock and the sleeping thread will be a =
sleepq lock.
>>>> =A0I believe that since &sched_lock is ordered after container loc=
ks it would
>>>> be sufficient to compare the two td_lock pointers and acquire sche=
d_lock if
>>>> they are not equal. =A0Someone should look at other maybe_resched =
callers or
>>>> add an assert that curthread->td_lock is always &sched_lock in thi=
s
>>>> function.
>>>
>>> I'm not sure I understand you well here, but I generally don't agre=
e,
>>> if we speak about the current code plus the patch I posted.
>>
>> I understood. If the current code plus your patch, meybe_resched() i=
s
>> no problem. I think, your patch is perfect if theare is no problem
>> even if a sleeping thread sets &sched_lock to td->td_lock.
>>
>> Why do we call thread_lock_set() in sleepq_switch() and turnstile_wa=
it()?
>> In case of sched_4bsd, is not thread_lock_set() needed?
> =

> This question may be needing a very long answer :)
> =

> The short one, though, is that the thread_*lock*() interface handle
> the locking of the thread containers (runqueues, sleepqueues,
> turnstiles) transparently and in atomic way.
> What happens is that threads may be (mostly, with some notable
> exceptions like the ithread case being parked on IWAIT and not having=

> a specific container) in one of the three above mentioned containers
> which also need to handle flags and option specific to the thread and=

> the container. In order to do that atomically you may need 'global'
> locks that protects such interactions (thus you have sched_lock which=

> protects runqueue, sleepqueue locks and turnstile locks). You could
> also have just a global spinlock, but that would mean having a lot of=

> intollerable contention on it.
> thread_lock_set() is just used to switch locks when threads passes
> from a container to another.
> For example, immagine a thread running  that goes blocking on a
> turnstile. At some point, the thread container lock, as the thread
> switches from runqueue to turnstile, needs to switch from sched_lock
> to ts_lock and it is precisely when thread_lock_set() takes place.
> Thus when the thread is resumed for running, it needs to switch again=

> from ts_lock to sched_lock.
> =

> Attilio

Thank you very much your detailed expositoin.
I can understand. I'm looking forward to the good result of your new
patch.

Thank you,
 Kohji Okuno.




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100122.170513.468378203492549701.okuno.kohji>