[PATCH v8] Cygwin: pipe: Switch pipe mode to blocking mode by default

[Takashi Yano]

On Mon, 28 Oct 2024 10:55:31 +0100
Corinna Vinschen wrote:
> Hi Takashi,
> 
> On Oct 27 17:57, Takashi Yano wrote:
> > Hi Corinna,
> > 
> > On Thu, 24 Oct 2024 12:21:04 +0200
> > Corinna Vinschen wrote:
> > > > > Before:
> > > > > 
> > > > >   $ ./x 40000
> > > > >   pipe capacity: 65536
> > > > >   write: writable 1, 40000 25536
> > > > >   write: writable 1, 24576 960
> > > > >   write: writable 0, 512 448
> > > > >   write: writable 0, 256 192
> > > > >   write: writable 0, 128 64
> > > > >   write: writable 0, 64 0
> > > > >   write: writable 0, -1 / Resource temporarily unavailable
> > > > > 
> > > > > After:
> > > > > 
> > > > >   $ ./x 40000
> > > > >   pipe capacity: 65536
> > > > >   write: writable 1, 40000 25536
> > > > >   write: writable 1, 25536 0
> > > > >   write: writable 0, -1 / Resource temporarily unavailable
> > > > > 
> > > > > This way, we get into the EAGAIN case much faster again, which was
> > > > > one reason for 170e6badb621.
> > > > > 
> > > > > Does this make more sense, and if so, why?  If this is really the
> > > > > way to go, the comment starting at line 634 (after applying your patch)
> > > > > will have to be changed as well.
> > > > 
> > > > Perhaps, I did not understand intent of 170e6badb621. Could you please
> > > > provide the test program (./x)? I will check my code.
> > > 
> > > I attached it.  If you call it with just the number of bytes per write,
> > > e.g. `./x 12345', the writes are blocking.  If you add another parameter,
> > > e.g. `./x 12345 1', the writes are nonblocking.
> > 
> > Thanks for the test case.
> > I think I could restore the previous behaviour. Please try v9 patch.
> > 
> > CYGWIN_NT-10.0-19045 HP-Z230 3.5.4-1.x86_64 2024-08-25 16:52 UTC x86_64 Cygwin
> > $ ./a.exe 40000 1
> > pipe capacity: 65536
> > write: writable 1, 40000 25536
> > write: writable 1, 24576 960
> > write: writable 0, -1 / Resource temporarily unavailable
> > 
> > Just after the commit 170e6badb621 (master branch)
> 
> Oops.  You tested in the wrong spot.  The original patch wasn't quite
> polished, the followup patches 1ed909e047a2 and 686e46ce7148 are also
> required to show the intended behaviour, and the intended behaviour is
> the same in the blocking and non-blocking case...
> 
> > $ ./a.exe 40000 1
> > pipe capacity: 65536
> > write: writable 1, 40000 25536
> > write: writable 1, 24576 960
> > write: writable 0, -1 / Resource temporarily unavailable
> 
> So this should actually be:
> 
>   pipe capacity: 65536
>   write: writable 1, 40000 25536
>   write: writable 1, 24576 960
>   write: writable 0, 512 448
>   write: writable 0, 256 192
>   write: writable 0, 128 64
>   write: writable 0, 64 0
>   write: writable 0, -1 / Resource temporarily unavailable
> 
> just as in the blocking case.
> 
> The ideal commit for testing the intendend behaviour is f78009cb1ccf,
> because that's your regression fix slowing down writes.
> 
> As I wrote in the commit message of 170e6badb621, the idea is to defer
> EAGAIN/EINTR when the write buffer starts to be filled up. 
> 
> The code I came up with does NOT resemble Linux closely, because the way
> Linux pipe buffers work is by some simple but fast paging mechanism,
> which may even lead to pipes being smaller than PIPE_BUF.  Nevertheless,
> except in some border cases, Linux often still returns some non-0 value
> when our former code already returned EAGAIN/EINTR.
> 
> While this was mainly a problem in the blocking case, I thought the
> buffer usage computation should be identical between blocking and
> non-blocking, just as on Linux.
> 
> > Please try:
> > $ ./a.out `expr 65536 - 4096 + 543` 1
> > pipe capacity: 65536
> > write: writable 1, 61983 3553
> > write: writable 0, 543 3010
> > write: writable 0, 543 2467
> > write: writable 0, 543 1924
> > write: writable 0, 543 1381
> > write: writable 0, 543 838
> > write: writable 0, 543 295
> > write: writable 0, -1 / Resource temporarily unavailable

The above case was result in Linux environment.

> > $ ./a.out `expr 65536 - 4096 + 1234` 1
> > pipe capacity: 65536
> > write: writable 1, 62674 2862
> > write: writable 0, 1234 1628
> > write: writable 0, 1234 394
> > write: writable 0, -1 / Resource temporarily unavailable

This also is the result in Linux environment.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>