On Tue, Jan 21, 2025 at 4:14 AM David Hildenbrand <david@xxxxxxxxxx> wrote: > > On 20.01.25 10:26, zhaoyang.huang wrote: > > From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx> > > > > Infinite loop within __get_longterm_locked detected in an unique usage > > of pin_user_pages where the VA's pages are all unpinnable(vm_ops->fault > > function allocate pages via cma_alloc for hardware purpose and leave them > > out of LRU) Fixing this by have 'collected' reflect the actual number> of pages in movable_folio_list. > > Maybe something like: > > " > We can run into an infinite loop in __get_longterm_locked() when > collect_longterm_unpinnable_folios() finds only folios that are isolated > from the LRU or were never added to the LRU. This can happen when all > folios to be pinned are never added to the LRU, for example when > vm_ops->fault allocated pages using cma_alloc() and never added them to > the LRU. > > We incorrectly update the "collected" variable even if nothing was > collected. Fix it by incrementing "collected" only when we isolated a > folio and added it to the list of folios to migrate. > " > > I assume, long-term these things will not actually be folios, but pages, > and we'll have to skip them in different code -- or assume they can be > longterm pinned even on CMA because they are allocated by the CMA-owning > driver. Thanks for the commit message. will update them to v2 > > > > > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx> > > --- > > mm/gup.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/mm/gup.c b/mm/gup.c > > index 3b75e631f369..2231ce7221f9 100644 > > --- a/mm/gup.c > > +++ b/mm/gup.c > > @@ -2341,8 +2341,6 @@ static unsigned long collect_longterm_unpinnable_folios( > > if (folio_is_longterm_pinnable(folio)) > > continue; > > > > - collected++; > > - > > if (folio_is_device_coherent(folio)) > > continue; > > > > @@ -2359,6 +2357,8 @@ static unsigned long collect_longterm_unpinnable_folios( > > if (!folio_isolate_lru(folio)) > > continue; > > > > + collected++; > > + > > list_add_tail(&folio->lru, movable_folio_list); > > node_stat_mod_folio(folio, > > NR_ISOLATED_ANON + folio_is_file_lru(folio), > > What if folio_isolate_hugetlb() succeeded? The return value can tell us > if it actually succeeded. How about remove the variable 'collected' and change the criteria to if(list_empty(&movable_folio_list)) > > -- > Cheers, > > David / dhildenb >
