Re: Bug about sched_4bsd?

2010/1/20 Kohji Okuno <okuno.kohji_at_jp.panasonic.com>:
> 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 curthread.
>>>  curthread will be sched_lock and the sleeping thread will be a sleepq lock.
>>>  I believe that since &sched_lock is ordered after container locks it would
>>> be sufficient to compare the two td_lock pointers and acquire sched_lock if
>>> they are not equal.  Someone should look at other maybe_resched callers or
>>> add an assert that curthread->td_lock is always &sched_lock in this
>>> function.
>>
>> I'm not sure I understand you well here, but I generally don't agree,
>> if we speak about the current code plus the patch I posted.
>
> I understood. If the current code plus your patch, meybe_resched() is
> 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_wait()?
> 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


-- 
Peace can only be achieved by understanding - A. Einstein