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'.
> +++ 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?
