Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 12 Jun 2012 12:47:49 +0200
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        davidxu@FreeBSD.org
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Pawel Jakub Dawidek <pjd@FreeBSD.org>
Subject:   Re: svn commit: r236935 - head/sys/kern
Message-ID:  <20120612104749.GB20749@dft-labs.eu>
In-Reply-To: <4FD6FD39.5090800@gmail.com>
References:  <201206112205.q5BM5QIv013266@svn.freebsd.org> <4FD6FD39.5090800@gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jun 12, 2012 at 04:26:33PM +0800, David Xu wrote:
> On 2012/6/12 6:05, Pawel Jakub Dawidek wrote:
> >Author: pjd
> >Date: Mon Jun 11 22:05:26 2012
> >New Revision: 236935
> >URL: http://svn.freebsd.org/changeset/base/236935
> >
> >Log:
> >   fdgrowtable() no longer drops the filedesc lock so it is enough to
> >   retry finding free file descriptor only once after fdgrowtable().
> >
> >   Spotted by:	pluknet
> >   MFC after:	1 month
> >
> >Modified:
> >   head/sys/kern/kern_descrip.c
> >
> >Modified: head/sys/kern/kern_descrip.c
> >==============================================================================
> >--- head/sys/kern/kern_descrip.c	Mon Jun 11 21:56:37 2012	(r236934)
> >+++ head/sys/kern/kern_descrip.c	Mon Jun 11 22:05:26 2012	(r236935)
> >@@ -1478,29 +1478,33 @@ fdalloc(struct thread *td, int minfd, in
> >  	/*
> >  	 * Search the bitmap for a free descriptor.  If none is found, try
> >  	 * to grow the file table.  Keep at it until we either get a file
> >-	 * descriptor or run into process or system limits; fdgrowtable()
> >-	 * may drop the filedesc lock, so we're in a race.
> >+	 * descriptor or run into process or system limits.
> >  	 */
> >-	for (;;) {
> >-		fd = fd_first_free(fdp, minfd, fdp->fd_nfiles);
> >-		if (fd>= maxfd)
> >-			return (EMFILE);
> >-		if (fd<  fdp->fd_nfiles)
> >-			break;
> >+	fd = fd_first_free(fdp, minfd, fdp->fd_nfiles);
> >+	if (fd>= maxfd)
> >+		return (EMFILE);
> >+	if (fd>= fdp->fd_nfiles) {
> >  #ifdef RACCT
> >  		PROC_LOCK(p);
> >-		error = racct_set(p, RACCT_NOFILE, min(fdp->fd_nfiles * 2, maxfd));
> >+		error = racct_set(p, RACCT_NOFILE,
> >+		    min(fdp->fd_nfiles * 2, maxfd));
> >  		PROC_UNLOCK(p);
> >  		if (error != 0)
> >  			return (EMFILE);
> >  #endif
> >  		fdgrowtable(fdp, min(fdp->fd_nfiles * 2, maxfd));
> >+		/* Retry... */
> >+		fd = fd_first_free(fdp, minfd, fdp->fd_nfiles);
> >+		if (fd>= maxfd)
> >+			return (EMFILE);
> >  	}
> >
> >  	/*
> >  	 * Perform some sanity checks, then mark the file descriptor as
> >  	 * used and return it to the caller.
> >  	 */
> >+	KASSERT((unsigned int)fd<  min(maxfd, fdp->fd_nfiles),
> >+	    ("invalid descriptor %d", fd));
> >  	KASSERT(!fdisused(fdp, fd),
> >  	    ("fd_first_free() returned non-free descriptor"));
> >  	KASSERT(fdp->fd_ofiles[fd] == NULL, ("file descriptor isn't free"));
> >
> My machine crashed with this change.
> http://people.freebsd.org/~davidxu/fdcrash.jpg
> 
> 

The problem is that fdalloc grows to at most fdp->fd_nfiles * 2, which
still may not be enough to have place for new fd with high number.

This fixed the problem for me, although I'm not sure whether it's ok to
grow the table like this:
http://people.freebsd.org/~mjg/patches/fdalloc.patch

-- 
Mateusz Guzik <mjguzik gmail.com>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120612104749.GB20749>