From owner-freebsd-arch@FreeBSD.ORG Tue Aug 12 23:36:24 2014 Return-Path: Delivered-To: freebsd-arch@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 347CF86B; Tue, 12 Aug 2014 23:36:24 +0000 (UTC) Received: from mail-wg0-x22e.google.com (mail-wg0-x22e.google.com [IPv6:2a00:1450:400c:c00::22e]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9A6E526BB; Tue, 12 Aug 2014 23:36:23 +0000 (UTC) Received: by mail-wg0-f46.google.com with SMTP id m15so10551940wgh.29 for ; Tue, 12 Aug 2014 16:36:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=Rqg7qTNYQktDwbS7h++60UQQhzncoH+D9BkJ3RUGK7g=; b=R9GWX5Qjpl2JOoyYbmYDpgajq3FlKkokHaiC1VTEmeH5l4OrAGX5SmeMAnXgm6c61t 7fKzK9DpGz8IDQeqzgh48vxNmhGrra1XV43AFSJACWbaZbqNSCMyni1Z+k4bD3g0cW/s HNaQdW96HCJOWxusKdrhBqdr1oogHTcDXXLx0toHWp/6mVPiQtcJBhBc1nATIQZC5eIk noim19NP6r2vT0LUhne7ySq2o5tVVeb1eLBAS+2L3xIsMHvnHFs6dbGIShDkdt8+ubLD 6f4F1wJH55cLFCjc8WLAcN++uUbuQexoIr6VeOPgZ+NwFm4c133N9T/zBA2WAUZmjFuR 8jzA== X-Received: by 10.194.110.7 with SMTP id hw7mr863796wjb.38.1407886581762; Tue, 12 Aug 2014 16:36:21 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id ex2sm395918wjd.30.2014.08.12.16.36.20 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 12 Aug 2014 16:36:20 -0700 (PDT) Date: Wed, 13 Aug 2014 01:36:18 +0200 From: Mateusz Guzik To: John Baldwin Subject: Re: current fd allocation idiom Message-ID: <20140812233617.GA17869@dft-labs.eu> References: <20140717235538.GA15714@dft-labs.eu> <20140718155959.GN93733@kib.kiev.ua> <20140718191928.GB7179@dft-labs.eu> <201408111124.52064.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <201408111124.52064.jhb@freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Konstantin Belousov , freebsd-arch@freebsd.org X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Aug 2014 23:36:24 -0000 On Mon, Aug 11, 2014 at 11:24:51AM -0400, John Baldwin wrote: > On Friday, July 18, 2014 3:19:28 pm Mateusz Guzik wrote: > > On Fri, Jul 18, 2014 at 06:59:59PM +0300, Konstantin Belousov wrote: > > > On Fri, Jul 18, 2014 at 04:40:12PM +0200, Mateusz Guzik wrote: > > > > On Fri, Jul 18, 2014 at 04:06:29PM +0300, Konstantin Belousov wrote: > > > > > It seems that all what is needed is conversion of places using > > > > > falloc() to falloc_noinstall()/finstall(). > > > > > > > > > > > > > This postpones fd allocation to after interested function did all work > > > > it wanted to do, which means we would need reliable ways of reverting > > > > all the work in case allocation failed. I'm not so confident we can do > > > > that for all current consumers and both current and my proposed approach > > > > don't impose such requirement. > > > Cleanup should be identical to the actions done on close(2). > > > > > > > > > > > Of course postponing fd allocation where possible is definitely worth > > > > doing. > > > Yes, and after that the rest of the cases should be evaluated. > > > But my gut feeling is that everything would be converted. > > > > > > > So let's say you accept() a connection. > > > > With current code, if you got to accept the connection you got it. > > > > With your proposal you may find that you can't allocate any fd and have > > to close fp. This will be visible as accept + close by the other end, > > while the caller never saw the connection. > > > > My guess is people would complain once they encounter such issue. > > Can't you already get this if you overflow the listen queue? (Having > "accepted" connections aborted where the user application doesn't know): > > } else { > /* > * Keep removing sockets from the head until there's room for > * us to insert on the tail. In pre-locking revisions, this > * was a simple if(), but as we could be racing with other > * threads and soabort() requires dropping locks, we must > * loop waiting for the condition to be true. > */ > while (head->so_incqlen > head->so_qlimit) { > struct socket *sp; > sp = TAILQ_FIRST(&head->so_incomp); > TAILQ_REMOVE(&head->so_incomp, sp, so_list); > head->so_incqlen--; > sp->so_qstate &= ~SQ_INCOMP; > sp->so_head = NULL; > ACCEPT_UNLOCK(); > soabort(sp); > ACCEPT_LOCK(); > } > TAILQ_INSERT_TAIL(&head->so_incomp, so, so_list); > so->so_qstate |= SQ_INCOMP; > head->so_incqlen++; > > I think the simplest approach would be to first convert as many places as > possible to use falloc_noinstall() / finstall(). If you end up with all of > them converted then you can just rename falloc_noinstall to falloc() and > retire the old falloc(). > I would expect soabort to result in a timeout/reset as opposed to regular connection close. Comments around soabort suggest it should not be used as a replacement for close, but maybe this is largely because of what the other end will see. That will need to be investigated. That said, I definitely support using delayed fd allocation (current falloc_noinstall) where possible, but I'm not convinced it is safe for all consumers. -- Mateusz Guzik