Re: kvm_gfn_to_pfn_cache_refresh started getting a warning recently

[Paolo Bonzini <pbonzini@redhat.com>]

On 4/26/22 20:15, Sean Christopherson wrote:
> On Tue, Apr 26, 2022, Sean Christopherson wrote:
>> On Tue, Apr 26, 2022, Paolo Bonzini wrote:
>>> On 4/26/22 18:45, Sean Christopherson wrote:
>>>> On Tue, Apr 26, 2022, Maxim Levitsky wrote:
>>>>> [  390.511995] BUG: sleeping function called from invalid context at include/linux/highmem-internal.h:161
>>>>> [  390.513681] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 4439, name: CPU 0/KVM
>>>>
>>>> This is my fault.  memremap() can sleep as well.  I'll work on a fix.
>>>
>>> Indeed, "KVM: Fix race between mmu_notifier invalidation and pfncache
>>> refresh" hadn't gone through a full test cycle yet.
>>
>> And I didn't run with PROVE_LOCKING :-(
>>
>> I'm pretty sure there's an existing memory leak too.  If a refresh occurs, but
>> the pfn ends up being the same, KVM will keep references to both the "old" and the
>> "new", but only release one when the cache is destroyed.
>>
>> The refcounting bug begs the question of why KVM even keeps a reference.  This code
>> really should look exactly like the page fault path, i.e. should drop the reference
>> to the pfn once the pfn has been installed into the cache and obtained protection
>> via the mmu_notifier.
> 
> This is getting a bit gnarly.  It probably makes sense to drop the existing patches
> from kvm/queue, and then I can send a full v2 instead of just the delta?

Yes, that's a good idea.

Paolo