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

[Takashi Yano <takashi.yano@nifty.ne.jp>]

Hi Corinna, Ken,

On Fri, 3 Sep 2021 09:27:37 -0400
Ken Brown wrote:
> On 9/3/2021 8:22 AM, Takashi Yano wrote:
> > 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.
> 
> I understand the reasoning now, but I think your patch isn't quite right.  As it 
> stands, if the call to NtQueryInformationFile fails but total_length != 0, 
> you're trying to read again without knowing that there's data in the pipe.
> 
> Also, I think you need the following:
> 
> diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
> index ef7823ae5..46bb96961 100644
> --- a/winsup/cygwin/fhandler_pipe.cc
> +++ b/winsup/cygwin/fhandler_pipe.cc
> @@ -348,8 +348,13 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
>       CloseHandle (evt);
>     if (status == STATUS_THREAD_SIGNALED)
>       {
> -      set_errno (EINTR);
> -      len = (size_t) -1;
> +      if (total_len == 0)
> +       {
> +         set_errno (EINTR);
> +         len = (size_t) -1;
> +       }
> +      else
> +       len = total_len;
>       }
>     else if (status == STATUS_THREAD_CANCELED)
>       pthread::static_cancel_self ();

Thanks for your advice. I fixed the issue and attached new patch.

On Fri, 3 Sep 2021 17:37:13 +0200
Corinna Vinschen wrote:
> Hmm, I see the point, but we might have another problem with that.
> 
> We can't keep the mutex while waiting on the pending read, and there
> could be more than one pending read running at the time.  if so,
> chances are extremly high, that the data written to the buffer gets
> split like this:
> 
>    reader 1		               reader 2
> 
>    calls read(65536)                   calls read(65536)
> 
>    calls NtReadFile(16384 bytes)
>                                        calls NtReadFile(16384 bytes)
> 
> writer writes 65536 bytes
> 
>    wakes up and gets 16384 bytes
>                                        wakes up and gets 16384 bytes
>    gets the mutex, calls
>    NtReadFile(32768) which 
>    returns immediately with
>    32768 bytes added to the
>    caller's buffer.
> 
> so the buffer returned to reader 1 is 49152 bytes, with 16384 bytes
> missing in the middle of it, *without* the reader knowing about that
> fact.  If reader 1 gets the first 16384 bytes, the 16384 bytes have
> been read in a single call, at least, so the byte order is not
> unknowingly broken on the application level.
> 
> Does that make sense?

Why can't we keep the mutex while waiting on the pending read?
If we can keep the mutex, the issue above mentioned does not
happen, right?

What about the patch attached? This keeps the mutex while read()
but I do not see any defects so far.


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