Return-Path: Received: from cantor.suse.de ([195.135.220.2]:58304 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755523Ab0LAXCL (ORCPT ); Wed, 1 Dec 2010 18:02:11 -0500 Date: Thu, 2 Dec 2010 10:01:58 +1100 From: Neil Brown To: Andrew Morton Cc: Trond Myklebust , Hugh Dickins , Linus Torvalds , Nick Bowler , Linux Kernel Mailing List , linux-nfs@vger.kernel.org, Rik van Riel , Christoph Hellwig , Al Viro , Nick Piggin Subject: Re: [PATCH v2 3/3] NFS: Fix a memory leak in nfs_readdir Message-ID: <20101202100158.3387566c@notabene.brown> In-Reply-To: <20101201144314.6a827f8d.akpm@linux-foundation.org> References: <1291217804-11257-1-git-send-email-Trond.Myklebust@netapp.com> <1291217804-11257-2-git-send-email-Trond.Myklebust@netapp.com> <20101201150428.GA2879@elliptictech.com> <1291217804-11257-3-git-send-email-Trond.Myklebust@netapp.com> <1291217804-11257-4-git-send-email-Trond.Myklebust@netapp.com> <1291229669.6609.24.camel@heimdal.trondhjem.org> <1291233938.6609.37.camel@heimdal.trondhjem.org> <20101201123929.ab7cef1d.akpm@linux-foundation.org> <20101202082913.7cb98444@notabene.brown> <20101201144314.6a827f8d.akpm@linux-foundation.org> Content-Type: text/plain; charset=US-ASCII Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 On Wed, 1 Dec 2010 14:43:14 -0800 Andrew Morton wrote: > On Thu, 2 Dec 2010 08:29:13 +1100 > Neil Brown wrote: > > > > > > > > > However, while reading Documentation/filesystems/vfs.txt (in order to > > > > add documentation for freepage) I was surprised to read that the > > > > ->releasepage() is itself supposed to be allowed to actually remove the > > > > page from the address space if it so desires. > > > > > > That doesn't sound right. It came from Neil in 2006. > > > > > > Neil, what were you thinking there? Did you find such a ->releasepage()? > > > > Nope, no idea, sorry. > > > > No releasepage functions do anything like that, and no call sites suggest it > > could be a possibility. Quite the reverse - they are likely to remove the > > page from the mapping without checking that it is still in the mapping. > > > > So that sentence should be deleted. > > This? Perfect, thanks. NeilBrown > > From: Andrew Morton > > ->releasepage() does not remove the page from the mapping. > > Cc: Neil Brown > Cc: Trond Myklebust > Signed-off-by: Andrew Morton > --- > > Documentation/filesystems/vfs.txt | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff -puN Documentation/filesystems/vfs.txt~documentation-filesystems-vfstxt-fix-repeasepage-description Documentation/filesystems/vfs.txt > --- a/Documentation/filesystems/vfs.txt~documentation-filesystems-vfstxt-fix-repeasepage-description > +++ a/Documentation/filesystems/vfs.txt > @@ -660,11 +660,10 @@ struct address_space_operations { > releasepage: releasepage is called on PagePrivate pages to indicate > that the page should be freed if possible. ->releasepage > should remove any private data from the page and clear the > - PagePrivate flag. It may also remove the page from the > - address_space. If this fails for some reason, it may indicate > - failure with a 0 return value. > - This is used in two distinct though related cases. The first > - is when the VM finds a clean page with no active users and > + PagePrivate flag. If releasepage() fails for some reason, it must > + indicate failure with a 0 return value. > + releasepage() is used in two distinct though related cases. The > + first is when the VM finds a clean page with no active users and > wants to make it a free page. If ->releasepage succeeds, the > page will be removed from the address_space and become free. > > _