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

Takashi Yano takashi.yano@nifty.ne.jp
Tue Sep 14 08:08:38 GMT 2021


On Tue, 14 Sep 2021 04:37:18 +0900
Takashi Yano wrote:
> On Mon, 13 Sep 2021 20:32:33 +0200
> Corinna Vinschen wrote:
> > On Sep 12 20:04, Takashi Yano wrote:
> > > On Sun, 12 Sep 2021 17:48:49 +0900
> > > Takashi Yano wrote:
> > > > 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.
> > > [...]
> > 
> > What I miss is a bit more detailed commit message...
> 
> I am sorry, I will add more detail commit message.

Done.

> > > diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
> > > index 9b4255cfd..b051f5c03 100644
> > > --- a/winsup/cygwin/fhandler_pipe.cc
> > > +++ b/winsup/cygwin/fhandler_pipe.cc
> > > @@ -56,6 +56,8 @@ fhandler_pipe::set_pipe_non_blocking (bool nonblocking)
> > >    fpi.ReadMode = FILE_PIPE_BYTE_STREAM_MODE;
> > >    fpi.CompletionMode = nonblocking ? FILE_PIPE_COMPLETE_OPERATION
> > >      : FILE_PIPE_QUEUE_OPERATION;
> > > +  if (query_hdl)
> > > +    fpi.CompletionMode = FILE_PIPE_COMPLETE_OPERATION;
> > 
> > This should be a single expression, i.e.
> > 
> >    fpi.CompletionMode = nonblocking || query_hdl
> >                         ? FILE_PIPE_COMPLETE_OPERATION
> >                         : FILE_PIPE_QUEUE_OPERATION;
> > 
> > ideally combined with a comment.
> 
> Thanks. I'll do that.

Done.


> > But then again... you're basically switching the write side of
> > a pipe to nonblocking mode unconditionally.  The downside is a
> > busy wait:
> > 
> > >  fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
> > >  {
> > > @@ -493,7 +490,20 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
> > >  			      get_obj_handle_count (select_sem), NULL);
> > >  	  /* 0 bytes returned?  EAGAIN.  See above. */
> > >  	  if (NT_SUCCESS (status) && nbytes == 0)
> > > -	    set_errno (EAGAIN);
> > > +	    {
> > > +	      if (reader_closed ())
> > > +		{
> > > +		  set_errno (EPIPE);
> > > +		  raise (SIGPIPE);
> > > +		}
> > > +	      else if (is_nonblocking ())
> > > +		set_errno (EAGAIN);
> > > +	      else
> > > +		{
> > > +		  cygwait (select_sem, 10);
> > > +		  continue;
> > 
> > I'm a bit puzzled.  The cygwait branch neglects to check if select_sem
> > is NULL (the preceeding ReleaseSemaphore expression does!)
> > And then it doesn't matter if the caller got blocked or not, it will
> > always perform a continue.  So why do it at all?  Worse, if this
> > expression loops, it will eat up the semaphore, because each call will
> > decrement the semaphore count until it blocks.  That sounds wrong to me.
> 
> It is by design. ReleaseSemaphore() releases maximum number of semaphore
> which the waiter can exists. If only one writer and one reader exist,
> ReleaseSemaphore releases 2 semaphores. Then cygwait here consume semaphore
> two times and return to wait state.
> This wait state is released by raw_read() or close().
>  
> > Btw., while looking into the current pipe code, I wonder what select_sem
> > is doing in the pipe code at all so far.  It gets released, but it never
> > gets waited on?!?  Am I missing something?
> 
> The semaphore is waited in select.cc.
> But, wait. Wat happens if select() is not called? Released semaphore
> can be accumulated up to INT32_MAX!!?
> 
> Let me consider.

See the second patch attached. With this patch, only minimum number
of semaphores needed are released.


Please apply following two patches I attached to previous mail first:
0001-Cygwin-fhandler_base-dup-Reflect-O_CLOEXEC-to-inheri.patch
0002-Cygwin-pipe-fifo-Call-set_no_inheritance-for-adjunct.patch

Then, apply the patches attached this mail.
0001-Cygwin-pipe-Use-read-pipe-handle-for-select-on-write.patch
0002-Cygwin-pipe-fifo-Release-select_sem-semaphore-as-muc.patch

Thanks

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Cygwin-pipe-Use-read-pipe-handle-for-select-on-write.patch
Type: application/octet-stream
Size: 12170 bytes
Desc: not available
URL: <https://cygwin.com/pipermail/cygwin-developers/attachments/20210914/0f924252/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Cygwin-pipe-fifo-Release-select_sem-semaphore-as-muc.patch
Type: application/octet-stream
Size: 5277 bytes
Desc: not available
URL: <https://cygwin.com/pipermail/cygwin-developers/attachments/20210914/0f924252/attachment-0003.obj>


More information about the Cygwin-developers mailing list