Return-Path: Received: from smtp1.linux-foundation.org ([140.211.169.13]:36904 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753690Ab0LAWZp (ORCPT ); Wed, 1 Dec 2010 17:25:45 -0500 In-Reply-To: <20101201141351.8609140b.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> <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> From: Linus Torvalds Date: Wed, 1 Dec 2010 14:24:42 -0800 Message-ID: Subject: Re: [PATCH v2 3/3] NFS: Fix a memory leak in nfs_readdir To: Andrew Morton Cc: Trond Myklebust , Hugh Dickins , Nick Piggin , Nick Bowler , Linux Kernel Mailing List , linux-nfs@vger.kernel.org, Rik van Riel , Christoph Hellwig , Al Viro Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 On Wed, Dec 1, 2010 at 2:13 PM, Andrew Morton wrote: > On Wed, 01 Dec 2010 16:51:12 -0500 > Trond Myklebust wrote: > >> On Wed, 2010-12-01 at 13:38 -0800, Andrew Morton wrote: >> > Probably on most call paths we'll be OK - if a process is in the middle >> > of a file truncate, holdin a file* ref which holds an inode ref then >> > nobody will be unmounting that fs and hence nobody will be unloading >> > that module. >> > >> > However on the random_code->alloc_page->vmscan->releasepage path, none >> > of that applies. >> >> Just out of interest, what ensures that the mapping is still around for >> the 'spin_unlock_irq(&mapping->tree_lock);' in __remove_mapping()? > > Nothing, afacit. No, we're good. Module unload has to go through a "stop_machine()" cycle, and that in turn requires an idle period for everything. And just a preemption reschedule isn't enough for that. So what is sufficient is that - we had the page locked and on the mapping This implies that we had an inode reference to the module, and the page lock means that the inode reference cannot go away (because it will involve invalidate-pages etc) - we're not sleeping after __remove_mapping, so unload can't happen afterwards. A _lot_ of the module races depend on that latter thing. We have almost no cases that are strictly about actual reference counts etc. Linus