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

Alex Bennée
2024-08-08T16:17:07Z

Sean Christopherson <seanjc@google.com> writes:

> On Thu, Aug 08, 2024, Alex Bennée wrote:
>> 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.
>
> Oh, sorry, I should have been more explicit. I'll fix the comment, I simply
> missed it.

No worries, with the fixed comment:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

--
Alex Bennée
Virtualisation Tech Lead @ Linaro