2023-06-02 17:22:28

by Sidhartha Kumar

[permalink] [raw]
Subject: Re: [PATCH 1/2] mm/hugetlb: Use a folio in copy_hugetlb_page_range()

On 6/1/23 6:54 PM, Peng Zhang wrote:
> From: ZhangPeng <[email protected]>
>
> We can replace five implict calls to compound_head() with one by using
> pte_folio. However, we still need to keep ptepage because we need to know
> which page in the folio we are copying.
>
> Suggested-by: Matthew Wilcox (Oracle) <[email protected]>
> Signed-off-by: ZhangPeng <[email protected]>
> ---
> mm/hugetlb.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index ea24718db4af..0b774dd3d57b 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -5017,6 +5017,7 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
> {
> pte_t *src_pte, *dst_pte, entry;
> struct page *ptepage;
> + struct folio *pte_folio;
> unsigned long addr;
> bool cow = is_cow_mapping(src_vma->vm_flags);
> struct hstate *h = hstate_vma(src_vma);
> @@ -5116,7 +5117,8 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
> } else {
> entry = huge_ptep_get(src_pte);
> ptepage = pte_page(entry);
> - get_page(ptepage);
> + pte_folio = page_folio(ptepage);
> + folio_get(pte_folio);
>
> /*
> * Failing to duplicate the anon rmap is a rare case
> @@ -5128,7 +5130,7 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
> * need to be without the pgtable locks since we could
> * sleep during the process.
> */
> - if (!PageAnon(ptepage)) {
> + if (!folio_test_anon(pte_folio)) {
> page_dup_file_rmap(ptepage, true);
> } else if (page_try_dup_anon_rmap(ptepage, true,
> src_vma)) {
> @@ -5140,14 +5142,14 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
> /* Do not use reserve as it's private owned */
> new_folio = alloc_hugetlb_folio(dst_vma, addr, 1);
> if (IS_ERR(new_folio)) {
> - put_page(ptepage);
> + folio_put(pte_folio);
> ret = PTR_ERR(new_folio);
> break;
> }
> ret = copy_user_large_folio(new_folio,
> - page_folio(ptepage),
> - addr, dst_vma);
> - put_page(ptepage);
> + pte_folio,
> + addr, dst_vma);
> + folio_put(pte_folio);
> if (ret) {
> folio_put(new_folio);
> break;

Reviewed-by Sidhartha Kumar <[email protected]>