Hi David, On Fri, Aug 02, 2024 at 05:55:20PM +0200, David Hildenbrand wrote: > continue; > } > > - /* FOLL_DUMP to ignore special (like zero) pages */ > - page = follow_page(vma, addr, FOLL_GET | FOLL_DUMP); > - > - if (IS_ERR_OR_NULL(page)) > + folio = folio_walk_start(&fw, vma, addr, 0); > + if (!folio) > continue; > > - folio = page_folio(page); > if (!is_transparent_hugepage(folio)) > goto next; > > @@ -3544,13 +3542,19 @@ static int split_huge_pages_pid(int pid, unsigned long vaddr_start, > > if (!folio_trylock(folio)) > goto next; > + folio_get(folio); Shouldn't we lock the folio after we increase the refcount on the folio? i.e we do folio_get() first and then folio_trylock()? That is how it was done before (through follow_page) and this patch changes that. Maybe it doesn't matter? To me increasing the refcount and then locking sounds more logical but I do see this ordering getting mixed all over the kernel. > + folio_walk_end(&fw, vma); > > if (!split_folio_to_order(folio, new_order)) > split++; > > folio_unlock(folio); > -next: > folio_put(folio); > + > + cond_resched(); > + continue; > +next: > + folio_walk_end(&fw, vma); > cond_resched(); > } > mmap_read_unlock(mm); > -- > 2.45.2 -- Pankaj Raghav
