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

[Ken Brown <kbrown@cornell.edu>]

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

On 9/6/2021 8:49 AM, Corinna Vinschen wrote:
> - I think setting chunk to DEFAULT_PIPEBUFSIZE - 1 in the read case and
>    DEFAULT_PIPEBUFSIZE in the write case by default is dangerous.
>    Assuming the pipe has been created by a non-Cygwin process, the values
>    may be way too high.
> 
>    Suggestion: Actually set max_atomic_write to something useful.
>    Set max_atomic_write to DEFAULT_PIPEBUFSIZE in fhandler_pipe::create.
>    In case of stdio handles inherited from non-Cygwin processes, fetch
>    the pipe buffer size via NtQueryInformationFile in
>    dtable::init_std_file_from_handle().  Better, in a matching
>    fhandler_pipe method called from init_std_file_from_handle().

How about something like the attached (untested)?

> - What about calling select for writing on pipes read by non-Cygwin
>    processes?  In that case, we still can't rely on WriteQuotaAvailable,
>    just as before.
> 
>    I have a vague idea that we might want to count readers in that case,
>    but I have to think about it some more.

Even if we count readers, we have no way of knowing whether a pending read has 
reduced WriteQuotaAvailable to 0.  Maybe this is a case where we should impose 
some artificial timeout, after which we report write ready.  Falsely reporting 
write ready in this corner case seems better than risking a deadlock.

Ken

[-- Attachment #2: pipe_buf.diff --]
[-- Type: text/plain, Size: 3225 bytes --]

diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 8085e656e..a638a5995 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -406,6 +406,11 @@ dtable::init_std_file_from_handle (int fd, HANDLE handle)
 	}
       if (!fh->init (handle, access, bin))
 	api_fatal ("couldn't initialize fd %d for %s", fd, fh->get_name ());
+      if (fh->ispipe ())
+	{
+	  fhandler_pipe *fhp = (fhandler_pipe *) fh;
+	  fhp->set_pipe_buf_size ();
+	}
 
       fh->open_setup (openflags);
       fh->usecount = 0;
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index bb7eb09ce..7aed089eb 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1172,7 +1172,7 @@ class fhandler_socket_unix : public fhandler_socket
 class fhandler_pipe_fifo: public fhandler_base
 {
  protected:
-  size_t max_atomic_write;
+  size_t pipe_buf_size;
 
  public:
   fhandler_pipe_fifo ();
@@ -1192,6 +1192,7 @@ public:
 
   bool ispipe() const { return true; }
   void set_read_mutex (HANDLE mtx) { read_mtx = mtx; }
+  void set_pipe_buf_size ();
 
   void set_popen_pid (pid_t pid) {popen_pid = pid;}
   pid_t get_popen_pid () const {return popen_pid;}
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index 4bd807a09..c75476040 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -29,7 +29,7 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
 		   || _s == STATUS_PIPE_EMPTY; })
 
 fhandler_pipe_fifo::fhandler_pipe_fifo ()
-  : fhandler_base (), max_atomic_write (DEFAULT_PIPEBUFSIZE)
+  : fhandler_base (), pipe_buf_size (DEFAULT_PIPEBUFSIZE)
 {
 }
 
@@ -269,7 +269,7 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
 	     buffer size - 1. Pending read lowers WriteQuotaAvailable on
 	     the write side and thus affects select's ability to return
 	     more or less reliable info whether a write succeeds or not. */
-	  ULONG chunk = max_atomic_write - 1;
+	  ULONG chunk = pipe_buf_size - 1;
 	  status = NtQueryInformationFile (get_handle (), &io,
 					   &fpli, sizeof (fpli),
 					   FilePipeLocalInformation);
@@ -391,12 +391,12 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
   if (!len)
     return 0;
 
-  if (len <= max_atomic_write)
+  if (len <= pipe_buf_size)
     chunk = len;
   else if (is_nonblocking ())
-    chunk = len = max_atomic_write;
+    chunk = len = pipe_buf_size;
   else
-    chunk = max_atomic_write;
+    chunk = pipe_buf_size;
 
   /* Create a wait event if the pipe or fifo is in blocking mode. */
   if (!is_nonblocking () && !(evt = CreateEvent (NULL, false, false, NULL)))
@@ -894,6 +894,21 @@ nt_create (LPSECURITY_ATTRIBUTES sa_ptr, HANDLE &r, HANDLE &w,
   return 0;
 }
 
+/* Called by dtable::init_std_file_from_handle for stdio handles
+   inherited from non-Cygwin processes. */
+void
+fhandler_pipe::set_pipe_buf_size ()
+{
+  NTSTATUS status;
+  IO_STATUS_BLOCK io;
+  FILE_PIPE_LOCAL_INFORMATION fpli;
+
+  status = NtQueryInformationFile (get_handle (), &io, &fpli, sizeof fpli,
+				   FilePipeLocalInformation);
+  if (NT_SUCCESS (status))
+    pipe_buf_size = fpli.InboundQuota;
+}
+
 int
 fhandler_pipe::ioctl (unsigned int cmd, void *p)
 {