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

Alex Bennée
2024-08-08T15:18:30Z

Sean Christopherson <seanjc@google.com> writes:

> On Thu, Aug 08, 2024, Alex Bennée wrote:
>> Sean Christopherson <seanjc@google.com> 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