Re: [PATCH 1/3] Use reliable sem_wait interruption in nptl/tst-sem6.

[OndÅej BÃlka <neleai at seznam dot cz>]

On Mon, Dec 08, 2014 at 12:43:17PM +0100, Torvald Riegel wrote:
> On Sat, 2014-12-06 at 14:50 +0100, OndÅej BÃlka wrote:
> > On Fri, Dec 05, 2014 at 07:52:57PM +0100, Torvald Riegel wrote:
> > >    alarm (1);
> > >  
> > >    int res = sem_wait (&s);
> > > -  if (res == 0)
> > > -    {
> > > -      puts ("wait succeeded");
> > > -      return 1;
> > > -    }
> > > -  if (res != -1 || errno != EINTR)
> > > +  /* We accept all allowed behavior: Implementations that return EINTR and
> > > +     those that rely on correct interruption through signals to use sem_post
> > > +     in the signal handler.  */
> > > +  if (res != 0 && !(res == -1 && errno == EINTR))
> > >      {
> > > -      puts ("wait didn't fail with EINTR");
> > > +      puts ("wait neiter succeeded nor failed with EINTR");
> > >        return 1;
> > >      }
> > >  
> > 
> > That does change test logic as it originally failed when wait succeeded.
> 
> Yes, but why do you think that this is inconsistent?  The previous test
> didn't add a token in the signal handler, so if wait succeeded, then the
> test should fail.
> 
> However, the correct way to interrupt the semaphore with a signal is to
> add a token.  My patch does that.  Second, if we do not want to make
> timing assumptions (which the existing test would do if we add a token
> to the semaphore in the signal handler), then we need to accept that the
> (first) signal handler execution might happen before sem_wait actually
> executes.  Therefore, we must not fail in this case.
> 
> We have to correctly interrupt with signals because as the futex
> documentation stands (allowing return of EINTR on spurious wake-ups),
> there's no way to implement the specific behavior of the existing
> implementation (which assumes EINTR is returned *only* on signals).
> 
> IOW, the existing test does white-box testing with timing assumptions;
> with this patch, we do make a slightly different black-box test with no
> timing assumptions.

Which misses point of test which is to find bugs. What if in new fooarch 
assembly one forget to return -1 on interrupt which breaks user application 
which will assume that semaphore is locked, one can deal with rare false 
positives in tests.

One does not need justify this fact by forward progress/fairness
assumptions. Just assumption that OS which keeps all CPU idle for one
second while there is suitable task is simply broken. It would be better
to add serialization so no other test runs in parallel with this.