Re: 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 <debian@tomlee.co> 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
> <Dejan.Latinovic@imgtec.com> 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 <debian@tomlee.co>
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();
Reply to: