Re: [PATCH 2/3] userfaultfd: convert __mcopy_atomic_hugetlb() to use a folio

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

 



On 2023/3/14 16:31, Matthew Wilcox wrote:

On Tue, Mar 14, 2023 at 03:37:33AM +0000, Peng Zhang wrote:
+++ b/include/linux/mm.h
@@ -3546,9 +3546,8 @@ extern void copy_user_huge_page(struct page *dst, struct page *src,
  				unsigned long addr_hint,
  				struct vm_area_struct *vma,
  				unsigned int pages_per_huge_page);
-extern long copy_huge_page_from_user(struct page *dst_page,
+extern long copy_large_folio_from_user(struct folio *dst_folio,
You can drop the 'extern'.

Got it.

+++ b/mm/memory.c
@@ -5769,26 +5769,28 @@ void copy_user_huge_page(struct page *dst, struct page *src,
  	process_huge_page(addr_hint, pages_per_huge_page, copy_subpage, &arg);
  }
-long copy_huge_page_from_user(struct page *dst_page,
+long copy_large_folio_from_user(struct folio *dst_folio,
  				const void __user *usr_src,
-				unsigned int pages_per_huge_page,
  				bool allow_pagefault)
  {
  	void *page_kaddr;
  	unsigned long i, rc = 0;
-	unsigned long ret_val = pages_per_huge_page * PAGE_SIZE;
+	unsigned int nr_pages = folio_nr_pages(dst_folio);
+	unsigned long ret_val = nr_pages * PAGE_SIZE;
  	struct page *subpage;
+	struct folio *inner_folio;
What is an 'inner folio'?

-	for (i = 0; i < pages_per_huge_page; i++) {
-		subpage = nth_page(dst_page, i);
+	for (i = 0; i < nr_pages; i++) {
+		subpage = folio_page(dst_folio, i);
+		inner_folio = page_folio(subpage);
  		if (allow_pagefault)
-			page_kaddr = kmap(subpage);
+			page_kaddr = kmap_local_folio(inner_folio, 0);
This doesn't do what you think it does.  Did you test this?

  		else
  			page_kaddr = kmap_atomic(subpage);
Pretty sure all this should be converted to kmap_local and the atomic
bits should go away.

  		rc = copy_from_user(page_kaddr,
  				usr_src + i * PAGE_SIZE, PAGE_SIZE);
  		if (allow_pagefault)
-			kunmap(subpage);
+			kunmap_local(page_kaddr);
  		else
  			kunmap_atomic(page_kaddr);
@@ -5796,7 +5798,7 @@ long copy_huge_page_from_user(struct page *dst_page,
  		if (rc)
  			break;
- flush_dcache_page(subpage);
+		flush_dcache_folio(inner_folio);
The flush should probably be pulled outside the loop.

+			err = copy_large_folio_from_user(folio,
+						(const void __user *) src_addr, true);
I wonder if this shouldn't be 'copy_folio_from_user()'.  after all,
it'll work for any size folio, right?

Thanks for your review.

I'll rename copy_large_folio_from_user() to copy_folio_from_user().
I'll delete the inner_folio. kmap() and kmap_atomic() will be
converted to the following code.

	page_kaddr = kmap_local_page(subpage);
	if (!allow_pagefault)
		pagefault_disable();
	rc = copy_from_user(page_kaddr,
			usr_src + i * PAGE_SIZE, PAGE_SIZE);
	if (!allow_pagefault)
		pagefault_enable();
	kunmap_local(page_kaddr);

flush_dcache_folio() will be placed outside the loop.

I'll fix all this in a v2 of this patch series.


Thanks,
Peng.





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux