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

[Takashi Yano <takashi.yano@nifty.ne.jp>]

[-- Attachment #1: Type: text/plain, Size: 2804 bytes --]

On Sun, 29 Aug 2021 11:57:04 -0400
Ken Brown wrote:
> On 8/29/2021 5:07 AM, Takashi Yano via Cygwin wrote:
> > On Sat, 28 Aug 2021 18:41:02 +0900
> > Takashi Yano wrote:
> >> On Sat, 28 Aug 2021 10:43:27 +0200
> >> Corinna Vinschen wrote:
> >>> On Aug 28 02:21, Takashi Yano via Cygwin wrote:
> >>>> On Fri, 27 Aug 2021 12:00:50 -0400
> >>>> Ken Brown wrote:
> >>>>> Two years ago I thought I needed nt_create to avoid problems when calling
> >>>>> set_pipe_non_blocking.  Are you saying that's not an issue?  Is
> >>>>> set_pipe_non_blocking unnecessary?  Is that the point of your modification to
> >>>>> raw_read?
> >>>>
> >>>> Yes. Instead of making windows read function itself non-blocking,
> >>>> it is possible to check if the pipe can be read before read using
> >>>> PeekNamedPipe(). If the pipe cannot be read right now, EAGAIN is
> >>>> returned.
> >>>
> >>> The problem is this:
> >>>
> >>>    if (PeekNamedPipe())
> >>>      ReadFile(blocking);
> >>>
> >>> is not atomic.  I. e., if PeekNamedPipe succeeds, nothing keeps another
> >>> thread from draining the pipe between the PeekNamedPipe and the ReadFile
> >>> call.  And as soon as ReadFile runs, it hangs indefinitely and we can't
> >>> stop it via a signal.
> >>
> >> Hmm, you are right. Mutex guard seems to be necessary like pty code
> >> if we go this way.
> > 
> > I have found that set_pipe_non_blocking() succeeds for both read and
> > write pipes if the write pipe is created by CreateNamedPipe() and the
> > read pipe is created by CreateFile() contrary to the current create()
> > code. Therefore, not only nt_create() but also PeekNamedPipe() become
> > unnecessary.
> > 
> > Please see the revised patch attached.
> 
> That's a great idea.
> 
> I've applied your two patches to the topic/pipe branch.  I also rebased it and 
> did a forced push in order to bring in Corinna's loader script fix.  So you'll 
> have to do 'git fetch' and 'git rebase --hard origin/topic/pipe'.
> 
> Does this now fix all known problems with pipes?

Only the small thing remaining is pipe mode. If the pipe mode
is changed to byte mode, the following issue will be solved.
https://cygwin.com/pipermail/cygwin/2021-March/247987.html

How about the simple patch attached?

The comment in pipe code says:
     Note that the write side of the pipe is opened as PIPE_TYPE_MESSAGE.
     This *seems* to more closely mimic Linux pipe behavior and is
     definitely required for pty handling since fhandler_pty_master
     writes to the pipe in chunks, terminated by newline when CANON mode
     is specified.

This mentions about pty behaiviour in canonical mode, however, the
pty pipe is created as message mode even with this patch. Are there
any other reasons that message mode is preferred for pipe?

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

[-- Attachment #2: 0003-Cygwin-pipe-Set-byte-mode-as-default-rather-than-mes.patch --]
[-- Type: application/octet-stream, Size: 1091 bytes --]

From ec2d43f25f7cf4fc3b01bd2e15e3b48e3c9461d1 Mon Sep 17 00:00:00 2001
From: Takashi Yano <takashi.yano@nifty.ne.jp>
Date: Mon, 30 Aug 2021 04:12:56 +0900
Subject: [PATCH 3/3] Cygwin: pipe: Set byte mode as default rather than
 message mode.

Currently, the pipe mode is message by default. This causes a issue
with C# (.NET) programs if they are piped. With this patch, default
pipe mode is changed to byte mode. The pipe mode can be revert to
message mode by setting CYGWIN=nopipe_byte.

Addresses: https://cygwin.com/pipermail/cygwin/2021-March/247987.html
---
 winsup/cygwin/globals.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index 48fb312de..3534381ec 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -69,7 +69,7 @@ int NO_COPY dynamically_loaded;
 /* Some CYGWIN environment variable variables. */
 bool allow_glob = true;
 bool ignore_case_with_glob;
-bool pipe_byte;
+bool pipe_byte = true;
 bool reset_com;
 bool wincmdln;
 winsym_t allow_winsymlinks = WSYM_default;
-- 
2.33.0