Date: Wed, 13 Jun 2012 10:00:14 +0800 From: David Xu <listlog2011@gmail.com> To: Mateusz Guzik <mjguzik@gmail.com> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Pawel Jakub Dawidek <pjd@FreeBSD.org>, davidxu@FreeBSD.org Subject: Re: svn commit: r236935 - head/sys/kern Message-ID: <4FD7F42E.5080505@gmail.com> In-Reply-To: <20120612191828.GD20749@dft-labs.eu> 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> <20120612191828.GD20749@dft-labs.eu>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2012/6/13 3:18, Mateusz Guzik wrote: > 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 I have tested it, the machine does not panic. Thanks, David Xu
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4FD7F42E.5080505>