Re: Bug about sched_4bsd?

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

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.