Re: Bug about sched_4bsd?

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.

Thanks,
Attilio


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