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

[Ken Brown]

On 9/12/2021 11:10 AM, Ken Brown wrote:
> On 9/12/2021 7:04 AM, Takashi Yano wrote:
>> On Sun, 12 Sep 2021 17:48:49 +0900
>> Takashi Yano wrote:
>>> On Sat, 11 Sep 2021 09:12:02 -0400
>>> Ken Brown wrote:
>>>> On 9/10/2021 10:35 PM, Takashi Yano wrote:
>>>>> On Fri, 10 Sep 2021 22:17:21 -0400
>>>>> Ken Brown wrote:
>>>>>> On 9/10/2021 6:57 PM, Takashi Yano wrote:
>>>>>>> On Fri, 10 Sep 2021 11:17:58 -0400
>>>>>>> Ken Brown wrote:
>>>>>>>> I've rerun your test with the latest version, and the test results are 
>>>>>>>> similar.
>>>>>>>>      I've also run a suite of fifo tests that I've accumulated, and they 
>>>>>>>> all pass
>>>>>>>> also, so I pushed your patch.
>>>>>>>>
>>>>>>>> I think we're in pretty good shape now.  The only detail remaining, 
>>>>>>>> AFAIK, is
>>>>>>>> how to best avoid a deadlock if the pipe has been created by a non-Cygwin
>>>>>>>> process.  I've proposed a timeout, but maybe there's a better idea.
>>>>>>>
>>>>>>> I am not pretty sure what is the problem, but is not the following
>>>>>>> patch enough?
>>>>>>>
>>>>>>> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
>>>>>>> index d309be2f7..13fba9a14 100644
>>>>>>> --- a/winsup/cygwin/fhandler.h
>>>>>>> +++ b/winsup/cygwin/fhandler.h
>>>>>>> @@ -1205,6 +1205,7 @@ public:
>>>>>>>       select_record *select_except (select_stuff *);
>>>>>>>       char *get_proc_fd_name (char *buf);
>>>>>>>       int open (int flags, mode_t mode = 0);
>>>>>>> +  void open_setup (int flags);
>>>>>>>       void fixup_after_fork (HANDLE);
>>>>>>>       int dup (fhandler_base *child, int);
>>>>>>>       int close ();
>>>>>>> diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
>>>>>>> index 6994a5dce..d84e6ad84 100644
>>>>>>> --- a/winsup/cygwin/fhandler_pipe.cc
>>>>>>> +++ b/winsup/cygwin/fhandler_pipe.cc
>>>>>>> @@ -191,6 +191,17 @@ out:
>>>>>>>       return 0;
>>>>>>>     }
>>>>>>>
>>>>>>> +void
>>>>>>> +fhandler_pipe::open_setup (int flags)
>>>>>>> +{
>>>>>>> +  fhandler_base::open_setup (flags);
>>>>>>> +  if (get_dev () == FH_PIPER && !read_mtx)
>>>>>>> +    {
>>>>>>> +      SECURITY_ATTRIBUTES *sa = sec_none_cloexec (flags);
>>>>>>> +      read_mtx = CreateMutexW (sa, FALSE, NULL);
>>>>>>> +    }
>>>>>>> +}
>>>>>>> +
>>>>>>>     off_t
>>>>>>>     fhandler_pipe::lseek (off_t offset, int whence)
>>>>>>>     {
>>>>>>>
>>>>>>>
>>>>>>> AFAIK, another problem remaining is:
>>>>>>>
>>>>>>> On Mon, 6 Sep 2021 14:49:55 +0200
>>>>>>> Corinna Vinschen wrote:
>>>>>>>> - What about calling select for writing on pipes read by non-Cygwin
>>>>>>>>      processes?  In that case, we still can't rely on WriteQuotaAvailable,
>>>>>>>>      just as before.
>>>>>>
>>>>>> This is the problem I was talking about.  In this case the non-Cygwin process
>>>>>> might have a large pending read, so that the Cygwin process calling select on
>>>>>> the write side will see WriteQuotaAvailable == 0.  This could lead to a 
>>>>>> deadlock
>>>>>> with the Cygwin process waiting for write ready while the non-Cygwin 
>>>>>> process is
>>>>>> blocked trying to read.
>>>>>
>>>>> Then, the above patch is for another issue.
>>>>> The problem happes when:
>>>>> 1) Start command prompt.
>>>>> 2) Run 'echo AAAAAAAAAAAA | \cygwin64\bin\cat
>>>>> This causes hang up in cat. In this case, pipe is created by cmd.exe.
>>>>> Therefore, read_mtx is not created.
>>>>
>>>> Confirmed, and your patch fixes it.  Maybe you should check for error in the
>>>> call to CreateMutexW and print a debug message in that case.
>>>>
>>>>>> My suggestion is that we impose a timeout in this situation, after which 
>>>>>> select
>>>>>> reports write ready.
>>>>>
>>>>> Keeping read handle in write pipe (Corinna's query_hdl) causes problem
>>>>> that write side cannot detect close on read side.
>>>>> Is it possible to open read handle temporally when pipe_data_available()
>>>>> is called?
>>>>
>>>> That would be nice, but I have no idea how you could do that.
>>>
>>> Hmm. Then, what about PoC code attached? This returns to Corinna's
>>> query_hdl, and counts read/write handles to detect closing reader side.
>>>
>>> If the number of read handles is equal to number of write handles,
>>> only the pairs of write handle and query_hdl are alive. So, read pipe
>>> supposed to be closed.
>>>
>>> This patch depends another patch I posted a few hours ago.
>>
>> Revised a bit.
> 
> I don't see how this solves the problem.  In the case we were worried about 
> where we have a non-Cygwin reader, the writer has no query_hdl, and you're just 
> always reporting write ready, aren't you?  Or am I missing something?

BTW, we could just decide that always reporting write ready in this corner case 
is acceptable.  But then we could just do that without going back to query_hdl.

Ken