From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from conssluserg-02.nifty.com (conssluserg-02.nifty.com [210.131.2.81]) by sourceware.org (Postfix) with ESMTPS id 520CC385840D for ; Sat, 4 Sep 2021 12:03:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 520CC385840D 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-02.nifty.com with ESMTP id 184C2uJu019258 for ; Sat, 4 Sep 2021 21:02:56 +0900 DKIM-Filter: OpenDKIM Filter v2.10.3 conssluserg-02.nifty.com 184C2uJu019258 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nifty.ne.jp; s=dec2015msa; t=1630756976; bh=UxdKUMdi9BQqYXJXvnT0E/8K5Ys86EynRUSE8b2Vr0Y=; h=Date:From:To:Subject:In-Reply-To:References:From; b=2R9JNj4JdRzXQ9/rC9RLgKggkQ2lEmm0r4JYGAVPLpFwOGEEnXVHchOync1MGzV7m F9sjBHVmxYvBobZhRW/in7eA7gq8ZRDU6oi88Wg2nJERbhUNYruiMk31a+B7UU0tQz e48BWijG33SRb88nQ6l095wmZzWALYgBqfn5u8uFtrFvJMNylnZEc1QiIkf+4xxebU bUBh5uqr25L6Q/VkKlbZ6nJZ+FjOjrjCkuI+2GOSvDewtDkVk8tWtb2+ewEHUelaEN niAr5arr/0+fRAypaLsFmezpMz6BnY23ssLDvXQImG+b2afb3kb74YpTje/IdEh8aN YibTRWGr6wM0g== X-Nifty-SrcIP: [110.4.221.123] Date: Sat, 4 Sep 2021 21:02:58 +0900 From: Takashi Yano To: cygwin-developers@cygwin.com Subject: Re: cygrunsrv + sshd + rsync = 20 times too slow -- throttled? Message-Id: <20210904210258.342eb795ac53f1d5352ea512@nifty.ne.jp> In-Reply-To: References: <9ba687eb-f4a0-18f8-b10b-76e7e51e123e@cornell.edu> <152bfc0c-2f72-c684-6fc5-aa7c36c136b8@cornell.edu> <20210903190046.663c60fb11c936e344821383@nifty.ne.jp> <20210903191340.c28ae366e79ca14799bacc1f@nifty.ne.jp> <20210903212205.acc2fc68cc4ffce9c1db3dd9@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.8 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: Sat, 04 Sep 2021 12:03:18 -0000 Hi Corinna, Ken, On Fri, 3 Sep 2021 09:27:37 -0400 Ken Brown wrote: > On 9/3/2021 8:22 AM, Takashi Yano wrote: > > POSIX says: > > The value returned may be less than nbyte if the number of bytes left > > in the file is less than nbyte, if the read() request was interrupted > > by a signal, or if the file is a pipe or FIFO or special file and has > > ~~~ > > fewer than nbyte bytes immediately available for reading. > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > https://pubs.opengroup.org/onlinepubs/009604599/functions/read.html > > > > If it is turned over, read() should read all data immediately available, > > I think. > > I understand the reasoning now, but I think your patch isn't quite right. As it > stands, if the call to NtQueryInformationFile fails but total_length != 0, > you're trying to read again without knowing that there's data in the pipe. > > Also, I think you need the following: > > diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc > index ef7823ae5..46bb96961 100644 > --- a/winsup/cygwin/fhandler_pipe.cc > +++ b/winsup/cygwin/fhandler_pipe.cc > @@ -348,8 +348,13 @@ fhandler_pipe::raw_read (void *ptr, size_t& len) > CloseHandle (evt); > if (status == STATUS_THREAD_SIGNALED) > { > - set_errno (EINTR); > - len = (size_t) -1; > + if (total_len == 0) > + { > + set_errno (EINTR); > + len = (size_t) -1; > + } > + else > + len = total_len; > } > else if (status == STATUS_THREAD_CANCELED) > pthread::static_cancel_self (); Thanks for your advice. I fixed the issue and attached new patch. On Fri, 3 Sep 2021 17:37:13 +0200 Corinna Vinschen wrote: > Hmm, I see the point, but we might have another problem with that. > > We can't keep the mutex while waiting on the pending read, and there > could be more than one pending read running at the time. if so, > chances are extremly high, that the data written to the buffer gets > split like this: > > reader 1 reader 2 > > calls read(65536) calls read(65536) > > calls NtReadFile(16384 bytes) > calls NtReadFile(16384 bytes) > > writer writes 65536 bytes > > wakes up and gets 16384 bytes > wakes up and gets 16384 bytes > gets the mutex, calls > NtReadFile(32768) which > returns immediately with > 32768 bytes added to the > caller's buffer. > > so the buffer returned to reader 1 is 49152 bytes, with 16384 bytes > missing in the middle of it, *without* the reader knowing about that > fact. If reader 1 gets the first 16384 bytes, the 16384 bytes have > been read in a single call, at least, so the byte order is not > unknowingly broken on the application level. > > Does that make sense? Why can't we keep the mutex while waiting on the pending read? If we can keep the mutex, the issue above mentioned does not happen, right? What about the patch attached? This keeps the mutex while read() but I do not see any defects so far. -- Takashi Yano