lkml.org 
[lkml]   [2021]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 2/2] mm/hugetlb: refactor subpage recording
    From
    Date
    On 1/26/21 9:21 PM, Mike Kravetz wrote:
    > On 1/26/21 11:21 AM, Joao Martins wrote:
    >> On 1/26/21 6:08 PM, Mike Kravetz wrote:
    >>> On 1/25/21 12:57 PM, Joao Martins wrote:
    >>>>
    >>>> +static void record_subpages_vmas(struct page *page, struct vm_area_struct *vma,
    >>>> + int refs, struct page **pages,
    >>>> + struct vm_area_struct **vmas)
    >>>> +{
    >>>> + int nr;
    >>>> +
    >>>> + for (nr = 0; nr < refs; nr++) {
    >>>> + if (likely(pages))
    >>>> + pages[nr] = page++;
    >>>> + if (vmas)
    >>>> + vmas[nr] = vma;
    >>>> + }
    >>>> +}
    >>>> +
    >>>> long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
    >>>> struct page **pages, struct vm_area_struct **vmas,
    >>>> unsigned long *position, unsigned long *nr_pages,
    >>>> @@ -4918,28 +4932,16 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
    >>>> continue;
    >>>> }
    >>>>
    >>>> - refs = 0;
    >>>> + refs = min3(pages_per_huge_page(h) - pfn_offset,
    >>>> + (vma->vm_end - vaddr) >> PAGE_SHIFT, remainder);
    >>>>
    >>>> -same_page:
    >>>> - if (pages)
    >>>> - pages[i] = mem_map_offset(page, pfn_offset);
    >>>> + if (pages || vmas)
    >>>> + record_subpages_vmas(mem_map_offset(page, pfn_offset),
    >>>
    >>> The assumption made here is that mem_map is contiguous for the range of
    >>> pages in the hugetlb page. I do not believe you can make this assumption
    >>> for (gigantic) hugetlb pages which are > MAX_ORDER_NR_PAGES. For example,
    >>>
    >
    > Thinking about this a bit more ...
    >
    > mem_map can be accessed contiguously if we have a virtual memmap. Correct?

    Right.

    > I suspect virtual memmap may be the most common configuration today. However,
    > it seems we do need to handle other configurations.
    >

    At the moment mem_map_offset(page, n) in turn does this for >= MAX_ORDER:

    pfn_to_page(page_to_pfn(page) + n)


    For CONFIG_SPARSE_VMEMMAP or FLATMEM will resolve into something:

    vmemmap + ((page - vmemmap) + n)

    It isn't really different than incrementing the @page.

    I can only think that CONFIG_SPARSEMEM and CONFIG_DISCONTIGMEM as the offending
    cases which respectively look into section info or pgdat.

    [CONFIG_DISCONTIGMEM doesnt isn't auto selected by any arch at the moment.]

    >> That would mean get_user_pages_fast() and pin_user_pages_fast() are broken for anything
    >> handling PUDs or above? See record_subpages() in gup_huge_pud() or even gup_huge_pgd().
    >> It's using the same page++.
    >
    > Yes, I believe those would also have the issue.
    > Cc: John and Jason as they have spent a significant amount of time in gup
    > code recently. There may be something that makes that code safe?
    >
    Maybe -- Looking back, gup-fast has always relied on that page pointer arithmetic, even
    before its refactors around __record_subpages() and what not.

    >> This adjustment below probably is what you're trying to suggest.
    >>
    >> Also, nth_page() is slightly more expensive and so the numbers above change from ~4.4k
    >> usecs to ~7.8k usecs.
    >
    > If my thoughts about virtual memmap are correct, then could we simply have
    > a !vmemmap version of mem_map_offset (or similar routine) to avoid overhead?
    > In that case, we could ifdef out on SPARSEMEM || DISCONTIGMEM for mem_map_offset() either
    internally or within the helper I added for follow_hugetlb_page().

    \
     
     \ /
      Last update: 2021-01-27 04:48    [W:4.732 / U:0.360 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site