Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756218Ab0LAX4T (ORCPT ); Wed, 1 Dec 2010 18:56:19 -0500 Received: from mx2.netapp.com ([216.240.18.37]:38369 "EHLO mx2.netapp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752371Ab0LAX4S convert rfc822-to-8bit (ORCPT ); Wed, 1 Dec 2010 18:56:18 -0500 X-IronPort-AV: E=Sophos;i="4.59,285,1288594800"; d="scan'208";a="489758665" Subject: Re: [PATCH v2 3/3] NFS: Fix a memory leak in nfs_readdir From: Trond Myklebust To: Andrew Morton Cc: Linus Torvalds , Hugh Dickins , Nick Piggin , Nick Bowler , Linux Kernel Mailing List , linux-nfs@vger.kernel.org, Rik van Riel , Christoph Hellwig , Al Viro In-Reply-To: <20101201154651.958678f6.akpm@linux-foundation.org> References: <1291217804-11257-1-git-send-email-Trond.Myklebust@netapp.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> <1291234251.6609.39.camel@heimdal.trondhjem.org> <20101201123341.d12ef362.akpm@linux-foundation.org> <20101201133831.ea6ba10a.akpm@linux-foundation.org> <1291240272.6609.50.camel@heimdal.trondhjem.org> <20101201141351.8609140b.akpm@linux-foundation.org> <20101201143856.51f4f9d9.akpm@linux-foundation.org> <1291243633.6609.59.camel@heimdal.trondhjem.org> <1291245676.6609.62.camel@heimdal.trondhjem.org> <20101201154651.958678f6.akpm@linux-foundation.org> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Organization: NetApp Inc Date: Wed, 01 Dec 2010 18:56:16 -0500 Message-ID: <1291247776.6609.79.camel@heimdal.trondhjem.org> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 (2.32.1-1.fc14) X-OriginalArrivalTime: 01 Dec 2010 23:56:17.0645 (UTC) FILETIME=[584791D0:01CB91B3] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1625 Lines: 53 On Wed, 2010-12-01 at 15:46 -0800, Andrew Morton wrote: > On Wed, 01 Dec 2010 18:21:16 -0500 > Trond Myklebust wrote: > > > --- a/mm/truncate.c > > +++ b/mm/truncate.c > > @@ -108,6 +108,10 @@ truncate_complete_page(struct address_space *mapping, struct page *page) > > clear_page_mlock(page); > > remove_from_page_cache(page); > > ClearPageMappedToDisk(page); > > + > > + if (mapping->a_ops->freepage) > > + mapping->a_ops->freepage(page); > > + > > page_cache_release(page); /* pagecache ref */ > > return 0; > > } > > So here we're assuming that `mapping' was pinned by other means. > > Fair enough, although subtle. Even drop_pagecache_sb() got it right ;) > > > @@ -390,6 +394,10 @@ invalidate_complete_page2(struct address_space *mapping, struct page *page) > > __remove_from_page_cache(page); > > spin_unlock_irq(&mapping->tree_lock); > > mem_cgroup_uncharge_cache_page(page); > > + > > + if (mapping->a_ops->freepage) > > + mapping->a_ops->freepage(page); > > + > > page_cache_release(page); /* pagecache ref */ > > return 1; > > failed: > > And here. Yes. Both these functions are static, and their callers are assuming that something is already pinning the underlying inode, so the above should be quite safe. -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@netapp.com www.netapp.com -- 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/