From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrea Arcangeli Subject: Re: KVM: mmu_notifiers release method Date: Wed, 24 Dec 2008 16:28:44 +0100 Message-ID: <20081224152844.GE29319@random.random> References: <20081210202326.GA7565@dmt.cnet> <49523031.1000305@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Marcelo Tosatti , kvm-devel To: Avi Kivity Return-path: Received: from mx2.redhat.com ([66.187.237.31]:38530 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750959AbYLXP2s (ORCPT ); Wed, 24 Dec 2008 10:28:48 -0500 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id mBOFSlE6024475 for ; Wed, 24 Dec 2008 10:28:47 -0500 Content-Disposition: inline In-Reply-To: <49523031.1000305@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Dec 24, 2008 at 02:50:57PM +0200, Avi Kivity wrote: > Marcelo Tosatti wrote: >> The destructor for huge pages uses the backing inode for adjusting >> hugetlbfs accounting. >> >> Hugepage mappings are destroyed by exit_mmap, after >> mmu_notifier_release, so there are no notifications through >> unmap_hugepage_range at this point. >> >> The hugetlbfs inode can be freed with pages backed by it referenced >> by the shadow. When the shadow releases its reference, the huge page >> destructor will access a now freed inode. >> >> Implement the release operation for kvm mmu notifiers to release page >> refs before the hugetlbfs inode is gone. >> >> > > I see this isn't it. Andrea, comments? Yeah, the patch looks good, I talked a bit with Marcelo about this by PM. The issue is that it's not as strightforward as it seems, basically when I implemented the ->release handlers and had sptes teardown running before the files were closed (instead of waiting the kvm anon inode release handler to fire) I was getting bugchecks from debug options including preempt=y (certain debug checks only becomes functional with preempt enabled unfortunately), so eventually I removed ->release because for kvm ->release wasn't useful because no guest mode can run any more by the time mmu notifier ->release is invoked, and that avoided the issues with the bugchecks. We'll be using the mmu notifiers ->release because it's always called just before the filehandle are destroyed, it's not really about the guest mode or secondary mmu but just an ordering issue with hugetlbfs internals. So in short if no bugcheck triggers this is fine (at least until hugetlbfs provides a way to register some callback to invoke at the start of the hugetlbfs->release handler).