Re: reply: [RFC] pin_user_pages_fast failure count increased

[David Hildenbrand <david@redhat.com>]

On 28.05.25 12:59, Zhaoyang Huang wrote:
> On Wed, May 28, 2025 at 3:55 PM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 28.05.25 05:36, Hyesoo Yu wrote:
>>> On Wed, May 28, 2025 at 10:49:36AM +0800, Zhaoyang Huang wrote:
>>>> On Wed, May 28, 2025 at 9:25 AM Hyesoo Yu <hyesoo.yu@samsung.com> wrote:
>>>>>
>>>>> On Mon, May 26, 2025 at 07:49:57PM +0800, Zhaoyang Huang wrote:
>>>>>
>>>>> Hello, Zhaoyang.
>>>>>
>>>>> I don't believe commit 1aaf8c was just intended to prevent an infinite loop.
>>>>> The commit was introduced to allow pinning CMA memory in the pKVM on AOSP.
>>>>>
>>>>> That leads me to question whether the assumption that CMA can be long-term pinned is actually valid.
>>>> That depends on the user of CMA, yes for my scenario since it worked
>>>> for the guest os. For common scenario such as the file/anon mapping,
>>>> the page will be judged as unpinnable for long-term and be migrated
>>>> out of CMA area.
>>>
>>> Your scenario and the common scenarios can not be distinguished from the kernel API's perspective.
>>> Even in common cases, the page may be in a non-LRU state temporiarily, and in such situations,
>>> pinning CMA can lead to bugs - we've encountered multiple issues because of this.
>>>
>>
>> Right. We just disallow long-term pinning CMA pages, because we don't
>> know who the real owner is that would be okay with long-term pinning them.
>>
>>>>>
>>>>> In my opinion, it might be more appropriate to revert that commit 1aaf8c and instead ensure
>>>>> that pKVM avoids using CMA for memory that requires long-term pinning through GUP ?
>>>> It is not a pkvm issue but a defect of applying FOLL_LONGTERM over
>>>> non-LRU CMA pages.
>>>
>>> In include/linux/mm_types.h, the CMA should be migrated when FOLL_LONGTERM.
>>>
>>> * In the CMA case: long term pins in a CMA region would unnecessarily fragment
>>> * that region.  And so, CMA attempts to migrate the page before pinning, when
>>> * FOLL_LONGTERM is specified.
>>>
>>> Given this, would it make sense to avoid using FOLL_LONGTERM in this code path ?
>>
>> If something is unbounded in time, FOLL_LONGTERM is the right thing to use.
>>
>>>>>
>>>>> Alternatively, instead of changing the current logic that prevents longterm GUP from pinning CMA,
>>>>> it would be better to propose a new patch that specifically addresses the pKVM scenario like adding new FOLL_flags ?
>>>> I don't think so. pin_user_pages is an exported API which can't make
>>>> assumptions over the caller.
>>>
>>> My point is not to base the patch on assumptions about the caller,
>>> but to define a clear mechanism that ensures safe behavior in the intended scenario.
>>>
>>> For example, you can add FOLL_NO_MIGRATION and skip to migrate unpinnable pages.
>>
>> Not sure which exact semantics you have in mind. But failing if we would
>> have to migrate might be ok. Not sure if the caller should worry about
>> that, though: the caller should not have to worry about page placement
>> in general.
> With going over the whole thread, I think the root cause is
> collect_longterm_unpinnable_folios() hit the race window between
> lru_add_drain_all() and folio_isolate_lru() by chance and returned
> with ret=0 which finally have the CMA page pinned, right? However, I
> find the proposed patch below will fail the PKVM
> scenario(FOLL_LONGTERM set with non-LRU CMA pages) again as the CMA
> pages never go to LRU which will have the __gup_longterm_locked loop
> in do while(ret == -EAGAIN) as it did before 1aaf8c.

For upstream that is the exact right thing to do.

CMA pages must not get longterm pinned.

-- 
Cheers,

David / dhildenb