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

[Ken Brown]

On 9/2/2021 3:35 PM, 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.

Yes.  I actually copied that bug from fhandler_base_overlapped::raw_write.  It's 
amazing that no one has no one has bumped into it up to now.
> 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?

Will do.

Ken