Messages in this thread | | | Date | Thu, 12 Apr 2007 11:36:42 -0700 | From | "Nate Diller" <> | Subject | Re: [PATCH 1/17] cramfs: use read_mapping_page |
| |
On 4/12/07, Roman Zippel <zippel@linux-m68k.org> wrote: > Hi, > > On Thu, 12 Apr 2007, Christoph Hellwig wrote: > > > On Wed, Apr 11, 2007 at 07:49:38PM -0700, Nate Diller wrote: > > > read_mapping_page_async() is going away, so convert its only user to > > > read_mapping_page(). This change has not been benchmarked, however, in > > > order to get real parallelism this wants something completely different, > > > like __do_page_cache_readahead(), which is not currently exported. > > > > Why is read_mapping_page_async going away? This probably needs a lot more > > testing, and I'd be much happier if you split it out of the series and > > sent it separately at the end. > > That function wasn't fully async anyway, as it would often sleep in > lock_page(). AFAICT only in the special case of a partial written page > would this function return a not yet uptodate page.
yes, exactly, the structure of read_cache_page() and friends is totally not appropriate for doing async I/O to more than one page at a time, and the whole point of the special treatment in cramfs was to read 4 pages at once rather than synchronously reading each of the 4 seperately. read_cache_page_async() is totally wrong for that use, its purpose would be to get a reference to a single page that is likely to be in cache already without having to take the page_lock. Turns out nobody needs to do that, so there's no point in keeping it around.
If the performance gain of reading all 4 pages at once would be worth the effort, this code should be using __do_page_cache_readahead(). That function allocates all the pages first, then reads them in asynchronously as a group. It is currently not exported.
NATE - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |