On Fri, Jan 08, 2021 at 12:59:16PM -0600, Scott Cheloha wrote: > On Mon, Dec 28, 2020 at 11:41:52AM -0300, Martin Pieuchot wrote: > > On 08/12/20(Tue) 10:06, Martin Pieuchot wrote: > > > Diff below aims to simplify the API to put a thread on a sleep queue and > > > reduce it to the following: > > > > > > sleep_setup(); > > > /* check condition or release lock */ > > > sleep_finish(); > > > > > > It is motivated by my work to sleep the SCHED_LOCK() but might as well > > > prevent/fix some bugs. > > > > > > The tricky part of the current implementation is that sleep_setup_signal() > > > can already park/stop the current thread resulting in a context change. > > > Should any custom accounting / lock check happen before that? At least > > > two lock primitives do so currently: drm's schedule_timeout() and > > > rwlock's rw_enter(). > > > > > > As a result of this diff various states can be removed and sleep_finish() > > > contains the following magic: > > > > > > 1. check for signal/parking > > > 2. context switch or remove from sleep queue > > > 3. check for signal/parking > > > > > > Note that sleep_finish() could be simplified even further but I left > > > that for later to ease the review. > > > > > > Comments? Oks? > > > > Anyone? > > I really like this simplification. > > It also makes my forthcoming kclock changes to tsleep_nsec(9)/etc. > simpler, so it's doubly good for me. > > I was hoping someone would step forward and OK this but nobody did, at > least not publicly. > > I see claudio@ is trying to break off a piece of this for commit in a > different thread. Unsure if that means this is dead or just being cut > up and merged piecemeal. > > FWIW, ok cheloha@. Obviously you need more OKs. > > Even if this is dead, some other simplification in this vein would be > nice.
I agree; I read through this, tried to puzzle my way through what would happen if sleep_setup_signal() slept and figured it was OK. I don't think I've written any code using this API so my opinion doesn't count for much, but I've always found it more complicated than I'd like, and particularly hard to determine if it's being used correctly, so simplifying it definitely sounds good to me.
