From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.126.134]) by sourceware.org (Postfix) with ESMTPS id 9652F3858414 for ; Mon, 6 Sep 2021 12:49:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9652F3858414 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=cygwin.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=cygwin.com Received: from calimero.vinschen.de ([24.134.7.25]) by mrelayeu.kundenserver.de (mreue009 [212.227.15.167]) with ESMTPSA (Nemesis) id 1Myb8P-1nBD3Z18Np-00yxvP for ; Mon, 06 Sep 2021 14:49:56 +0200 Received: by calimero.vinschen.de (Postfix, from userid 500) id BA6B2A80DAA; Mon, 6 Sep 2021 14:49:55 +0200 (CEST) Date: Mon, 6 Sep 2021 14:49:55 +0200 From: Corinna Vinschen To: cygwin-developers@cygwin.com Subject: Re: cygrunsrv + sshd + rsync = 20 times too slow -- throttled? Message-ID: Reply-To: cygwin-developers@cygwin.com Mail-Followup-To: cygwin-developers@cygwin.com References: <20210904210258.342eb795ac53f1d5352ea512@nifty.ne.jp> <20210904213713.8760e7ba3a4d68fbb78d677e@nifty.ne.jp> <51cb0cef-c3fd-1320-c2dd-a868bf1ffaae@cornell.edu> <20210905081523.0db04d9402abf87635066eb7@nifty.ne.jp> <20210905224059.cfdc8f23d3eeaa1ea16ecf2e@nifty.ne.jp> <20210905225037.c625ee0bcd479181848763f8@nifty.ne.jp> <20210906050950.56b397be7c5eb3da848691e9@nifty.ne.jp> <20210906201643.2e84c0d3a7ac7c8878548408@nifty.ne.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210906201643.2e84c0d3a7ac7c8878548408@nifty.ne.jp> X-Provags-ID: V03:K1:eYbZXJJ2r/pu5xHayXsoNIlZjxoQQq5hMvDIuBCeZiSu/dF/IHB K/5ty1UM680pg1aLCB0g3KPKfYg1voAEV4vmyF0TCmCKE5CnBD7HKti6App8xDSmOirw8xR sAlwciKuagBG/8KFzQc0gCzLgbLxJfn0q+Sm/hZt3UZtO5y+Fnh+lGwyPaOQ5ygbv9YF7Ee KrgBgE+JcrQB/uuGAv8jw== X-UI-Out-Filterresults: notjunk:1;V03:K0:MYZxz0aiwdA=:I38ofO25mKHMVy3V124dcW jgbBS+IReBVRemo2Nz3C+ltyVFwOHm2H6IMW5YFoHnVoIoQoXA3sB80z/WtKecZtrgpTSt+5b XPuyE/hqnVU/AC8nF1cNlgjVrcqFFB9kzYQO9u+lXpv2n719QpVwa8yXF/WT4d8vwSaF3ADGj pcUvyP0oNUA/8iSFVnF6LDZz0roevUDAlVDzm8Iu/RqiINqGv1xLfUgpHk6cjWO8JeHZ/ZpBN o6vQXhMOFanNF9GA2iXGelZe4LkWhJZEm8OgVGFHoSSM4QqMqZvgQ/iSvzyps35wosGAl4JsA IOYth0ABizhC+sZzmVTLacpDIHQrfgbxmMPqDOXvshoBw6vggCxP4ISm1KNK3Di8awzLxKueq 0tctgg5QUYJrQNM7JLnoqpyw9lS8zON08OjM6sFLJbVgt0zYXEfoRxnMf2z3WmPAREv3jIicO A94FXU4cP02W5+NtCVQOIqpmHaDIgzclHas+fCq76HV5bhuVSUcgPnRIbF/WAdodEym/wZwSE lBSnYwYUQrn4vIe0wBev+EY6CnL+JHODLxyGfSBitfKzthdih8SdgJ7SOOPPVtoFSAPr8K3/f owlrqckVklGIuYj0ehfUdeYsAeMxzdTkTsQdD18GjN3RJ41zi0ngFVKmAXqN+NX2LPhZosklR 3NEn4p6bWeYj8BJM/oaSCJQBmIQsLgQ4sZi7zTLCDeM8nsgKiBC/T9jSIW4NsnoFbOR2F3PP9 pCtQH5rlEwV4fHsPzou4vm+bfZ81o/O1bD94I4kIFepKN8TRO8qqq8DwO24= X-Spam-Status: No, score=-99.7 required=5.0 tests=BAYES_00, GOOD_FROM_CORINNA_CYGWIN, JMQ_SPF_NEUTRAL, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: cygwin-developers@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Cygwin core component developers mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Sep 2021 12:49:59 -0000 On Sep 6 20:16, Takashi Yano wrote: > Hi Corinna, > > On Mon, 6 Sep 2021 10:13:10 +0200 > Corinna Vinschen wrote: > > If you have multiple readers, all but one of them will hang in this > > WFSO. They will block here without a chance to kill or Ctrl-C them > > and thread cancellation won't work. > > > > To fix that you have to use cygwait and handle signals and thread > > cancellation the same way as in the below code following the NtReadFile. > > OK. Thanks for the advice. Then what about the patch attached? > > > > /* If the pipe is empty, don't request more bytes than half the > > > - pipe buffer size. Every 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. > > > - > > > - Let the size of the request depend on the number of readers > > > - at the time. */ > > > + pipe buffer size. 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 / 2; > > > status = NtQueryInformationFile (get_handle (), &io, > > > &fpli, sizeof (fpli), > > > FilePipeLocalInformation); > > > - if (NT_SUCCESS (status) && fpli.ReadDataAvailable == 0) > > > > If the readers are serialized anyway, why fetch only half the remaining > > buffer size? In that case fetching fpli.InboundQuota - 1 is as good > > as fetching just the half of it, isn't it? > > It sounds reasonable. Adopted in the attached patch. Patch looks ok. I added one more patches: - It occured to me that the code is lacking a CancelIo in case cygwait is waiting for NtReadFile/NtWriteFile. Actually, calling cygwait without "mask" parameter will result in cygwait performing the thread cancellation by itself, but cancelling a thread does not cancel the async IO started by that thread. So I fixed the cygwait calls in raw_read/raw_write to return to the caller and then call CancelIo before cancelling the thread. I planned to push one more patch: - Drop max_atomic_write, it's == DEFAULT_PIPEBUFSIZE anyway But then some things were coming to mind, which we still have to discuss. - 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(). - 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. Corinna