From owner-svn-src-all@FreeBSD.ORG Wed Jun 13 02:00:28 2012 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id A3E8D106566C; Wed, 13 Jun 2012 02:00:28 +0000 (UTC) (envelope-from listlog2011@gmail.com) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 270AB8FC19; Wed, 13 Jun 2012 02:00:17 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q5D20Exu093894; Wed, 13 Jun 2012 02:00:15 GMT (envelope-from listlog2011@gmail.com) Message-ID: <4FD7F42E.5080505@gmail.com> Date: Wed, 13 Jun 2012 10:00:14 +0800 From: David Xu User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:12.0) Gecko/20120428 Thunderbird/12.0.1 MIME-Version: 1.0 To: Mateusz Guzik 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> In-Reply-To: <20120612191828.GD20749@dft-labs.eu> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Pawel Jakub Dawidek , davidxu@FreeBSD.org Subject: Re: svn commit: r236935 - head/sys/kern X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: davidxu@FreeBSD.org List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Jun 2012 02:00:28 -0000 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