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

[Ken Brown]

On 9/3/2021 7:41 AM, Corinna Vinschen wrote:
> On Sep  3 13:31, Corinna Vinschen wrote:
>> On Sep  3 19:13, Takashi Yano wrote:
>>> On Fri, 3 Sep 2021 19:00:46 +0900
>>> Takashi Yano wrote:
>>>> On Thu, 2 Sep 2021 21:35:21 +0200
>>>> Corinna Vinschen wrote:
>>>>> On Sep  2 21:00, Corinna Vinschen wrote:
>>>>> 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?
>>>>
>>>> Thanks.
>>>>
>>>> Your code seems that read() returns only the partial data even
>>>> if the pipe stil has more data. Is this by design?
>>>>
>>>> This happes in both blocking and non-blocking case.
>>>
>>> The patch attached seems to fix the issue.
>>
>> I'm sorry, but I don't see what your patch is supposed to do in the
>> first place.  What I see is that it now calls NtQueryInformationFile
>> even in the non-blocking case, which is not supposed to happen.
>>
>> I'm a bit puzzled what the actual bug is.
>>
>> The code changing len is only called if there's no data in the pipe.
>> In that case we only request a partial buffer so as not to block
>> the writer on select.
>>
>> If there *is* data in the pipe, it will just go straight to the
>> NtReadFile code without changing len.
>>
>> Where's the mistake?
> 
> Oh, wait.  Do you mean, if we only request less than len bytes, but
> after NtReadFile there's still data in the buffer, we should try to
> deplete the buffer up to len bytes in a subsequent NtReadFile?
> 
> I thought this is unnecessary, actually, because of POSIX:
> 
>     The standard developers considered adding atomicity requirements  to  a
>     pipe  or FIFO, but recognized that due to the nature of pipes and FIFOs
>     there could be no guarantee of atomicity of reads of {PIPE_BUF} or  any
>     other size that would be an aid to applications portability.

I agree.  I think if read returns less than what was requested, it's up to the 
caller to call read again if desired.

One tiny thing I noticed: get_obj_handle_count can return 0.  So the line

   reader_count = get_obj_handle_count (get_handle ());

should be

   reader_count = get_obj_handle_count (get_handle ()) ?: 1;

Or else get_obj_handle_count should be changed to return 1 instead of 0 if 
NtQueryObject fails.

Ken