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

Takashi Yano takashi.yano@nifty.ne.jp
Sun Sep 12 23:54:31 GMT 2021


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

where

W: Writer
P: Pipe
R: Reder
c: cygwin
n: non-cygwin

* Reder requests larger block than pipe size.
* Writer cannot be non-cygwin because we assume the case
  that writer uses select().

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


More information about the Cygwin-developers mailing list