Date: Mon, 11 Aug 2014 11:24:51 -0400 From: John Baldwin <jhb@freebsd.org> To: freebsd-arch@freebsd.org Cc: Konstantin Belousov <kostikbel@gmail.com>, Mateusz Guzik <mjguzik@gmail.com> Subject: Re: current fd allocation idiom Message-ID: <201408111124.52064.jhb@freebsd.org> In-Reply-To: <20140718191928.GB7179@dft-labs.eu> References: <20140717235538.GA15714@dft-labs.eu> <20140718155959.GN93733@kib.kiev.ua> <20140718191928.GB7179@dft-labs.eu>
next in thread | previous in thread | raw e-mail | index | archive | help
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().
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201408111124.52064.jhb>