Re: [Qemu-devel] [RFC 11/38] qemu-thread: handle spurious futex_wait wakeups

[Emilio G. Cota]

On Thu, Sep 10, 2015 at 14:22:49 +0100, Alex Bennée wrote:
> Emilio G. Cota <address@hidden> writes:
> 
> > Signed-off-by: Emilio G. Cota <address@hidden>
> > ---
> >  util/qemu-thread-posix.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> > index 04dae0f..3760e27 100644
> > --- a/util/qemu-thread-posix.c
> > +++ b/util/qemu-thread-posix.c
> > @@ -303,7 +303,16 @@ static inline void futex_wake(QemuEvent *ev, int n)
> >  
> >  static inline void futex_wait(QemuEvent *ev, unsigned val)
> >  {
> > -    futex(ev, FUTEX_WAIT, (int) val, NULL, NULL, 0);
> > +    while (futex(ev, FUTEX_WAIT, (int) val, NULL, NULL, 0)) {
> > +        switch (errno) {
> > +        case EWOULDBLOCK:
> > +            return;
> > +        case EINTR:
> > +            break; /* get out of switch and retry */
> > +        default:
> > +            abort();
> 
> I'd be tempted to error_exit with the errno in this case so additional
> information is reported before we bail out.

Yes that's a good suggestion.

> The man pages seems to indicate other errnos are possible for FUTUX_WAIT
> although they may be unlikely:
> 
>        EACCES No read access to futex memory.
>        EFAULT Error retrieving timeout information from user space.
> 
>        I guess things would have gone very wrong for these
> 
>        EINVAL Invalid argument.
> 
>        Hard to get wrong
> 
>        ENFILE The system limit on the total number of open files has
>        been reached.
> 
>        Might happen under system load?
> 
>        ENOSYS Invalid operation specified in op.
> 
>        Hardcoded op so no
> 
>        ETIMEDOUT
>               Timeout during the FUTEX_WAIT operation.
> 
>        No timeout specified so we shouldn't hit it

Of these I'd say all would be bugs in our code except for ENFILE, so
it might be worth adding.

Thanks,

                Emilio