Subject: Re: [PATCH v12 13/84] KVM: Annotate that all paths in hva_to_pfn() might sleep

From: Alex Bennée <[email-masked]>
Date: Thu, 08 Aug 2024 16:18:17 +0100

Sean Christopherson <[email-masked]> writes:

> On Thu, Aug 08, 2024, Alex Bennée wrote:
>> Sean Christopherson <[email-masked]> writes:
>> 
>> > Now that hva_to_pfn() no longer supports being called in atomic context,
>> > move the might_sleep() annotation from hva_to_pfn_slow() to
>> > hva_to_pfn().
>> 
>> The commentary for hva_to_pfn_fast disagrees.
>> 
>>   /*
>>    * The fast path to get the writable pfn which will be stored in @pfn,
>>    * true indicates success, otherwise false is returned.  It's also the
>>    * only part that runs if we can in atomic context.
>>    */
>>   static bool hva_to_pfn_fast(struct kvm_follow_pfn *kfp, kvm_pfn_t *pfn)
>> 
>> At which point did it loose the ability to run in the atomic context? I
>> couldn't work it out from the commits.
>
> It didn't lose the ability per se (calling hva_to_pfn_fast() in atomic context
> would still be functionally ok), rather the previous patch
>
>   KVM: Drop @atomic param from gfn=>pfn and hva=>pfn APIs
>
> removed support for doing so in order to simplify hva_to_pfn() as a whole.

It still sticks out given the only caller no longer enforces this. 

How about:

    * true indicates success, otherwise false is returned.  It's also the
    * only part that could run in an atomic context if we wanted to
    * (although no callers expect it to).

?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro