Re: KVM: mmu_notifiers release method

[Andrea Arcangeli <aarcange@redhat.com>]

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).