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

[Takashi Yano]

On Mon, 13 Sep 2021 08:54:31 +0900
Takashi Yano wrote:
> On Sun, 12 Sep 2021 17:46:47 -0400
> Ken Brown wrote:
> > 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.
> 
> The various combination of cygwin and non-cygwin cases resuts in:
> 
> W P R: current, query_hdl
> c c c: OK     , OK
> c c n: NG     , OK
> c n c: OK     , OK
> c n n: NG     , select() always report write ready

Sorry, this was not correct. In fact,
W P R: current, query_hdl
c c c: OK     , OK
c c n: NG     , OK
c n c: OK     , select() always report write ready
c n n: NG     , select() always report write ready

:-(

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