Bug about sched_4bsd?

[Attilio Rao]

2010/1/19 Attilio Rao <attilio at freebsd.org>:
> 2010/1/19 Jeff Roberson <jroberson at jroberson.net>:
>> On Tue, 19 Jan 2010, Kohji Okuno wrote:
>>
>>> 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.
> Without the patch, there is a general problem of maybe_preempt()
> because sched_switch() will handle TDF_NEEDRESCHED just in racy ways
> (not ensuring atomicity of td_lock operations for sleeping threads).
> That's, however, still not specific to maybe_preempt() only. However:
> * If you make a problem about the callers of maybe_resched() I agree.
> The callers should assert for sched_lock to be in place. But that is
> not a general problem of maybe_resched(), it is on the callers
> ballpark
> * If you make a problem about the locking itself, the patch IMHO
> should fix it or there is still something I can't see.

s/maybe_preempt/maybe_resched, of course :(

Attilio


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