From owner-freebsd-current@FreeBSD.ORG Wed Jan 22 20:29:51 2014 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 197997D2; Wed, 22 Jan 2014 20:29:51 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id CA6191FEC; Wed, 22 Jan 2014 20:29:50 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 9DD3EB964; Wed, 22 Jan 2014 15:29:49 -0500 (EST) From: John Baldwin To: freebsd-current@freebsd.org Subject: Re: possible selrecord optimization ? Date: Wed, 22 Jan 2014 14:29:56 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.4-CBSD-20130906; KDE/4.5.5; amd64; ; ) References: In-Reply-To: X-KMail-Markup: true MIME-Version: 1.0 Message-Id: <201401221429.56745.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Wed, 22 Jan 2014 15:29:49 -0500 (EST) Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.17 Cc: Luigi Rizzo , FreeBSD Current X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Jan 2014 20:29:51 -0000 On Tuesday, January 21, 2014 9:25:27 pm Luigi Rizzo wrote: > Looking at how selrecord() / selwakeup() and their Linux counterparts > poll_wait() and wake_up() are used, i noticed the following: > > - linux tends to call wake_up() unconditionally > at the beginning of the poll handler > > - FreeBSD tends to call selrecord() only when it detects a blocking > situation, and this also requires something (a lock or a retry; > the lock in selinfo is not good for this) to avoid the race between > the blocking_test..selrecord pair and the selwakeup(). > > FreeBSD could call selrecord unconditionally (and save the extra > lock/retry), but selrecord is expensive as it queues the thread on > the struct selinfo, and this takes a lock. > > I wonder if we could use the same optimization as Linux: > as soon as pollscan/selscan detects a non-blocking fd, > make selrecord a no-op (which is probably as simple > as setting SELTD_RESCAN; and since it only goes up > we do not need to lock to check it). Yes, I think this would work fine. I think setting SELTD_RESCAN as a way to do it is fine as well. > This way, we would pay at most for one extra selrecord per > poll/select. > > Even more interesting, it could simplify the logic and locking > in poll handlers. > > As an example, in sys/uipc_socket.c :: sopoll_generic() > we could completely decouple the locks on so_snd and so_rcv. Splitting the locks up is not really feasible because the so_rcv lock doubles as SOCK_LOCK() and is needed even in the POLLOUT case as sowritable() needs it for so_state. What you might be able to do instead is be a bit fancier and only lock so_snd if writing is being polled for by making it conditional on the values in events. This would avoid locking so_snd when just polling for read: Index: uipc_socket.c =================================================================== --- uipc_socket.c (revision 261029) +++ uipc_socket.c (working copy) @@ -2912,7 +2912,12 @@ sopoll_generic(struct socket *so, int events, stru { int revents = 0; - SOCKBUF_LOCK(&so->so_snd); + if (events & (POLLOUT | POLLWRNORM)) + SOCKBUF_LOCK(&so->so_snd); + /* + * The so_rcv lock doubles as SOCK_LOCK so it it is needed for + * all requests. + */ SOCKBUF_LOCK(&so->so_rcv); if (events & (POLLIN | POLLRDNORM)) if (soreadabledata(so)) @@ -2947,7 +2952,8 @@ sopoll_generic(struct socket *so, int events, stru } SOCKBUF_UNLOCK(&so->so_rcv); - SOCKBUF_UNLOCK(&so->so_snd); + if (events & (POLLOUT | POLLWRNORM)) + SOCKBUF_UNLOCK(&so->so_snd); return (revents); } -- John Baldwin