Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 04 Nov 2010 18:56:10 +0200
From:      Andriy Gapon <avg@freebsd.org>
To:        Kostik Belousov <kostikbel@gmail.com>, Pawel Jakub Dawidek <pjd@freebsd.org>, Alan Cox <alc@freebsd.org>
Cc:        freebsd-fs@freebsd.org
Subject:   Re: vop_getpages for zfs
Message-ID:  <4CD2E5AA.1090208@freebsd.org>
In-Reply-To: <20101010134717.GA59922@deviant.kiev.zoral.com.ua>
References:  <4C91F031.1010801@freebsd.org> <20101010134717.GA59922@deviant.kiev.zoral.com.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
on 10/10/2010 16:47 Kostik Belousov said the following:
> I think that readahead requests should be satisfied, if possible. Esp.
> in the case the readahead does not cause additional i/o.

So, per Kostik's suggestion, here is patch for your review and testing:
http://people.freebsd.org/~avg/zfs_getpages.2.diff

The patch tries to fill as many of the passed in pages as possible as long as that
can be done from a block (or even two blocks) that has to be read to fill the
requested page.
This means that the optimization kicks in only if a vnode has a block size greater
than the page size.
I mentioned "two blocks" above because I tried to optimize the case when, for
example, block size is 6KB and page size is 4KB and the requested page covers
parts of two adjacent blocks (e.g. range from 4KB to 8KB).  Not sure if this was
worth the trouble.

In other words, the code tries to avoid reading more blocks than is strictly
required, because additional blocks may be far apart on a disk and thus read
latency could get increased.

You will also notice that I added vop_bmap method to ZFS vops.
This is needed to satisfy logic in vnode_pager_haspage(); otherwise, for example,
vm_fault() would never request any additional pages besides the required one.
zfs_bmap() implementation is really simple - it returns zeros for a_runp and
a_runb (and some reasonable-ish but unused values in other fields), and
vnode_pager_haspage() figures out number of eligible pages from the block size
(f_iosize actually[*]) and the page size.

Now, that ZFS has vop_getpages method, it is safe to provide this dummy-ish
implementation of vop_bmap, because this operation would not be called by any
other place but vnode_pager_haspage().  Code from vfs_bio.c and vfs_cluster.c is
never called for ZFS, because it doesn't use the buffer cache layer.

[*] There is a few interesting questions with respect to f_iosize usage in
vnode_pager_haspage() and ZFS.
For one, ZFS has a variable block size and f_iosize for ZFS has a currently
configured maximum block ('record' in ZFS terms) size.  This should not result in
non-optimal behavior for files with smaller block sizes, because those files would
have smaller sizes too (within the block size).
Unless, of course, recordsize property is modified after some files are created on
a ZFS filesystem.  In that case there could be large files with some smaller
(previously maximum) block size; or files with a block size larger than the
current recordsize == f_iosize.  So, such situations may lead to sub-optimal
performance.
Another issue related to the above is that f_iosize for ZFS can change on the fly
when an administrators runs zfs set recordsize=NNN, and that may happen
concurrently with vnode_pager_haspage() using f_iosize for its calculations.
Not sure how to handle this correctly.

I will appreciate your comment comments, test reports, reviews and suggestions.
Thanks a lot!
-- 
Andriy Gapon



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