From owner-freebsd-net@FreeBSD.ORG Mon Oct 6 16:38:09 2014 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 831BC5A7; Mon, 6 Oct 2014 16:38:09 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 595386CD; Mon, 6 Oct 2014 16:38:09 +0000 (UTC) Received: from ralph.baldwin.cx (pool-173-70-85-31.nwrknj.fios.verizon.net [173.70.85.31]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id C3D23B949; Mon, 6 Oct 2014 12:38:07 -0400 (EDT) From: John Baldwin To: "Robert N. M. Watson" Subject: Re: [PATCH] Only lock send buffer in sopoll() if needed Date: Mon, 06 Oct 2014 12:37:42 -0400 Message-ID: <1527564.dunJKn8zWW@ralph.baldwin.cx> User-Agent: KMail/4.12.5 (FreeBSD/10.1-BETA2; KDE/4.12.5; amd64; ; ) In-Reply-To: <9DED9207-89B7-47C3-B823-047A5FDFC511@FreeBSD.org> References: <201409301400.09999.jhb@freebsd.org> <9DED9207-89B7-47C3-B823-047A5FDFC511@FreeBSD.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 06 Oct 2014 12:38:07 -0400 (EDT) Cc: net@freebsd.org X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Oct 2014 16:38:09 -0000 On Sunday, October 05, 2014 08:08:17 AM Robert N. M. Watson wrote: > I'm not convinced that the race with SBS_CANTSENDMORE is OK, even though I > really want to write that it is. The risk here is that we miss an > asynchronous disconnect event, and that the thread then blocks even though > an event is pending, which is a nasty turn of events. We might want to dig > about a bit and see whether this case 'matters' -- e.g., that there are > important cases where a test for writability might need to catch > SBS_CANTRCVMORE but SBS_CANTSENDMORE is not simultaneously set. I thought about this a bit more and this would be ok so long as the code that does a selwakeup() on so_rcv does so after setting SBS_CANTSENDMORE. However, checking both soisdisconnecting() and soisdisconnected() shows that they call sorwakeup() and unlock the sb_rcv lock before setting the flag, so the race is not ok. > Could you say a bit more about the performance benefits of this change -- is > it substantial? If so, we might need to add a new socket-buffer flag along > the lines of SB[RS]_DISCONNECTED that is 'broadcast' to both socket buffers > on certain events. Doing so might require rejiggering elsewhere by causing > additional locks to need to be held, possibly out of order. I had posted this to an older thread on current@ where Luigi asked about the overhead of locking both for sopoll(). I never got a reply from Luigi (and no one else responded on that thread). I found this again recently in an old tree and was curious what other folks thought of the idea. I do not have any workloads I am working with where this is a factor. -- John Baldwin