On Thu, Dec 03, 2020 at 11:13:35PM +0100, Alexander Graf wrote: > > On 03.12.20 19:42, Peter Collingbourne wrote: > > On Thu, Dec 3, 2020 at 1:41 AM Roman Bolshakov <r.bolsha...@yadro.com> > > wrote: > > > On Mon, Nov 30, 2020 at 04:00:11PM -0800, Peter Collingbourne wrote: > > > > What I observe is that when returning from a pending signal pselect > > > > consumes the signal (which is also consistent with my understanding of > > > > what pselect does). That means that it doesn't matter if we take a > > > > second WFx exit because once we reach the pselect in the second WFx > > > > exit the signal will have been consumed by the pselect in the first > > > > exit and we will just wait for the next one. > > > > > > > Aha! Thanks for the explanation. So, the first WFI in the series of > > > guest WFIs will likely wake up immediately? After a period without WFIs > > > there must be a pending SIG_IPI... > > > > > > It shouldn't be a critical issue though because (as defined in D1.16.2) > > > "the architecture permits a PE to leave the low-power state for any > > > reason, it is permissible for a PE to treat WFI as a NOP, but this is > > > not recommended for lowest power operation." > > > > > > BTW. I think a bit from the thread should go into the description of > > > patch 8, because it's not trivial and it would really be helpful to keep > > > in repo history. At least something like this (taken from an earlier > > > reply in the thread): > > > > > > In this implementation IPI is blocked on the CPU thread at startup and > > > pselect() is used to atomically unblock the signal and begin sleeping. > > > The signal is sent unconditionally so there's no need to worry about > > > races between actually sleeping and the "we think we're sleeping" > > > state. It may lead to an extra wakeup but that's better than missing > > > it entirely. > > Okay, I'll add something like that to the next version of the patch I send > > out. > > > If this is the only change, I've already added it for v4. If you want me to > change it further, just let me know what to replace the patch description > with. > >
Thanks, Alex. I'm fine with the description and all set. -Roman