Date: Mon, 6 Feb 2012 00:12:59 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Pawel Jakub Dawidek <pjd@freebsd.org> Cc: arch@freebsd.org Subject: Re: Prefaulting for i/o buffers Message-ID: <20120205221259.GL3283@deviant.kiev.zoral.com.ua> In-Reply-To: <20120205192045.GH30033@garage.freebsd.pl> References: <20120203193719.GB3283@deviant.kiev.zoral.com.ua> <20120205114345.GE30033@garage.freebsd.pl> <20120205164729.GH3283@deviant.kiev.zoral.com.ua> <20120205192045.GH30033@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
--rYkcoROieYsUh8uu
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sun, Feb 05, 2012 at 08:20:45PM +0100, Pawel Jakub Dawidek wrote:
> On Sun, Feb 05, 2012 at 06:47:29PM +0200, Konstantin Belousov wrote:
> > On Sun, Feb 05, 2012 at 12:43:46PM +0100, Pawel Jakub Dawidek wrote:
> > > @@ -199,6 +200,7 @@ thread_init(void *mem, int size, int flags)
> > > =20
> > > td->td_sleepqueue =3D sleepq_alloc();
> > > td->td_turnstile =3D turnstile_alloc();
> > > + td->td_rlqe =3D rlqentry_alloc();
> > >=20
> > > What is the purpose of td_rlqe field? From what I see thread can lock
> > > more than one range. Could you elaborate on that as well?
> > td_rlqe is a cached rangelock entry used for the typical case of the
> > thread not doing recursive i/o. In this case, it is possible to avoid
> > memory allocation on the i/o hot path by using entry allocated during
> > thread creation. Since thread creation already allocates many chunks
> > of memory, and typical thread performs much more i/o then it suffers
> > creation, this shorten the i/o calculation path.
>=20
> I see. I'd be in favour of dropping entry allocation on thread creation.
> This would make the allocation lazy and it will be done on the first I/O
> only. What do you think? Thread creation should be fast, so if we can
> avoid adding up, I would go that route.
I think this is negligible change in the speed of much rare operation
(thread creation) comparing with a hit on the first time i/o, but I just
did it in a way you suggested.
>=20
> > > This is a bit hard to understand immediately. Maybe something like the
> > > following is a bit more readable (I assume this goto could happen only
> > > once):
> > >=20
> > > len =3D MIN(uio->uio_iov->iov_len, io_hold_cnt * PAGE_SIZE);
> > > addr =3D (vm_offset_t)uio->uio_iov->iov_base;
> > > end =3D round_page(addr + len);
> > > if (howmany(end - trunc_page(addr), PAGE_SIZE) > io_hold_cnt) {
> > > len -=3D PAGE_SIZE;
> > > end =3D round_page(addr + len);
> > > }
> > I think it can happen twice, if both start and len are perfectly mis-al=
igned.
>=20
> I see. That make sense. It would be nice to add comment there, as it
> wasn't immediately obvious (at least for me) what's going on.
>=20
> Another way to simplify this piece of code would be to either make ma[]
> array of 'io_hold_cnt + 2' elements or set 'len' to '(io_hold_cnt - 2) *
> PAGE_SIZE'. Short comment describing why we add or subtract 2 would be
> of course welcome.
I am not sure that +2 would be improvement in readability, but indeed it
removes some code. I implemented this and added a comment.
>=20
> Just curious, why io_hold_cnt is 'static const int' instead of define?
I want to change it into a sysctl or tunable one day.
http://people.freebsd.org/~kib/misc/vm1.9.patch
--rYkcoROieYsUh8uu
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (FreeBSD)
iEYEARECAAYFAk8u/usACgkQC3+MBN1Mb4ghkwCgiGZND59lcqCwmTNWk2ztHADq
w1wAnjUFE/i1HhxtS2Z7DxSQJOhec4hN
=FtVf
-----END PGP SIGNATURE-----
--rYkcoROieYsUh8uu--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120205221259.GL3283>