Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 13 Jun 2012 12:29:01 +0200
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        Mateusz Guzik <mjguzik@gmail.com>
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:  <20120613102900.GD1386@garage.freebsd.pl>
In-Reply-To: <20120613093807.GA3893@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>

next in thread | previous in thread | raw e-mail | index | archive | help

--p2kqVDKq5asng8Dg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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 w=
ell?
> > > >=20
> > >=20
> > > http://people.freebsd.org/~mjg/patches/fdalloc%2bfd_first_free2.patch
> >=20
> > Ok. Merge racct_set() call into one line, it now fits into 80 chars.
> > I have no more objections.
> >=20
>=20
> This patch contains terrible brain-o:
>=20
> if (fd >=3D fdp->fd_nfiles) {
> 	allocfd =3D 2 * max(fdp->fd_nfiles, fd);
>=20
> allocfd is always 2 * fd, which I think is ok, just no reason to compare
> again with fdp->fd_nfiles.
>=20
> Also the patch can be simplified.
>=20
> fdp xlock is held for whole duration of fdalloc.
>=20
> Code:
> fdgrowtable(fdp, min(allocfd, maxfd));
> /* Retry... */
> fd =3D fd_first_free(fdp, minfd, fdp->fd_nfiles);
> if (fd >=3D maxfd)
> 	return (EMFILE);
>=20
> 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.
>=20
> To sum up:
> http://people.freebsd.org/~mjg/patches/fdalloc%2bfd_first_free4.patch
>=20
> 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).

--=20
Pawel Jakub Dawidek                       http://www.wheelsystems.com
FreeBSD committer                         http://www.FreeBSD.org
Am I Evil? Yes, I Am!                     http://tupytaj.pl

--p2kqVDKq5asng8Dg
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)

iEYEARECAAYFAk/Ya2wACgkQForvXbEpPzQXXgCg4w58aYAPKd6Sme7c6LdJgekn
BW4An0Qlnq5YQeM+VHwawpBcw8ScVm9s
=aGeV
-----END PGP SIGNATURE-----

--p2kqVDKq5asng8Dg--



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