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 <mjguzik gmail.com> _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
