Bug#790571: please give-back capnproto on mipsel

Control: tags -1 +patch

Alrighty, potential patch is attached. Can you folks try it out & let
me know if it works? If all looks good I'll prep a new upload & send
the patch upstream.

Dejan, you were right on the money with the page size thing. Looks
like the write operation in async-unix-test.c++ writes data to the
pipe until the underlying buffer is completely filled. At that point,
the write operation fails & the test continues until we hit the read
operation. At that point we try to read 4096 bytes. It seems like if
PIPE_BUF > 4096 (where PIPE_BUF is typically the size of a page), the
pipe doesn't become writable & so the test fails.

I find this behavior a little surprising, but I can reproduce it
myself on x86_64 by simply changing the buffer size to something less
than PIPE_BUF. Interesting stuff.

With this patch applied I wouldn't be surprised if we saw the write
notification fire multiple times (once per PIPE_BUF bytes read). Even
if that is happening, I feel like we're testing the intended behavior.

Cheers,
Tom


On Sat, Aug 1, 2015 at 9:04 AM, Tom Lee <[email protected]> wrote:
> Control: tags +confirmed
>
> Dejan, Arturo, thanks for looking into this. Sorry I've been so slow
> to get back to you.
>
> Arturo, I understand you've followed up with upstream regarding the
> affected tests & Kenton's helping you out with some experimental
> patches to disable the failing tests. I'd be interested to know how
> that works out, but I do worry that by disabling the tests we'll be
> potentially glossing over a real issue.
>
> Dejan, I think you might be onto something with that 4k buffer but let
> me look into it a little. Are you aware of any porterboxes with
> similar setups to mips-aql-02 + mipsel-manda-0{1,2}?
> https://db.debian.org/machines.cgi?host=eder (Loongson 2E) looks like
> it might be a promising candidate from a quick google around.
>
> I'm guessing this is likely what's causing builds to fail on several
> other archs too.
>
> Cheers,
> Tom
>
> On Fri, Jul 17, 2015 at 8:20 AM, Dejan Latinovic
> <[email protected]> wrote:
>>
>> Hi,
>> I have tested capnproto on a few local machines.
>> Initially, build failed on all boards.
>> On different MIPS boards, different tests were failing.
>>
>>
>> AsyncUnixTest.WriteObserver fails if the kernel PAGESIZE is larger that 4k.
>>
>> After I reduced PAGESIZE to 4096 on CI20, all tests passed.
>>
>> The solution could be to increase buffer size:
>>> char buffer[4096]
>> (src/kj/async-unix-test.c++ +416)
>> I had increased it to 16384 and tried it on Loongson 3A
>> (PAGESIZE is 16k, same board as mipsel-manda-01, mipsel-manda-02),
>> all test passed.
>> We should keep on mind that pagesize on some MIPS board is up to 64k.
>> This solution should be discussed upstream.
>>
>>
>> On EdgeRouter Pro (mips-aql-02),
>> these two test failed:
>>> [  FAILED  ] 2 tests, listed below:
>>> [  FAILED  ] AsyncUnixTest.SignalWithValue
>>> [  FAILED  ] AsyncUnixTest.SignalWithPointerValue
>>
>> I will do further investigating.
>>
>> Best Regards,
>> Dejan
>
>
>
> --
> Tom Lee / http://tomlee.co / @tglee



-- 
Tom Lee / http://tomlee.co / @tglee

Description: Read PIPE_BUF bytes from pipes in tests.
  When we write() <= PIPE_BUF bytes to an anonymous pipe on Linux,
  PIPE_BUF bytes appear to be received by the other side of the
  pipe (irrespective of the length passed to write()).

  This would cause the read() in async-unix-test to succeed with
  EAGAIN when PIPE_BUF >= 4096 because the pipe would not enter the
  writable state until all PIPE_BUF bytes have been read.

  Affects certain flavors of mips{,el} and ppc{,64el} as far as I
  can tell.
Author: Tom Lee <[email protected]>
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=790571
Last-Update: 2015-08-01
--- a/src/kj/async-unix-test.c++
+++ b/src/kj/async-unix-test.c++
@@ -394,15 +394,20 @@
   KJ_SYSCALL(pipe(pipefds));
   kj::AutoCloseFd infd(pipefds[0]), outfd(pipefds[1]);
   setNonblocking(outfd);
+  setNonblocking(infd);
 
   UnixEventPort::FdObserver observer(port, outfd, UnixEventPort::FdObserver::OBSERVE_WRITE);
 
-  // Fill buffer.
+  // Fill the pipe buffer.
   ssize_t n;
   do {
     KJ_NONBLOCKING_SYSCALL(n = write(outfd, "foo", 3));
   } while (n >= 0);
 
+  // Make sure we haven't hit some unexpected failure.
+  EXPECT_EQ(n, -1);
+  EXPECT_EQ(errno, EAGAIN);
+
   bool writable = false;
   auto promise = observer.whenBecomesWritable()
       .then([&]() { writable = true; }).eagerlyEvaluate(nullptr);
@@ -414,7 +419,13 @@
   EXPECT_FALSE(writable);
 
   char buffer[4096];
-  KJ_SYSCALL(read(infd, &buffer, sizeof(buffer)));
+  do {
+      KJ_NONBLOCKING_SYSCALL(n = read(infd, &buffer, sizeof(buffer)));
+  } while (n >= 0);
+
+  // Make sure we've fully drained the buffer.
+  EXPECT_EQ(n, -1);
+  EXPECT_EQ(errno, EAGAIN);
 
   loop.run();
   port.poll();