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

[Takashi Yano]

Hi Corinna,

On Thu, 2 Sep 2021 21:35:21 +0200
Corinna Vinschen wrote:
> On Sep  2 21:00, Corinna Vinschen wrote:
> > On Sep  2 09:01, Ken Brown wrote:
> > > On 9/2/2021 4:17 AM, Corinna Vinschen wrote:
> > > > What if the readers never request more than, say, 50 or even 25% of the
> > > > available buffer space?  Our buffer is 64K and there's no guarantee that
> > > > any read > PIPE_BUF (== 4K) is atomic anyway.  This can work without
> > > > having to check the other side of the pipe.  Something like this,
> > > > ignoring border cases:
> > > > 
> > > > pipe::create()
> > > > {
> > > >     [...]
> > > >     mutex = CreateMutex();
> > > > }
> > > > 
> > > > pipe::raw_read(char *buf, size_t num_requested)
> > > > {
> > > >    if (blocking)
> > > >      {
> > > >        WFSO(mutex);
> > > >        NtQueryInformationFile(FilePipeLocalInformation);
> > > >        if (!fpli.ReadDataAvailable
> > > > 	  && num_requested > fpli.InboundQuota / 4)
> > > > 	num_requested = fpli.InboundQuota / 4;
> > > >        NtReadFile(pipe, buf, num_requested);
> > > >        ReleaseMutex(mutex);
> > > >      }
> > > > }
> > > > 
> > > > It's not entirely foolproof, but it should fix 99% of the cases.
> > > 
> > > I like it!
> > > 
> > > Do you think there's anything we can or should do to avoid a deadlock in the
> > > rare cases where this fails?  The only thing I can think of immediately is
> > > to always impose a timeout if select is called with infinite timeout on the
> > > write side of a pipe, after which we report that the pipe is write ready.
> > > After all, we've lived since 2008 with a bug that caused select to *always*
> > > report write ready.
> > 
> > Indeed.  Hmm.  What timeout are you thinking of?  Seconds?  Minutes?
> > 
> > > Alternatively, we could just wait and see if there's an actual use case in
> > > which someone encounters a deadlock.
> > 
> > Or that.  Fixing up select isn't too hard in that case, I guess.
> 
> It's getting too late again.  I drop off for tonight, but I attached
> my POC code I have so far.  It also adds the snippets from my previous
> patch which fixes stuff Takashi found during testing.  It also fixes
> something which looks like a bug in raw_write:
> 
> -	  ptr = ((char *) ptr) + chunk;
> +	  ptr = ((char *) ptr) + nbytes_now;
> 
> Incrementing ptr by chunk bytes while only nbytes_now have been written
> looks incorrect.
> 
> As for the reader, it makes the # of bytes to read dependent on the
> number of reader handles.  I don't know if that's such a bright idea,
> but this can be changed easily.
> 
> Anyway, this runs all my testcases successfully but they are anything
> but thorough.
> 
> Patch relativ to topic/pipe attached.  Would you both mind to take a
> scrutinizing look?

Sorry for replying to old post.

As for this patch, read_mtx was introduced. This handle is initialized
only for read pipe. However, this seems to be NULL even without
initialization in write pipe. I wonder why initializing read_mtx in
the constructor is not necessary.

How do you guarantee that read_mtx is NULL on the write pipe?

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