Re: reply: [RFC] pin_user_pages_fast failure count increased

[Zhaoyang Huang <huangzhaoyang@gmail.com>]

On Mon, May 26, 2025 at 3:50 PM Hyesoo Yu <hyesoo.yu@samsung.com> wrote:
>
> On Thu, May 22, 2025 at 07:52:41PM -0700, John Hubbard wrote:
> > On 5/22/25 7:37 PM, 김재원 wrote:
> > ...
> > > I think this is what you meant, please let me know if you have an idea to make this nicer.
> > > We may be to able to prepare the patch next week.
> > >
> > >  static long
> > >  check_and_migrate_movable_pages_or_folios(struct pages_or_folios *pofs)
> > >  {
> > > +       bool any_unpinnable;
> > >         LIST_HEAD(movable_folio_list);
> > >
> > > -       collect_longterm_unpinnable_folios(&movable_folio_list, pofs);
> > > -       if (list_empty(&movable_folio_list))
> > > -               return 0;
> > > +       any_unpinnable = collect_longterm_unpinnable_folios(&movable_folio_list, pofs);
> > > +       if (list_empty(&movable_folio_list)) {
> > > +               if (any_unpinnable)
> > > +                       pofs_unpin(pofs);
> >
> > I think this is correct, although as I mentioned in the other thread,
> > that implies that commit 1aaf8c122918 (which didn't add nor remove
> > any pof unpinning) is probably not the true or only culprit, right?
> >
> > > +               return any_unpinnable ? -EAGAIN : 0;
> >
> > Ha, the "?" operator almost always does more harm than good.
> >
> > Here, for example, it has obscured from you the fact that any_unpinnable
> > is being checked twice, when you could have merged those into a single "if".
> >
>
> Hello,
>
> I was wondering if the original problem - an infinite loop when pages allocated by
> cma_alloc() in vm_ops->fault are passed to GUP - still remains unresolved.
> (To be honest, I'm not quite sure how such pages end up being pinned via GUP.
>  Is that the expected behavior, or could it possibly indicate a bug ?)
The original problem arises from applying CMA as guestOS's memory
slots for kvm which use GUP to setup its 2nd stage mapping(HVA->PFN).
You can check KVM code if you are interested.

> Would it be make sense to try calling __lru_add_drain_all(false) in collect_longterm_unpinnable_folios ?
Please be noted that not all the pages allocated from vm_ops->fault
will be added to LRU.
> We could consider limiting the number of -EAGAIN retries to a fixed count (e.g., 5 attempts) to
> avoid an infinite loop. Or perhaps in check_and_migrate_movable_pages_or_folio(),
> if the list is empty but any_unpinnable is true, we should consider returning an error
> instead of EAGAIN to explicitly prevent longterm pinning of CMA allocated memory.
>
> I'd be interested to hear what others think.
>
> Thanks,
> Hyesoo Yu.
>
>
> > > +       }
> > >
> > >         return migrate_longterm_unpinnable_folios(&movable_folio_list, pofs);
> > >  }
> > thanks,
> > --
> > John Hubbard
> >
> >