From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030686AbXDLSgp (ORCPT ); Thu, 12 Apr 2007 14:36:45 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1030720AbXDLSgp (ORCPT ); Thu, 12 Apr 2007 14:36:45 -0400 Received: from qb-out-0506.google.com ([72.14.204.234]:56305 "EHLO qb-out-0506.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030685AbXDLSgn (ORCPT ); Thu, 12 Apr 2007 14:36:43 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=WrWzZbfYT1Nk21ulEhIkCCvCe72ALCkKxQEvpVJeWcczWtQ35fhdJGQa5v8fANQvKpOR4xmov27Ad5pZwrFVvBkvi9QxhhaqOe8gziRNs9pIGGC4s4iyt4Tc+8LzQ5bbrbb0o96lZALE/sj1grxDbXh6x/AThZr6Nm1zLrN9Z5s= Message-ID: <5c49b0ed0704121136uce5b15ah3004cd19e44994f3@mail.gmail.com> Date: Thu, 12 Apr 2007 11:36:42 -0700 From: "Nate Diller" To: "Roman Zippel" Subject: Re: [PATCH 1/17] cramfs: use read_mapping_page Cc: "Christoph Hellwig" , "Andrew Morton" , "Alexander Viro" , "Mikulas Patocka" , "David Woodhouse" , "Dave Kleikamp" , "Anton Altaparmakov" , "Evgeniy Dushistov" , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, reiserfs-dev@namesys.com In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20070412024938.27380.54538.patchbomb.py@localhost> <20070412024938.27380.87060.patchbomb.py@localhost> <20070412095428.GA22675@infradead.org> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On 4/12/07, Roman Zippel 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