From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756898AbZBFBos (ORCPT ); Thu, 5 Feb 2009 20:44:48 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752659AbZBFBok (ORCPT ); Thu, 5 Feb 2009 20:44:40 -0500 Received: from mx2.redhat.com ([66.187.237.31]:38241 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752650AbZBFBok (ORCPT ); Thu, 5 Feb 2009 20:44:40 -0500 Date: Fri, 6 Feb 2009 02:44:00 +0100 From: Andrea Arcangeli To: Christoph Lameter Cc: Robin Holt , linux-mm@kvack.org, Nick Piggin , Andrew Morton , linux-kernel@vger.kernel.org Subject: Re: [Patch] mmu_notifiers destroyed by __mmu_notifier_release() retain extra mm_count. Message-ID: <20090206014400.GM14011@random.random> References: <20090205172303.GB8559@sgi.com> <20090205200214.GN8577@sgi.com> <20090206013805.GL14011@random.random> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090206013805.GL14011@random.random> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 06, 2009 at 02:38:05AM +0100, Andrea Arcangeli wrote: > It all boils down if unregister is mandatory or not. If it's mandatory Oh I just found I documented it too!! ;) /* * Must not hold mmap_sem nor any other VM related lock when calling * this registration function. Must also ensure mm_users can't go down * to zero while this runs to avoid races with mmu_notifier_release, * so mm has to be current->mm or the mm should be pinned safely such * as with get_task_mm(). If the mm is not current->mm, the mm_users * pin should be released by calling mmput after mmu_notifier_register * returns. mmu_notifier_unregister must be always called to ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ * unregister the notifier. mm_count is automatically pinned to allow ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ * mmu_notifier_unregister to safely run at any time later, before or ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ * after exit_mmap. ->release will always be called before exit_mmap * frees the pages. */ So in short the current code has no bugs and the fact you have to call unregister is intentional. Not patch required unless you request to change API. If you don't call unregister mm will be leaked, simply. For a moment I thought unregister wasn't mandatory because at some point in one of the dozen versions of the api it wasn't, but in the end I thought having an mm_count auto-pinning leaving no window for corrupted mmu_notifier list was preferable ;).