From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Lameter Subject: Re: [patch 01/19] Define functions for page cache handling Date: Tue, 18 Dec 2007 12:20:16 -0800 (PST) Message-ID: References: <20071130173448.951783014@sgi.com> <20071130173506.366983341@sgi.com> <20071203141020.c8119197.akpm@linux-foundation.org> Mime-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, hch@lst.de, mel@skynet.ie, wli@holomorphy.com, dgc@sgi.com, jens.axboe@oracle.com, pbadari@gmail.com, maximlevitsky@gmail.com, fengguang.wu@gmail.com, wangswin@gmail.com, totty.lu@gmail.com, hugh@veritas.com, joern@lazybastard.org To: Andrew Morton Return-path: Received: from netops-testserver-3-out.sgi.com ([192.48.171.28]:42105 "EHLO relay.sgi.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751933AbXLRUUS (ORCPT ); Tue, 18 Dec 2007 15:20:18 -0500 In-Reply-To: <20071203141020.c8119197.akpm@linux-foundation.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, 3 Dec 2007, Andrew Morton wrote: > These will of course all work OK as they are presently implemented. > > But you have callsites doing things like > > page_cache_size(page_mapping(page)); > > which is a whole different thing. Once page_cache_size() is changed to > look inside the address_space we need to handle races against truncation > and we need to handle the address_space getting reclaimed, etc. Right. The page must be locked for that to work right. I tried to avoid the above construct as much as possible by relying on the inode mapping. I can go over this again to make sure that there is nothing amiss after the recent changes. > So I think it would be misleading to merge these changes at present - they > make it _look_ like we can have variable PAGE_CACHE_SIZE just by tweaking a > bit of core code, but we in fact cannot do that without a careful review of > all callsites and perhaps the addition of new locking and null-checking. The mapping is generally available in some form if you cannot get it from the page. In some cases I added a new parameter to functions to pass the mapping so that we do not have to use page->mapping. I can recheck that all is fine on that level. > And a coding nit: when you implement the out-of-line versions of these > functions you're going to stick with VFS conventions and use the identifier > `mapping' to identify the address_space*. So I think it would be better to > also call in `mapping' in these inlined stubbed functions, rather than `a'. > No? Ok. A trivial change. But a is shorter and made the functions more concise.