Messages in this thread | ![/](/images/icornerl.gif) | | Date | Sat, 8 Apr 2023 12:42:42 +0800 | Subject | Re: [PATCH v5 5/6] mm: convert copy_user_huge_page() to copy_user_folio() | From | "zhangpeng (AS)" <> |
| |
On 2023/4/7 7:55, Mike Kravetz wrote:
> On 03/31/23 17:39, Peng Zhang wrote: >> From: ZhangPeng <zhangpeng362@huawei.com> >> >> Replace copy_user_huge_page() with copy_user_folio(). copy_user_folio() >> does the same as copy_user_huge_page(), but takes in folios instead of >> pages. Convert copy_user_gigantic_page() to take in folios. >> Remove pages_per_huge_page from copy_user_folio(), because we can get >> that from folio_nr_pages(dst). >> >> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com> >> --- >> include/linux/mm.h | 7 +++---- >> mm/hugetlb.c | 10 ++++------ >> mm/memory.c | 28 ++++++++++++++-------------- >> 3 files changed, 21 insertions(+), 24 deletions(-) > No technical problems with the patch, but ... >> >> @@ -5847,15 +5847,15 @@ static void copy_subpage(unsigned long addr, int idx, void *arg) >> addr, copy_arg->vma); >> } >> >> -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) >> +void copy_user_folio(struct folio *dst, struct folio *src, >> + unsigned long addr_hint, struct vm_area_struct *vma) >> { >> + unsigned int pages_per_huge_page = folio_nr_pages(dst); >> unsigned long addr = addr_hint & >> ~(((unsigned long)pages_per_huge_page << PAGE_SHIFT) - 1); >> struct copy_subpage_arg arg = { >> - .dst = dst, >> - .src = src, >> + .dst = &dst->page, >> + .src = &src->page, >> .vma = vma, >> }; >> > I seem to recall that Matthew suggested changing the function name to > copy_user_folio. My only concern is that the name now sounds like a > general purpose routine for copying folios. It certainly would work > for a single page folio, but there is a bunch of unnecessary overhead > in that case. > > That makes me think there should perhaps be an optimized path for single > page folios that just does copy_user_highpage(). But, the argument addr_hint > does not make much sense in the single page folio case. So, I am not > sure if I agree with leaving large/huge out of the function name. > > Just wondering if Matthew has any additional thoughts?
Agreed. In my opinion, it's better to leave large/huge out of the function name. Also wondering if Matthew has any additional considerations?
Best Regards, Peng
| ![\](/images/icornerr.gif) |