Re: [PATCH v1 07/11] mm/huge_memory: convert split_huge_pages_pid() from follow_page() to folio_walk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 15.08.24 12:04, Pankaj Raghav wrote:
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.

There is no need to grab a folio reference if we hold an implicit reference through the mapping that cannot go away (not that we hold the page table lock). Locking the folio is not special in that regard: we just have to make sure that the folio cannot get freed concurrently, which is the case here.

So here, we really only grab a reference if we have to -- when we are about to drop the page table lock and will continue using the folio afterwards.

--
Cheers,

David / dhildenb





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux