Re: [PATCH 1/17] cramfs: use read_mapping_page
On 4/12/07, Roman Zippel <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/17] cramfs: use read_mapping_page
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. bye, Roman - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/17] cramfs: use read_mapping_page
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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/17] cramfs: use read_mapping_page
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. Signed-off-by: Nate Diller <[EMAIL PROTECTED]> --- diff -urpN -X dontdiff linux-2.6.21-rc6-mm1/fs/cramfs/inode.c linux-2.6.21-rc6-mm1-test/fs/cramfs/inode.c --- linux-2.6.21-rc6-mm1/fs/cramfs/inode.c 2007-04-09 17:24:03.0 -0700 +++ linux-2.6.21-rc6-mm1-test/fs/cramfs/inode.c 2007-04-09 21:37:09.0 -0700 @@ -180,8 +180,7 @@ static void *cramfs_read(struct super_bl struct page *page = NULL; if (blocknr + i < devsize) { - page = read_mapping_page_async(mapping, blocknr + i, - NULL); + page = read_mapping_page(mapping, blocknr + i, NULL); /* synchronous error? */ if (IS_ERR(page)) page = NULL; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/