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

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jun 12, 2012 at 06:01:29PM +0200, Pawel Jakub Dawidek wrote:
> On Tue, Jun 12, 2012 at 03:49:50PM +0200, Mateusz Guzik wrote:
> > On Tue, Jun 12, 2012 at 01:43:35PM +0200, Pawel Jakub Dawidek wrote:
> > > On Tue, Jun 12, 2012 at 12:47:49PM +0200, Mateusz Guzik wrote:
> > > > 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.
> > > 
> > > I was under impression that fd_first_free() can return at most
> > > fdp->fd_nfiles, but indeed I missed this:
> > > 
> > > 	if (low >= size)
> > > 		return (low);
> > > 
> > > So fd_first_free() can return number biffer than size...
> > > 
> > > > 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
> > > 
> > > The patch looks good to me, could you please commit it, preferably after
> > > David's trying it and also update fd_first_free() comment, so it is
> > > clear that returned value can exceed 'size -1'?
> > > 
> > 
> > Given that you partially reverted r236935 I created a combined patch:
> > http://people.freebsd.org/~mjg/patches/fdalloc%2bfd_first_free.patch
> > 
> > Is this ok?
> > 
> > Most changes are obiously yours, so I see no problem if you prefer to
> > commit this yourself.
> > 
> > Otherwise I plan to commit it with the following:
> > Re-apply reverted parts of r236935 by pjd with some fixes.
> > 
> > If fdalloc decides to grow fdtable it does it once and at most doubles
> > the size. This still may be not enough for sufficiently large fd. Use fd
> > in calculations of new size in order to fix this.
> > 
> > Fix description of fd_first_free to note that it returns passed fd if it
> > exceeds fdtable's size.
> > 
> > ======
> 
> Look good and you can just add 'In co-operation with: pjd'.
> One minor thing is that fd_first_free() can return 'size' if there are
> no free slots available. Could you include that in the comment as well?
> 

http://people.freebsd.org/~mjg/patches/fdalloc%2bfd_first_free2.patch

> > fd_last_used has very same problem with comment as fd_first_free. This
> > function is static and the only caller always passes 0 as low. Given
> > that, how about the following:
> > http://people.freebsd.org/~mjg/patches/fd_last_used-cleanup.patch
> 
> Looks good too.
> 

Updated in similar manner:
http://people.freebsd.org/~mjg/patches/fd_last_used-cleanup2.patch

-- 
Mateusz Guzik <mjguzik gmail.com>



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