cygrunsrv + sshd + rsync = 20 times too slow -- throttled?

[Takashi Yano]

On Fri, 3 Sep 2021 13:41:45 +0200
Corinna Vinschen wrote:
> On Sep  3 13:31, Corinna Vinschen wrote:
> > On Sep  3 19:13, Takashi Yano wrote:
> > > On Fri, 3 Sep 2021 19:00:46 +0900
> > > Takashi Yano wrote:
> > > > On Thu, 2 Sep 2021 21:35:21 +0200
> > > > Corinna Vinschen wrote:
> > > > > On Sep  2 21:00, Corinna Vinschen wrote:
> > > > > It's getting too late again.  I drop off for tonight, but I attached
> > > > > my POC code I have so far.  It also adds the snippets from my previous
> > > > > patch which fixes stuff Takashi found during testing.  It also fixes
> > > > > something which looks like a bug in raw_write:
> > > > > 
> > > > > -	  ptr = ((char *) ptr) + chunk;
> > > > > +	  ptr = ((char *) ptr) + nbytes_now;
> > > > > 
> > > > > Incrementing ptr by chunk bytes while only nbytes_now have been written
> > > > > looks incorrect.
> > > > > 
> > > > > As for the reader, it makes the # of bytes to read dependent on the
> > > > > number of reader handles.  I don't know if that's such a bright idea,
> > > > > but this can be changed easily.
> > > > > 
> > > > > Anyway, this runs all my testcases successfully but they are anything
> > > > > but thorough.
> > > > > 
> > > > > Patch relativ to topic/pipe attached.  Would you both mind to take a
> > > > > scrutinizing look?
> > > > 
> > > > Thanks.
> > > > 
> > > > Your code seems that read() returns only the partial data even
> > > > if the pipe stil has more data. Is this by design?
> > > > 
> > > > This happes in both blocking and non-blocking case.
> > > 
> > > The patch attached seems to fix the issue.
> > 
> > I'm sorry, but I don't see what your patch is supposed to do in the
> > first place.  What I see is that it now calls NtQueryInformationFile
> > even in the non-blocking case, which is not supposed to happen.
> > 
> > I'm a bit puzzled what the actual bug is.
> > 
> > The code changing len is only called if there's no data in the pipe.
> > In that case we only request a partial buffer so as not to block
> > the writer on select.
> > 
> > If there *is* data in the pipe, it will just go straight to the
> > NtReadFile code without changing len.
> > 
> > Where's the mistake?
> 
> Oh, wait.  Do you mean, if we only request less than len bytes, but
> after NtReadFile there's still data in the buffer, we should try to
> deplete the buffer up to len bytes in a subsequent NtReadFile?

Yes. I am sorry, my intent was not clear because I did more than
necessary in the previous patch. Please see attached patch revised.

> I thought this is unnecessary, actually, because of POSIX:
> 
>    The standard developers considered adding atomicity requirements  to  a
>    pipe  or FIFO, but recognized that due to the nature of pipes and FIFOs
>    there could be no guarantee of atomicity of reads of {PIPE_BUF} or  any
>    other size that would be an aid to applications portability.

POSIX says:
    The value returned may be less than nbyte if the number of bytes left
    in the file is less than nbyte, if the read() request was interrupted
    by a signal, or if the file is a pipe or FIFO or special file and has
                                                                      ~~~
    fewer than nbyte bytes immediately available for reading.
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

https://pubs.opengroup.org/onlinepubs/009604599/functions/read.html

If it is turned over, read() should read all data immediately available,
I think.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pipe-corinna4-fix.patch
Type: application/octet-stream
Size: 2301 bytes
Desc: not available
URL: <https://cygwin.com/pipermail/cygwin-developers/attachments/20210903/5ace1365/attachment.obj>