From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from conssluserg-04.nifty.com (conssluserg-04.nifty.com [210.131.2.83]) by sourceware.org (Postfix) with ESMTPS id EBB67385842C for ; Mon, 13 Sep 2021 18:55:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EBB67385842C Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=nifty.ne.jp Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=nifty.ne.jp Received: from Express5800-S70 (z221123.dynamic.ppp.asahi-net.or.jp [110.4.221.123]) (authenticated) by conssluserg-04.nifty.com with ESMTP id 18DIsh4q019605 for ; Tue, 14 Sep 2021 03:54:43 +0900 DKIM-Filter: OpenDKIM Filter v2.10.3 conssluserg-04.nifty.com 18DIsh4q019605 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nifty.ne.jp; s=dec2015msa; t=1631559283; bh=JQ2vZfNmGwwuhcX9fb7uRrUrMSpRBfT5PpOHjpxASt4=; h=Date:From:To:Subject:In-Reply-To:References:From; b=lweM7aY5Ux4HvEHwwMAmVo6U7Sx0vsLSi3a00jOyXjFpuOV0DxNYyzf0/IWQQZ9RI /gpMoUVn3aT5oYgitJjllZmHLTCMEHn2WPpEbV5zUoqx1Cn7xZIxHgVz5S6cSjTIr2 9EQg5rX0/04UmAlYY1CJFSoXQOny/gl85nKL03I4MPQtYB0QRQOqCElusmG/TVKCIK kEV8BtYLEwwxv2FSfzy1sEW76l8tM0jiQJHF5IFCRn7fNYqtZw3crm0uW45ei9IAas 3ppTwDZzi2xF4sY9621gfDXoMVmMxqEsD2M0qjrTv3b6voYDrkIcCFO0yeqncfUVqs sYWF/PoVtMSaQ== X-Nifty-SrcIP: [110.4.221.123] Date: Tue, 14 Sep 2021 03:54:48 +0900 From: Takashi Yano To: cygwin-developers@cygwin.com Subject: Re: cygrunsrv + sshd + rsync = 20 times too slow -- throttled? Message-Id: <20210914035448.33737a2e7ea1d4eb8351d9ec@nifty.ne.jp> In-Reply-To: References: <41A583E1-C8E7-42AB-9F24-EEC33A41EC60@house.org> <3b560051-ab27-f392-ca4b-d1fd9b5733b0@cornell.edu> <20210827202440.47706fc2fc07c5e9a1bc0047@nifty.ne.jp> <20210907122631.65452be8d021ec72259431d5@nifty.ne.jp> <20210909124115.555c6be15d675500617d284a@nifty.ne.jp> <20210909170549.506cc3c1f6029d904fece6dd@nifty.ne.jp> <20210909211940.51ef391e27d43f0421962cb8@nifty.ne.jp> <20210909214246.cd1ff1a3062fea27e51ad4ae@nifty.ne.jp> <33386baf-3b2d-d57f-2ad3-1bd328ed7935@cornell.edu> <20210911075734.aaf37697ba7db2ad14d911a3@nifty.ne.jp> <20210911113517.f74fc3ac1971bbf04c7a9bd1@nifty.ne.jp> <695ce1f4-4f7d-f3f3-6dd3-087467d67b28@cornell.edu> <20210912174849.3d38107568065a95aeb19c7c@nifty.ne.jp> <20210912200423.667e40eb1adc52461bbefa20@nifty.ne.jp> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.30; i686-pc-mingw32) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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, 13 Sep 2021 18:55:14 -0000 On Mon, 13 Sep 2021 13:42:46 -0400 Ken Brown wrote: > > diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h > > index 13fba9a14..f09af2c37 100644 > > --- a/winsup/cygwin/fhandler.h > > +++ b/winsup/cygwin/fhandler.h > > @@ -1176,10 +1176,15 @@ class fhandler_pipe_fifo: public fhandler_base > > { > > protected: > > size_t pipe_buf_size; > > + HANDLE query_hdl; > > > > public: > > fhandler_pipe_fifo (); > > > > + HANDLE get_query_handle () const { return query_hdl; } > > + void close_query_handle () { CloseHandle (query_hdl); query_hdl = NULL; } > > Should you use if(query_hdl) here? Or is it up to the caller to check that? Right. I will fix it. > > @@ -522,6 +532,10 @@ fhandler_pipe::fixup_after_fork (HANDLE parent) > > fork_fixup (parent, read_mtx, "read_mtx"); > > if (select_sem) > > fork_fixup (parent, select_sem, "select_sem"); > > + /* Do not duplicate query_hdl if it has been already inherited. */ > > + if (query_hdl && !get_obj_handle_count (query_hdl)) > > + fork_fixup (parent, query_hdl, "query_hdl"); > > > Why do you need to call get_obj_handle_count here? Shouldn't fork_fixup take > care of the case where the handle has been inherited? I also thought so, however, counting query_hdl did not work as expected without this check. I am not sure why... There seems to be the case that handle is already inherited here without fork_fixup. > > @@ -608,12 +608,43 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, bool writing) > > } > > if (writing) > > { > > - /* WriteQuotaAvailable is decremented by the number of bytes requested > > - by a blocking reader on the other side of the pipe. Cygwin readers > > - are serialized and never request a number of bytes equivalent to the > > - full buffer size. So WriteQuotaAvailable is 0 only if either the > > - read buffer on the other side is really full, or if we have non-Cygwin > > - readers. */ > > + /* If there is anything available in the pipe buffer then signal > > + that. This means that a pipe could still block since you could > > + be trying to write more to the pipe than is available in the > > + buffer but that is the hazard of select(). > > + > > + Note that WriteQuotaAvailable is unreliable. > > + > > + Usually WriteQuotaAvailable on the write side reflects the space > > + available in the inbound buffer on the read side. However, if a > > + pipe read is currently pending, WriteQuotaAvailable on the write side > > + is decremented by the number of bytes the read side is requesting. > > + So it's possible (even likely) that WriteQuotaAvailable is 0, even > > + if the inbound buffer on the read side is not full. This can lead to > > + a deadlock situation: The reader is waiting for data, but select > > + on the writer side assumes that no space is available in the read > > + side inbound buffer. > > + > > + Consequentially, the only reliable information is available on the > > + read side, so fetch info from the read side via the pipe-specific > > + query handle. Use fpli.WriteQuotaAvailable as storage for the actual > > + interesting value, which is the OutboundQuote on the write side, > > I thought Corinna's experiments showed that InboundQuota and OutboundQuota are > the same on the read and write sides, and that InboundQuota is the one we should > be using. I have confirmed that behaviour. You are right. Using InboundQuota is right thing. I'll fix it. Thanks. -- Takashi Yano