Correctly lock pages for .readpages()
authorBrian Behlendorf <behlendorf1@llnl.gov>
Thu, 4 Aug 2011 23:25:43 +0000 (16:25 -0700)
committerBrian Behlendorf <behlendorf1@llnl.gov>
Mon, 8 Aug 2011 20:24:52 +0000 (13:24 -0700)
commitf3ab88d6461dec46dea240763843f66300facfab
treeff5e53dfd289253d96b38347158047b1e26a3f51
parent76659dc110ef2ada13bcb8e4e2ec60d8216c6836
Correctly lock pages for .readpages()

Unlike the .readpage() callback which is passed a single locked page
to be populated.  The .readpages() callback is passed a list of unlocked
pages which are all marked for read-ahead (PG_readahead set).  It is
the responsibly of .readpages() to ensure to pages are properly locked
before being populated.

Prior to this change the requested read-ahead pages would be updated
outside of the page lock which is unsafe.  The unlocked pages would then
be unlocked again which is harmless but should have been immediately
detected as bug.  Unfortunately, newer kernels failed detect this issue
because the check is done with a VM_BUG_ON which is disabled by default.
Luckily, the old Debian Lenny 2.6.26 kernel caught this because it
simply uses a BUG_ON.

The straight forward fix for this is to update the .readpages() callback
to use the read_cache_pages() helper function.  The helper function will
ensure that each page in the list is properly locked before it is passed
to the .readpage() callback.  In addition resolving the bug, this results
in a nice simplification of the existing code.

The downside to this change is that instead of passing one large read
request to the dmu multiple smaller ones are submitted.  All of these
requests however are marked for readahead so the lower layers should
issue a large I/O regardless.  Thus most of the request should hit the
ARC cache.

Futher optimization of this code can be done in the future is a perform
analysis determines it to be worthwhile.  But for the moment, it is
preferable that code be correct and understandable.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #355
module/zfs/zpl_file.c