From owner-svn-src-all@FreeBSD.ORG Wed Jun 13 10:53:55 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 6D6401065670; Wed, 13 Jun 2012 10:53:55 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-ee0-f54.google.com (mail-ee0-f54.google.com [74.125.83.54]) by mx1.freebsd.org (Postfix) with ESMTP id 3EFC18FC17; Wed, 13 Jun 2012 10:53:54 +0000 (UTC) Received: by eeke49 with SMTP id e49so162383eek.13 for ; Wed, 13 Jun 2012 03:53:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=40EiSlTavIZfLXH/c3ZBh4wDYpU6Yq2r3Cr2mz10Hy8=; b=Ldtr/ESX5Cy7aFD0pqiFkzJOxoSF2yfo8STn5/P6Qwt9fuzhwus8JpIcRsumCiT6OV 5OuTqWEekNN2AwAfGRU7r4HEUQGAqaecjI9kvzyBrc4hpbgkx66XtfFFiBvz+Aqew5kp v6YgNCxTAj6Lhtch298luKSkUm8xFgIfSAASQZA3JCX12PeAnPt61rKOx4t6A2e0NrsH /N5wK61ZHlrRWqfinJOROu5itaTNQGOXUE/HSNePfCe7Ez1kowAFCK4QYL9dtZl7aXaw X4kdBrk5mtGLjqvUo38z034vtXsgI83dZ4aoCBet2luDRvFLOLUX5zoHPsjPKN6bOZbM 1BYw== Received: by 10.14.97.137 with SMTP id t9mr7890617eef.73.1339584833303; Wed, 13 Jun 2012 03:53:53 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPS id c42sm6143642eeb.2.2012.06.13.03.53.51 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 13 Jun 2012 03:53:52 -0700 (PDT) Date: Wed, 13 Jun 2012 12:53:44 +0200 From: Mateusz Guzik To: Pawel Jakub Dawidek Message-ID: <20120613105344.GB3893@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> <20120612214400.GB1429@garage.freebsd.pl> <20120613093807.GA3893@dft-labs.eu> <20120613102900.GD1386@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20120613102900.GD1386@garage.freebsd.pl> User-Agent: Mutt/1.5.20 (2009-06-14) 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 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list 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 10:53:55 -0000 On Wed, Jun 13, 2012 at 12:29:01PM +0200, Pawel Jakub Dawidek wrote: > On Wed, Jun 13, 2012 at 11:38:07AM +0200, Mateusz Guzik wrote: > > On Tue, Jun 12, 2012 at 11:44:00PM +0200, Pawel Jakub Dawidek wrote: > > > On Tue, Jun 12, 2012 at 09:18:28PM +0200, Mateusz Guzik wrote: > > > > On Tue, Jun 12, 2012 at 06:01:29PM +0200, Pawel Jakub Dawidek wrote: > > > > > 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 > > > > > > Ok. Merge racct_set() call into one line, it now fits into 80 chars. > > > I have no more objections. > > > > > > > This patch contains terrible brain-o: > > > > if (fd >= fdp->fd_nfiles) { > > allocfd = 2 * max(fdp->fd_nfiles, fd); > > > > allocfd is always 2 * fd, which I think is ok, just no reason to compare > > again with fdp->fd_nfiles. > > > > Also the patch can be simplified. > > > > fdp xlock is held for whole duration of fdalloc. > > > > Code: > > fdgrowtable(fdp, min(allocfd, maxfd)); > > /* Retry... */ > > fd = fd_first_free(fdp, minfd, fdp->fd_nfiles); > > if (fd >= maxfd) > > return (EMFILE); > > > > fdgrowtable never fails and when we call it we know that new fd will fit > > in the table. In case of no free entries fd_first_free returns size, > > which coressponds to first newly allocated fd after growth. So fd after > > table growth is already the first free descriptor that can be used and > > would be returned by another call to fd_first_free. > > > > To sum up: > > http://people.freebsd.org/~mjg/patches/fdalloc%2bfd_first_free4.patch > > > > Any comments? > > All makes sense and the code looks good, but I'd love to see such an > explanation in the code as well (below or above fdgrowtable() call). > http://people.freebsd.org/~mjg/patches/fdalloc%2bfd_first_free5.patch David reported that previous version of the patch fixed panic he was experiencing, so I consider the patch tested. Thanks, -- Mateusz Guzik