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. EinsteinReceived on Tue Jan 19 2010 - 08:53:25 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:00 UTC